Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

review: fix: consolidate and improve _targeted expression precedence #6105

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

review: fix: consolidate and improve _targeted expression precedence #6105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0244fe0
fix: consolidate and improve _targeted expression precedence
SirYwell Dec 17, 2024
667905c
formatting
SirYwell Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,25 +424,42 @@ public DefaultJavaPrettyPrinter scan(CtElement e) {
}

private boolean shouldSetBracketAroundExpressionAndCast(CtExpression<?> e) {
boolean hasCasts = !e.getTypeCasts().isEmpty();
if (isMinimizeRoundBrackets()) {
RoundBracketAnalyzer.EncloseInRoundBrackets requiresBrackets =
RoundBracketAnalyzer.requiresRoundBrackets(e);
RoundBracketAnalyzer.requiresRoundBrackets(e);
if (requiresBrackets != RoundBracketAnalyzer.EncloseInRoundBrackets.UNKNOWN) {
return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES || !e.getTypeCasts().isEmpty();
return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES || hasCasts;
}
if (e.isParentInitialized() && e.getParent() instanceof Ct_targetedExpression && ((Ct_targetedExpression) e.getParent()).get_target() == e) {
return e instanceof CtVariableRead<?> && !e.getTypeCasts().isEmpty() || e instanceof CtBinaryOperator<?>;
}
} else if (!e.getTypeCasts().isEmpty()) {
} else if (hasCasts) {
return true;
}
if (e.isParentInitialized()) {
if ((e.getParent() instanceof CtBinaryOperator) || (e.getParent() instanceof CtUnaryOperator)) {
return (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || e instanceof CtBinaryOperator;
}
if (e.getParent() instanceof Ct_targetedExpression && ((Ct_targetedExpression) e.getParent()).get_target() == e) {
return (e instanceof CtBinaryOperator) || (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator);
}
return requiresParenthesesIn_targetContext(e);
}
return false;
}

/**
* {@return whether the expression requires parentheses if it is the child of a _targeted expression}
* The _targets of method calls typically don't require additional parentheses, e.g.,
* {@code var.method()}, {@code method().method()}. However, some expressions do, e.g.,
* {@code (x = y).method()}, {@code ("a" + "b").method()}.
* If the expression is not the _target of a method call, {@code false} is returned.
* <br/>
* Note: This method does not consider casts.
*/
private static boolean requiresParenthesesIn_targetContext(CtExpression<?> expression) {
if (expression.isParentInitialized() && expression.getParent() instanceof Ct_targetedExpression<?, ?> _targeted && _targeted.get_target() == expression) {
return !expression.getTypeCasts().isEmpty()
|| expression instanceof CtBinaryOperator<?>
|| expression instanceof CtSwitchExpression<?, ?>
|| expression instanceof CtAssignment<?, ?>
|| expression instanceof CtConditional<?>
|| expression instanceof CtUnaryOperator<?>;
}
return false;
}
Expand Down
17 changes: 16 additions & 1 deletion src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
import spoon.reflect.reference.CtArrayTypeReference;
import spoon.reflect.reference.CtTypeReference;
import spoon.reflect.visitor.filter.TypeFilter;
import spoon.support.compiler.VirtualFile;
import spoon.support.reflect.reference.CtArrayTypeReferenceImpl;
import spoon.test.SpoonTestHelpers;
import spoon.testing.assertions.SpoonAssertions;
import spoon.testing.utils.GitHubIssue;
import spoon.testing.utils.ModelTest;

Expand Down Expand Up @@ -78,13 +80,26 @@ public class DefaultJavaPrettyPrinterTest {
"((int) (1 + 2)) * 3",
"(int) (int) (1 + 1)",
"(\"1\" + \"2\").contains(\"1\")",
"null instanceof java.lang.String s && (s = \"1\").contains(\"1\")",
"null instanceof java.lang.String s && (s += \"1\").contains(\"1\")",
"null instanceof java.lang.String[] arr && (arr[0] = \"1\").contains(\"1\")",
"null instanceof java.lang.Integer i && (++i).toString().isEmpty()",
"null instanceof java.lang.Integer i && (i--).toString().isEmpty()",
"(true ? \"1\" : \"2\").contains(\"1\")",
"""
(switch (0) {
default -> "1";
}).contains("1")
""",
})
public void testParenOptimizationCorrectlyPrintsParenthesesForExpressions(String rawExpression) {
// contract: When input expressions are minimally parenthesized, pretty-printed output
// should match the input
CtExpression<?> expr = createLauncherWithOptimizeParenthesesPrinter()
.getFactory().createCodeSnippetExpression(rawExpression).compile();
assertThat(expr.toString(), equalTo(rawExpression));
SpoonAssertions.assertThat(expr)
.asString()
.containsIgnoringWhitespaces(rawExpression);
}

@ParameterizedTest
Expand Down
Loading
  NODES
COMMUNITY 2
Note 2
Project 2
USERS 1