From ee363b180cbaf140592fab8b765cd43e7a51f840 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Tue, 1 Nov 2022 06:58:47 +0100 Subject: [PATCH 01/16] Introduce failing test cases --- .../spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java index 1e557a7b068..849b5cf78de 100644 --- a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -78,7 +78,9 @@ public void testParenOptimizationCorrectlyPrintsParenthesesForExpressions(String "int sum = 1 + 2 + 3", "java.lang.String s = \"Sum: \" + (1 + 2)", "java.lang.String s = \"Sum: \" + 1 + 2", - "java.lang.System.out.println(\"1\" + \"2\" + \"3\" + \"4\")" + "java.lang.System.out.println(\"1\" + \"2\" + \"3\" + \"4\")", + "int myInt = (int) 0.0d", + "int myInt = (int) (float) 0.0d", }) public void testParenOptimizationCorrectlyPrintsParenthesesForStatements(String rawStatement) { // contract: When input expressions as part of statements are minimally parenthesized, From edea7200fee86094753055ed38a7df89368fcbad Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Tue, 1 Nov 2022 07:02:56 +0100 Subject: [PATCH 02/16] Apply fix --- .../spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 5b74eb52646..fccfc0aa770 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -305,8 +305,10 @@ protected void enterCtExpression(CtExpression e) { printer.writeSeparator("("); scan(r); printer.writeSeparator(")").writeSpace(); - printer.writeSeparator("("); - context.parenthesedExpression.push(e); + if (!isMinimizeRoundBrackets()) { + printer.writeSeparator("("); + context.parenthesedExpression.push(e); + } } } } @@ -400,7 +402,7 @@ public DefaultJavaPrettyPrinter scan(CtElement e) { } private boolean shouldSetBracket(CtExpression e) { - if (!e.getTypeCasts().isEmpty()) { + if (!e.getTypeCasts().isEmpty() && !isMinimizeRoundBrackets()) { return true; } try { From 2c4fc67e79ff4220b025debf3063ebfa9f02907b Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Tue, 1 Nov 2022 07:10:51 +0100 Subject: [PATCH 03/16] Remove 'd' suffix --- .../spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java index 849b5cf78de..2cc06642dec 100644 --- a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -79,8 +79,8 @@ public void testParenOptimizationCorrectlyPrintsParenthesesForExpressions(String "java.lang.String s = \"Sum: \" + (1 + 2)", "java.lang.String s = \"Sum: \" + 1 + 2", "java.lang.System.out.println(\"1\" + \"2\" + \"3\" + \"4\")", - "int myInt = (int) 0.0d", - "int myInt = (int) (float) 0.0d", + "int myInt = (int) 0.0", + "int myInt = (int) (float) 0.0", }) public void testParenOptimizationCorrectlyPrintsParenthesesForStatements(String rawStatement) { // contract: When input expressions as part of statements are minimally parenthesized, From a566d5e560e526e7de9ad7fa2647693f60112de3 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Tue, 1 Nov 2022 07:42:26 +0100 Subject: [PATCH 04/16] Reproduce user's issue --- .../visitor/DefaultJavaPrettyPrinterTest.java | 16 ++++++++++++++++ .../printer-test/TypeCastOnFieldRead.java | 6 ++++++ 2 files changed, 22 insertions(+) create mode 100644 src/test/resources/printer-test/TypeCastOnFieldRead.java diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java index 2cc06642dec..43c5bb4018d 100644 --- a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -27,6 +27,7 @@ import spoon.reflect.path.CtRole; import spoon.reflect.reference.CtArrayTypeReference; import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.visitor.filter.TypeFilter; import spoon.support.reflect.reference.CtArrayTypeReferenceImpl; import spoon.test.GitHubIssue; import spoon.test.SpoonTestHelpers; @@ -324,4 +325,19 @@ void testKeepGenericType(Factory factory) { assertThat(printed, containsRegexMatch("List<.*List<\\? extends T>>")); assertThat(printed, containsRegexMatch("List<.*List<\\? super T>>")); } + + @GitHubIssue(issueNumber = 4881, fixed = true) + void bracketsShouldBeMinimallyPrintedForTypeCastOnFieldRead() { + // contract: the brackets should be minimally printed for type cast on field read + // arrange + Launcher launcher = createLauncherWithOptimizeParenthesesPrinter(); + launcher.addInputResource("src/test/resources/printer-test/TypeCastOnFieldRead.java"); + + // act + CtModel model = launcher.buildModel(); + + // assert + CtLocalVariable localVariable = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(0); + assertThat(localVariable.toString(), equalTo("int myInt = (int) myDouble")); + } } diff --git a/src/test/resources/printer-test/TypeCastOnFieldRead.java b/src/test/resources/printer-test/TypeCastOnFieldRead.java new file mode 100644 index 00000000000..a3d5f19616e --- /dev/null +++ b/src/test/resources/printer-test/TypeCastOnFieldRead.java @@ -0,0 +1,6 @@ +public class TypeCastOnFieldRead { + double myDouble = 0.0; + public void where() { + int myInt = (int) myDouble; + } +} From 24385dc7b4df9993732806e08fbbf1af0105e7af Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Thu, 10 Nov 2022 16:34:54 +0100 Subject: [PATCH 05/16] Compile pretty-printed string --- .../reflect/visitor/DefaultJavaPrettyPrinterTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java index 43c5bb4018d..4cbd4099947 100644 --- a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -10,6 +10,8 @@ import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.ValueSource; import spoon.Launcher; +import spoon.SpoonModelBuilder; +import spoon.compiler.SpoonResourceHelper; import spoon.reflect.CtModel; import spoon.reflect.code.CtBlock; import spoon.reflect.code.CtExecutableReferenceExpression; @@ -33,6 +35,7 @@ import spoon.test.SpoonTestHelpers; import spoon.testing.utils.ModelTest; +import java.io.FileNotFoundException; import java.util.ArrayList; import java.util.List; import java.util.stream.Stream; @@ -327,16 +330,21 @@ void testKeepGenericType(Factory factory) { } @GitHubIssue(issueNumber = 4881, fixed = true) - void bracketsShouldBeMinimallyPrintedForTypeCastOnFieldRead() { + void bracketsShouldBeMinimallyPrintedForTypeCastOnFieldRead() throws FileNotFoundException { // contract: the brackets should be minimally printed for type cast on field read // arrange Launcher launcher = createLauncherWithOptimizeParenthesesPrinter(); launcher.addInputResource("src/test/resources/printer-test/TypeCastOnFieldRead.java"); + Launcher launcherForCompilingPrettyPrintedString = createLauncherWithOptimizeParenthesesPrinter(); // act CtModel model = launcher.buildModel(); + launcher.prettyprint(); + SpoonModelBuilder spoonModelBuilder = launcherForCompilingPrettyPrintedString.createCompiler(SpoonResourceHelper.resources("spooned/TypeCastOnFieldRead.java")); // assert + assertThat(spoonModelBuilder.build(), equalTo(true)); + CtLocalVariable localVariable = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(0); assertThat(localVariable.toString(), equalTo("int myInt = (int) myDouble")); } From 5361c2ae9ad5db961d5fc5f280f270337ce234d2 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Thu, 10 Nov 2022 16:39:23 +0100 Subject: [PATCH 06/16] Incorporate @SirYwell's test case Co-Authored-By: Hannes Greule --- src/test/resources/printer-test/TypeCastOnFieldRead.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/resources/printer-test/TypeCastOnFieldRead.java b/src/test/resources/printer-test/TypeCastOnFieldRead.java index a3d5f19616e..576d5f1a0cd 100644 --- a/src/test/resources/printer-test/TypeCastOnFieldRead.java +++ b/src/test/resources/printer-test/TypeCastOnFieldRead.java @@ -2,5 +2,6 @@ public class TypeCastOnFieldRead { double myDouble = 0.0; public void where() { int myInt = (int) myDouble; + int myInt2 = ((Double) myDouble).intValue(); } } From 4c681343f10f6f755ecc77878b6751e59a14e1fc Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Thu, 10 Nov 2022 16:42:29 +0100 Subject: [PATCH 07/16] Apply fix --- .../reflect/visitor/DefaultJavaPrettyPrinter.java | 12 ++++++++++++ .../visitor/DefaultJavaPrettyPrinterTest.java | 3 +++ 2 files changed, 15 insertions(+) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index fccfc0aa770..210c6ebe5bf 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -104,6 +104,7 @@ import spoon.reflect.declaration.CtUsedService; import spoon.reflect.declaration.CtVariable; import spoon.reflect.declaration.ParentNotInitializedException; +import spoon.reflect.path.CtRole; import spoon.reflect.reference.CtArrayTypeReference; import spoon.reflect.reference.CtCatchVariableReference; import spoon.reflect.reference.CtExecutableReference; @@ -413,6 +414,9 @@ private boolean shouldSetBracket(CtExpression e) { return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES; } } + if (isTargetOfInvocation(e)) { + return true; + } if ((e.getParent() instanceof CtBinaryOperator) || (e.getParent() instanceof CtUnaryOperator)) { return (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || e instanceof CtBinaryOperator; } @@ -425,6 +429,14 @@ private boolean shouldSetBracket(CtExpression e) { return false; } + /** + * If the target of an invocation is type-casted, it must be enclosed in round brackets before attaching the + * executable to it. + */ + private static boolean isTargetOfInvocation(CtExpression e) { + return e.getRoleInParent() == CtRole.TARGET; + } + /** * Gets the currently pretty-printed string. */ diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java index 4cbd4099947..fc2310d3e16 100644 --- a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -347,5 +347,8 @@ void bracketsShouldBeMinimallyPrintedForTypeCastOnFieldRead() throws FileNotFoun CtLocalVariable localVariable = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(0); assertThat(localVariable.toString(), equalTo("int myInt = (int) myDouble")); + + CtLocalVariable localVariable2 = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(1); + assertThat(localVariable2.toString(), equalTo("int myInt2 = ((java.lang.Double) myDouble).intValue()")); } } From 928b8043bb172a0f0ecfe0df0870220cab8140bb Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Thu, 10 Nov 2022 17:35:46 +0100 Subject: [PATCH 08/16] Make the fix more conservative --- .../reflect/visitor/DefaultJavaPrettyPrinter.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 210c6ebe5bf..7167bbd68e0 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -414,14 +414,11 @@ private boolean shouldSetBracket(CtExpression e) { return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES; } } - if (isTargetOfInvocation(e)) { - return true; - } 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 CtTargetedExpression && ((CtTargetedExpression) e.getParent()).getTarget() == e) { - return (e instanceof CtBinaryOperator) || (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator); + return (e instanceof CtBinaryOperator) || (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || (e instanceof CtFieldRead && !e.getTypeCasts().isEmpty()); } } catch (ParentNotInitializedException ex) { // nothing we accept not to have a parent @@ -429,14 +426,6 @@ private boolean shouldSetBracket(CtExpression e) { return false; } - /** - * If the target of an invocation is type-casted, it must be enclosed in round brackets before attaching the - * executable to it. - */ - private static boolean isTargetOfInvocation(CtExpression e) { - return e.getRoleInParent() == CtRole.TARGET; - } - /** * Gets the currently pretty-printed string. */ From b813c3f046774bcccd87acee44821bbadd31abc5 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Thu, 10 Nov 2022 17:52:36 +0100 Subject: [PATCH 09/16] Remove unused import --- .../java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 7167bbd68e0..b99524f70a7 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -104,7 +104,6 @@ import spoon.reflect.declaration.CtUsedService; import spoon.reflect.declaration.CtVariable; import spoon.reflect.declaration.ParentNotInitializedException; -import spoon.reflect.path.CtRole; import spoon.reflect.reference.CtArrayTypeReference; import spoon.reflect.reference.CtCatchVariableReference; import spoon.reflect.reference.CtExecutableReference; From 9555cfc2ca9ae4bcba377624cb265512a9b902e7 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Thu, 10 Nov 2022 18:18:03 +0100 Subject: [PATCH 10/16] Add test where there are no type casts --- .../spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java | 3 +++ src/test/resources/printer-test/TypeCastOnFieldRead.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java index fc2310d3e16..e2e847f9ffa 100644 --- a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -350,5 +350,8 @@ void bracketsShouldBeMinimallyPrintedForTypeCastOnFieldRead() throws FileNotFoun CtLocalVariable localVariable2 = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(1); assertThat(localVariable2.toString(), equalTo("int myInt2 = ((java.lang.Double) myDouble).intValue()")); + + CtLocalVariable localVariable3 = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(2); + assertThat(localVariable3.toString(), equalTo("double withoutTypeCast = myDoubleObject.doubleValue()")); } } diff --git a/src/test/resources/printer-test/TypeCastOnFieldRead.java b/src/test/resources/printer-test/TypeCastOnFieldRead.java index 576d5f1a0cd..818d22d8b50 100644 --- a/src/test/resources/printer-test/TypeCastOnFieldRead.java +++ b/src/test/resources/printer-test/TypeCastOnFieldRead.java @@ -1,7 +1,10 @@ public class TypeCastOnFieldRead { double myDouble = 0.0; + Double myDoubleObject = 0.0; + public void where() { int myInt = (int) myDouble; int myInt2 = ((Double) myDouble).intValue(); + double withoutTypeCast = myDoubleObject.doubleValue(); } } From 47b4ff8004c7658e83cfee488ba72d920a8a4f5e Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Mon, 14 Nov 2022 12:08:39 +0100 Subject: [PATCH 11/16] Add @SirYwell's test case --- .../visitor/DefaultJavaPrettyPrinterTest.java | 20 +++++++++++++++++++ .../printer-test/ShadowFieldRead.java | 14 +++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 src/test/resources/printer-test/ShadowFieldRead.java diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java index e2e847f9ffa..6546fa2e7cf 100644 --- a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -354,4 +354,24 @@ void bracketsShouldBeMinimallyPrintedForTypeCastOnFieldRead() throws FileNotFoun CtLocalVariable localVariable3 = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(2); assertThat(localVariable3.toString(), equalTo("double withoutTypeCast = myDoubleObject.doubleValue()")); } + + @Test + void bracketsShouldBeMinimallyPrintedOnShadowedFields() throws FileNotFoundException { + // contract: the brackets should be minimally printed for type cast on shadowed field read + // arrange + Launcher launcher = createLauncherWithOptimizeParenthesesPrinter(); + launcher.addInputResource("src/test/resources/printer-test/ShadowFieldRead.java"); + Launcher launcherForCompilingPrettyPrintedString = createLauncherWithOptimizeParenthesesPrinter(); + + // act + CtModel model = launcher.buildModel(); + launcher.prettyprint(); + SpoonModelBuilder spoonModelBuilder = launcherForCompilingPrettyPrintedString.createCompiler(SpoonResourceHelper.resources("spooned/ShadowFieldRead.java", "spooned/A.java", "spooned/C.java")); + + // assert + assertThat(spoonModelBuilder.build(), equalTo(true)); + + CtLocalVariable localVariable = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(1); + assertThat(localVariable.toString(), equalTo("int fieldReadOfA = ((A) c).a.i")); + } } diff --git a/src/test/resources/printer-test/ShadowFieldRead.java b/src/test/resources/printer-test/ShadowFieldRead.java new file mode 100644 index 00000000000..ecbb71e7955 --- /dev/null +++ b/src/test/resources/printer-test/ShadowFieldRead.java @@ -0,0 +1,14 @@ +class A { + A a; + int i; +} +class C extends A { + Object a; +} + +public class ShadowFieldRead { + public static void main(String[] args) { + C c = new C(); + int fieldReadOfA = ((A) c).a.i; + } +} \ No newline at end of file From e07a35914a8a66a0e47900bd56374fc537ffc788 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Mon, 14 Nov 2022 12:09:00 +0100 Subject: [PATCH 12/16] Add fix --- .../java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index b99524f70a7..4cfe9974be5 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -417,7 +417,7 @@ private boolean shouldSetBracket(CtExpression e) { return (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || e instanceof CtBinaryOperator; } if (e.getParent() instanceof CtTargetedExpression && ((CtTargetedExpression) e.getParent()).getTarget() == e) { - return (e instanceof CtBinaryOperator) || (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || (e instanceof CtFieldRead && !e.getTypeCasts().isEmpty()); + return (e instanceof CtBinaryOperator) || (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || (e instanceof CtVariableRead && !e.getTypeCasts().isEmpty()); } } catch (ParentNotInitializedException ex) { // nothing we accept not to have a parent From e8998760eb8a45b89fa97f6bb17ad70b401debea Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Tue, 15 Nov 2022 18:07:32 +0100 Subject: [PATCH 13/16] Refactor `shouldSetBracket` --- .../visitor/DefaultJavaPrettyPrinter.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 4cfe9974be5..98eda8c4ede 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -402,25 +402,22 @@ public DefaultJavaPrettyPrinter scan(CtElement e) { } private boolean shouldSetBracket(CtExpression e) { - if (!e.getTypeCasts().isEmpty() && !isMinimizeRoundBrackets()) { + if (isMinimizeRoundBrackets()) { + RoundBracketAnalyzer.EncloseInRoundBrackets requiresBrackets = + RoundBracketAnalyzer.requiresRoundBrackets(e); + if (requiresBrackets != RoundBracketAnalyzer.EncloseInRoundBrackets.UNKNOWN) { + return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES; + } + } else if (!e.getTypeCasts().isEmpty()) { return true; } - try { - if (isMinimizeRoundBrackets()) { - RoundBracketAnalyzer.EncloseInRoundBrackets requiresBrackets = - RoundBracketAnalyzer.requiresRoundBrackets(e); - if (requiresBrackets != RoundBracketAnalyzer.EncloseInRoundBrackets.UNKNOWN) { - return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES; - } - } + 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 CtTargetedExpression && ((CtTargetedExpression) e.getParent()).getTarget() == e) { - return (e instanceof CtBinaryOperator) || (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || (e instanceof CtVariableRead && !e.getTypeCasts().isEmpty()); + return (e instanceof CtBinaryOperator) || (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator); } - } catch (ParentNotInitializedException ex) { - // nothing we accept not to have a parent } return false; } From d574e12956180160169cce55a8d8cc1874ea2315 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Tue, 15 Nov 2022 18:46:35 +0100 Subject: [PATCH 14/16] Wrap brackets around variable read if there are type casts --- .../java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 98eda8c4ede..36c71649558 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -408,6 +408,9 @@ private boolean shouldSetBracket(CtExpression e) { if (requiresBrackets != RoundBracketAnalyzer.EncloseInRoundBrackets.UNKNOWN) { return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES; } + if (e.isParentInitialized() && e.getParent() instanceof CtTargetedExpression && ((CtTargetedExpression) e.getParent()).getTarget() == e) { + return e instanceof CtVariableRead && !e.getTypeCasts().isEmpty(); + } } else if (!e.getTypeCasts().isEmpty()) { return true; } From b82443b9244ecd77c643b271c12e85b4da46bb96 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Wed, 16 Nov 2022 10:26:43 +0100 Subject: [PATCH 15/16] Add trivial check --- .../java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 36c71649558..9a8b4a935c1 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -406,7 +406,7 @@ private boolean shouldSetBracket(CtExpression e) { RoundBracketAnalyzer.EncloseInRoundBrackets requiresBrackets = RoundBracketAnalyzer.requiresRoundBrackets(e); if (requiresBrackets != RoundBracketAnalyzer.EncloseInRoundBrackets.UNKNOWN) { - return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES; + return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES && e.getTypeCasts().isEmpty(); } if (e.isParentInitialized() && e.getParent() instanceof CtTargetedExpression && ((CtTargetedExpression) e.getParent()).getTarget() == e) { return e instanceof CtVariableRead && !e.getTypeCasts().isEmpty(); From 5579eec7715f0b7559c2f1f82e182231ec6afcfb Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Wed, 16 Nov 2022 18:45:34 +0100 Subject: [PATCH 16/16] Some comments Co-authored-by: Hannes Greule --- .../visitor/DefaultJavaPrettyPrinter.java | 32 +++++++++++++++---- .../visitor/DefaultJavaPrettyPrinterTest.java | 4 ++- src/test/java/spoon/test/eval/EvalTest.java | 2 +- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 9a8b4a935c1..ede37c5002c 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -68,6 +68,7 @@ import spoon.reflect.code.CtTypeAccess; import spoon.reflect.code.CtTypePattern; import spoon.reflect.code.CtUnaryOperator; +import spoon.reflect.code.CtVariableAccess; import spoon.reflect.code.CtVariableRead; import spoon.reflect.code.CtVariableWrite; import spoon.reflect.code.CtWhile; @@ -296,7 +297,7 @@ protected void enterCtExpression(CtExpression e) { elementPrinterHelper.writeComment(e, CommentOffset.BEFORE); } getPrinterHelper().mapLine(e, sourceCompilationUnit); - if (shouldSetBracket(e)) { + if (shouldSetBracketAroundExpressionAndCast(e)) { context.parenthesedExpression.push(e); printer.writeSeparator("("); } @@ -305,12 +306,29 @@ protected void enterCtExpression(CtExpression e) { printer.writeSeparator("("); scan(r); printer.writeSeparator(")").writeSpace(); - if (!isMinimizeRoundBrackets()) { - printer.writeSeparator("("); - context.parenthesedExpression.push(e); - } } + if (shouldSetBracketAroundCastTarget(e)) { + printer.writeSeparator("("); + context.parenthesedExpression.push(e); + } + } + } + + private boolean shouldSetBracketAroundCastTarget(CtExpression expr) { + if (!isMinimizeRoundBrackets()) { + return true; } + + if (expr instanceof CtTargetedExpression) { + return false; + } + if (expr instanceof CtLiteral) { + return false; + } + if (expr instanceof CtVariableAccess) { + return false; + } + return true; } /** @@ -401,12 +419,12 @@ public DefaultJavaPrettyPrinter scan(CtElement e) { return this; } - private boolean shouldSetBracket(CtExpression e) { + private boolean shouldSetBracketAroundExpressionAndCast(CtExpression e) { if (isMinimizeRoundBrackets()) { RoundBracketAnalyzer.EncloseInRoundBrackets requiresBrackets = RoundBracketAnalyzer.requiresRoundBrackets(e); if (requiresBrackets != RoundBracketAnalyzer.EncloseInRoundBrackets.UNKNOWN) { - return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES && e.getTypeCasts().isEmpty(); + return requiresBrackets == RoundBracketAnalyzer.EncloseInRoundBrackets.YES || !e.getTypeCasts().isEmpty(); } if (e.isParentInitialized() && e.getParent() instanceof CtTargetedExpression && ((CtTargetedExpression) e.getParent()).getTarget() == e) { return e instanceof CtVariableRead && !e.getTypeCasts().isEmpty(); diff --git a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java index 6546fa2e7cf..84ff94fe4ba 100644 --- a/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java +++ b/src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java @@ -67,7 +67,9 @@ public class DefaultJavaPrettyPrinterTest { "1 | 2 & 3", "(1 | 2) & 3", "1 | 2 ^ 3", - "(1 | 2) ^ 3" + "(1 | 2) ^ 3", + "((int) (1 + 2)) * 3", + "(int) (int) (1 + 1)", }) public void testParenOptimizationCorrectlyPrintsParenthesesForExpressions(String rawExpression) { // contract: When input expressions are minimally parenthesized, pretty-printed output diff --git a/src/test/java/spoon/test/eval/EvalTest.java b/src/test/java/spoon/test/eval/EvalTest.java index 8a565bde28f..c6b55b00952 100644 --- a/src/test/java/spoon/test/eval/EvalTest.java +++ b/src/test/java/spoon/test/eval/EvalTest.java @@ -96,7 +96,7 @@ public void testDoNotSimplifyCasts() throws Exception { CtBlock b = type.getMethodsByName("testDoNotSimplifyCasts").get(0).getBody(); assertEquals(1, b.getStatements().size()); b = b.partiallyEvaluate(); - assertEquals("return ((U) ((java.lang.Object) (spoon.test.eval.testclasses.ToEvaluate.castTarget(element).getClass())))", b.getStatements().get(0).toString()); + assertEquals("return ((U) (java.lang.Object) (spoon.test.eval.testclasses.ToEvaluate.castTarget(element).getClass()))", b.getStatements().get(0).toString()); } @Test