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.

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 printing parenthesis for type casts in expression #4995

Merged
merged 17 commits into from
Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
28 changes: 15 additions & 13 deletions src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
Expand Down Expand Up @@ -400,25 +402,25 @@ public DefaultJavaPrettyPrinter scan(CtElement e) {
}

private boolean shouldSetBracket(CtExpression<?> e) {
if (!e.getTypeCasts().isEmpty()) {
if (isMinimizeRoundBrackets()) {
RoundBracketAnalyzer.EncloseInRoundBrackets requiresBrackets =
RoundBracketAnalyzer.requiresRoundBrackets(e);
if (requiresBrackets != RoundBracketAnalyzer.EncloseInRoundBrackets.UNKNOWN) {
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();
}
} 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);
}
} catch (ParentNotInitializedException ex) {
// nothing we accept not to have a parent
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,11 +29,13 @@
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;
import spoon.testing.utils.ModelTest;

import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
Expand Down Expand Up @@ -78,7 +82,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.0",
"int myInt = (int) (float) 0.0",
})
public void testParenOptimizationCorrectlyPrintsParenthesesForStatements(String rawStatement) {
// contract: When input expressions as part of statements are minimally parenthesized,
Expand Down Expand Up @@ -322,4 +328,50 @@ 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() 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<Integer> localVariable = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(0);
assertThat(localVariable.toString(), equalTo("int myInt = (int) myDouble"));

CtLocalVariable<Integer> localVariable2 = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(1);
assertThat(localVariable2.toString(), equalTo("int myInt2 = ((java.lang.Double) myDouble).intValue()"));

CtLocalVariable<Integer> 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<Integer> localVariable = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(1);
assertThat(localVariable.toString(), equalTo("int fieldReadOfA = ((A) c).a.i"));
}
}
14 changes: 14 additions & 0 deletions src/test/resources/printer-test/ShadowFieldRead.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
10 changes: 10 additions & 0 deletions src/test/resources/printer-test/TypeCastOnFieldRead.java
Original file line number Diff line number Diff line change
@@ -0,0 +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();
}
}