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

fix: minimise bracket printing in case of string operands of binary operator #4809

Merged
merged 5 commits into from
Jul 25, 2022
Merged

fix: minimise bracket printing in case of string operands of binary operator #4809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49b86be
Reproduce #4807
algomaster99 Jul 20, 2022
71ceeef
Switch order of binary operators for StringLiteralConcatenation
algomaster99 Jul 20, 2022
986db4d
Fix bracket printing in CommentTest::testInLineComment
algomaster99 Jul 20, 2022
8d59817
Add test to verify patch at AST level
algomaster99 Jul 21, 2022
60d6e64
Remove line break
algomaster99 Jul 21, 2022
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
7 changes: 4 additions & 3 deletions src/main/java/spoon/support/compiler/jdt/ParentExiter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,11 @@ public <T> void visitCtBinaryOperator(CtBinaryOperator<T> operator) {
} else if (jdtTreeBuilder.getContextBuilder().stack.peek().node instanceof StringLiteralConcatenation) {
CtBinaryOperator<?> op = operator.getFactory().Core().createBinaryOperator();
op.setKind(BinaryOperatorKind.PLUS);
op.setLeftHandOperand(operator.getRightHandOperand());
op.setRightHandOperand((CtExpression<?>) child);
op.setLeftHandOperand(operator.getLeftHandOperand());
op.setRightHandOperand(operator.getRightHandOperand());
op.setType((CtTypeReference) operator.getFactory().Type().STRING.clone());
operator.setRightHandOperand(op);
operator.setLeftHandOperand(op);
operator.setRightHandOperand(((CtExpression<?>) child));
Comment on lines -448 to +452
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done exactly what I have explained here. I ensured that the left operand takes the nested operator whenever possible.

int[] lineSeparatorPositions = jdtTreeBuilder.getContextBuilder().getCompilationUnitLineSeparatorPositions();
SourcePosition leftPosition = op.getLeftHandOperand().getPosition();
SourcePosition rightPosition = op.getRightHandOperand().getPosition();
Expand Down
28 changes: 27 additions & 1 deletion src/test/java/spoon/reflect/ast/AstCheckerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@
import spoon.reflect.declaration.CtClass;
import spoon.reflect.reference.CtExecutableReference;
import spoon.support.modelobs.FineModelChangeListener;
import spoon.reflect.code.CtBinaryOperator;
import spoon.reflect.code.CtBlock;
import spoon.reflect.code.CtExpression;
import spoon.reflect.code.CtFieldRead;
import spoon.reflect.code.CtIf;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtReturn;
import spoon.reflect.code.CtThrow;
import spoon.reflect.declaration.CtExecutable;
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.declaration.ModifierKind;
import spoon.reflect.factory.Factory;
Expand All @@ -50,10 +51,35 @@
import java.util.Set;
import java.util.stream.Collectors;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.junit.jupiter.api.Assertions.assertEquals;

public class AstCheckerTest {

@Test
void leftOperandShouldBeGivenPriorityForStoringTheNestedOperator_stringLiteralConcatenation() {
// contract: string concatenation should be left associative.
// arrange
Launcher launcher = new Launcher();
Factory factory = launcher.getFactory();
CtClass<?> classContainingStringLiteral = Launcher.parseClass("class A { private String x = \"a\" + \"b\" + \"c\" }");

// act
CtBinaryOperator<?> binaryOperator = classContainingStringLiteral
.filterChildren(element -> element instanceof CtBinaryOperator)
.first();

// assert
CtExpression<?> firstOperand = ((CtBinaryOperator<?>)binaryOperator.getLeftHandOperand()).getLeftHandOperand();
CtExpression<?> secondOperand = ((CtBinaryOperator<?>)binaryOperator.getLeftHandOperand()).getRightHandOperand();
CtExpression<?> thirdOperand = binaryOperator.getRightHandOperand();

assertThat(firstOperand, equalTo(factory.createLiteral("a")));
assertThat(secondOperand, equalTo(factory.createLiteral("b")));
assertThat(thirdOperand, equalTo(factory.createLiteral("c")));
}

@Test
public void testExecutableReference() {

Expand Down
3 changes: 2 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 @@ -57,7 +57,8 @@ public void testParenOptimizationCorrectlyPrintsParenthesesForExpressions(String
@ValueSource(strings = {
"int sum = 1 + 2 + 3",
"java.lang.String s = \"Sum: \" + (1 + 2)",
"java.lang.String s = \"Sum: \" + 1 + 2"
"java.lang.String s = \"Sum: \" + 1 + 2",
"java.lang.System.out.println(\"1\" + \"2\" + \"3\" + \"4\")"
})
public void testParenOptimizationCorrectlyPrintsParenthesesForStatements(String rawStatement) {
// contract: When input expressions as part of statements are minimally parenthesized,
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/spoon/test/comment/CommentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,9 +465,9 @@ public void testInLineComment() {


CtLocalVariable ctLocalVariableString = m1.getBody().getStatement(12);
assertEquals(createFakeComment(f, "comment multi line string"), ((CtBinaryOperator) ((CtBinaryOperator) ctLocalVariableString.getDefaultExpression()).getRightHandOperand()).getLeftHandOperand().getComments().get(0));
assertEquals("\"\" + (\"\"// comment multi line string" + newLine
+ " + \"\")", ctLocalVariableString.getDefaultExpression().toString());
assertEquals(createFakeComment(f, "comment multi line string"), (((CtBinaryOperator) ctLocalVariableString.getDefaultExpression()).getLeftHandOperand()).getComments().get(0));
assertEquals("(\"\" + \"\")// comment multi line string" + newLine
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minimizeRoundBrackets was false for this test case. Hence, I rearranged the brackets.

+ " + \"\"", ctLocalVariableString.getDefaultExpression().toString());

ctLocalVariable1 = m1.getBody().getStatement(13);
assertEquals("boolean c = (i == 1) ? // comment before then boolean CtConditional" + newLine
Expand Down