diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 7867f9f6c8d..b901cb71b56 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -360,9 +360,9 @@ private boolean shouldSetBracket(CtExpression e) { } try { if ((e.getParent() instanceof CtBinaryOperator) || (e.getParent() instanceof CtUnaryOperator)) { - return (e instanceof CtTargetedExpression) || (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || e instanceof CtBinaryOperator; + return (e instanceof CtAssignment) || (e instanceof CtConditional) || (e instanceof CtUnaryOperator) || e instanceof CtBinaryOperator; } - if (e.getParent() instanceof CtTargetedExpression) { + 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) { diff --git a/src/test/java/spoon/test/comment/CommentTest.java b/src/test/java/spoon/test/comment/CommentTest.java index 925156376ca..8cb6ba5ead3 100644 --- a/src/test/java/spoon/test/comment/CommentTest.java +++ b/src/test/java/spoon/test/comment/CommentTest.java @@ -371,7 +371,7 @@ public void testInLineComment() { assertEquals("// comment if" + newLine + "if ((1 % 2) == 0) {" + newLine + " // comment unary operator" + newLine - + " (field)++;" + newLine + + " field++;" + newLine + "}", ctIf.toString()); CtConstructorCall ctConstructorCall = m1.getBody().getStatement(3); @@ -433,7 +433,7 @@ public void testInLineComment() { + ") ? // comment before then CtConditional" + newLine + "null// comment after then CtConditional" + newLine + " : // comment before else CtConditional" + newLine - + "new java.lang.Double((j / ((double) (i - 1))))// comment after else CtConditional" + newLine, ctLocalVariable1.toString()); + + "new java.lang.Double(j / ((double) (i - 1)))// comment after else CtConditional" + newLine, ctLocalVariable1.toString()); CtNewArray ctNewArray = (CtNewArray) ((CtLocalVariable) m1.getBody().getStatement(11)).getDefaultExpression(); assertEquals(createFakeComment(f, "last comment at the end of array"), ctNewArray.getComments().get(0)); @@ -567,7 +567,7 @@ public void testBlockComment() { assertEquals("/* comment if */" + newLine + "if ((1 % 2) == 0) {" + newLine + " /* comment unary operator */" + newLine - + " (field)++;" + newLine + + " field++;" + newLine + "}", ctIf.toString()); CtConstructorCall ctConstructorCall = m1.getBody().getStatement(3); diff --git a/src/test/java/spoon/test/eval/EvalTest.java b/src/test/java/spoon/test/eval/EvalTest.java index 423a6c23cf3..396416adeb5 100644 --- a/src/test/java/spoon/test/eval/EvalTest.java +++ b/src/test/java/spoon/test/eval/EvalTest.java @@ -81,7 +81,7 @@ public void testDoNotSimplify() throws Exception { CtBlock b = type.getMethodsByName("testDoNotSimplify").get(0).getBody(); assertEquals(1, b.getStatements().size()); b = b.partiallyEvaluate(); - assertEquals("java.lang.System.out.println((((\"enter: \" + className) + \" - \") + methodName))", b.getStatements().get(0).toString()); + assertEquals("java.lang.System.out.println(((\"enter: \" + className) + \" - \") + methodName)", b.getStatements().get(0).toString()); } @Test diff --git a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java index d4c3e86768f..414f2c21425 100644 --- a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java +++ b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java @@ -256,7 +256,7 @@ public void testFieldAccessNoClasspath() { CtFieldAccess ctFieldAccess = ctType .getElements(new TypeFilter<>(CtFieldAccess.class)).get(0); - assertEquals("(game.board.width)", ctFieldAccess.toString()); + assertEquals("game.board.width", ctFieldAccess.toString()); CtFieldReference ctFieldReferenceWith = ctFieldAccess.getVariable(); assertEquals("width", ctFieldReferenceWith.getSimpleName()); @@ -290,12 +290,12 @@ public void testIncrementationOnAVarIsAUnaryOperator() throws Exception { final CtUnaryOperator first = unaryOperators.get(0); assertEquals(UnaryOperatorKind.POSTINC, first.getKind()); assertEquals(fieldRead, first.getOperand()); - assertEquals("(i)++", first.toString()); + assertEquals("i++", first.toString()); final CtUnaryOperator second = unaryOperators.get(1); assertEquals(UnaryOperatorKind.PREINC, second.getKind()); assertEquals(fieldRead, second.getOperand()); - assertEquals("++(i)", second.toString()); + assertEquals("++i", second.toString()); } @Test @@ -452,4 +452,15 @@ public void testFieldAccessAutoExplicit() throws Exception { //now the field access must use explicit "this." assertEquals("this.age", ageFR.getParent().toString()); } + + @Test + public void testFieldAccessWithParenthesis() { + // contract: there should not be any redundant parentheses around fields + // https://github.com/INRIA/spoon/pull/3021 + CtClass c1 = Launcher.parseClass("class C1 { int count ; void m() { for(int i=0;i(CtFieldAccess.class)).get(0).toString()); + + CtClass c2 = Launcher.parseClass("class C1 { int count ; void m() { for(int i=0;i<(long)count;i++){}}}"); + assertEquals("((long) (count))", c2.getElements(new TypeFilter<>(CtFieldAccess.class)).get(0).toString()); + } } diff --git a/src/test/java/spoon/test/lambda/LambdaTest.java b/src/test/java/spoon/test/lambda/LambdaTest.java index e6c682cd280..81a0299f36f 100644 --- a/src/test/java/spoon/test/lambda/LambdaTest.java +++ b/src/test/java/spoon/test/lambda/LambdaTest.java @@ -175,7 +175,7 @@ public void testLambdaExpressionWithExpressionBodyAndWithoutTypeForParameter() { assertHasExpressionBody(lambda); assertIsWellPrinted( - "((java.util.function.Predicate) (( p) -> (p.age) > 10))", + "((java.util.function.Predicate) (( p) -> p.age > 10))", lambda); } @@ -194,7 +194,7 @@ public void testLambdaExpressionWithExpressionBodyAndWithMultiParameters() { assertHasExpressionBody(lambda); assertIsWellPrinted( - "((spoon.test.lambda.testclasses.Foo.CheckPersons) (( p1, p2) -> ((p1.age) - (p2.age)) > 0))", + "((spoon.test.lambda.testclasses.Foo.CheckPersons) (( p1, p2) -> (p1.age - p2.age) > 0))", lambda); } @@ -210,7 +210,7 @@ public void testLambdaExpressionWithExpressionBodyAndWithParameterTyped() { assertHasExpressionBody(lambda); assertIsWellPrinted( - "((java.util.function.Predicate) ((spoon.test.lambda.testclasses.Foo.Person p) -> (p.age) > 10))", + "((java.util.function.Predicate) ((spoon.test.lambda.testclasses.Foo.Person p) -> p.age > 10))", lambda); } @@ -229,7 +229,7 @@ public void testLambdaExpressionWithExpressionBodyAndWithMultiParametersTyped() assertHasExpressionBody(lambda); assertIsWellPrinted( - "((spoon.test.lambda.testclasses.Foo.CheckPersons) ((spoon.test.lambda.testclasses.Foo.Person p1,spoon.test.lambda.testclasses.Foo.Person p2) -> ((p1.age) - (p2.age)) > 0))", + "((spoon.test.lambda.testclasses.Foo.CheckPersons) ((spoon.test.lambda.testclasses.Foo.Person p1,spoon.test.lambda.testclasses.Foo.Person p2) -> (p1.age - p2.age) > 0))", lambda); } @@ -262,7 +262,7 @@ public void testLambdaExpressionWithStatementBodyAndWithParameter() { "((java.util.function.Predicate) (( p) -> {" + System.lineSeparator() + " p.doSomething();" + System.lineSeparator() - + " return (p.age) > 10;" + System.lineSeparator() + + " return p.age > 10;" + System.lineSeparator() + "}))", lambda); } @@ -285,7 +285,7 @@ public boolean matches(CtIf element) { } }).get(0); final String expected = - "if (((java.util.function.Predicate) (( p) -> (p.age) > 18)).test(new spoon.test.lambda.testclasses.Foo.Person(10))) {" + "if (((java.util.function.Predicate) (( p) -> p.age > 18)).test(new spoon.test.lambda.testclasses.Foo.Person(10))) {" + System.lineSeparator() + " java.lang.System.err.println(\"Enjoy, you have more than 18.\");" + System .lineSeparator() diff --git a/src/test/java/spoon/test/prettyprinter/PrinterTest.java b/src/test/java/spoon/test/prettyprinter/PrinterTest.java index cdb0c8820b1..97cbba19e92 100644 --- a/src/test/java/spoon/test/prettyprinter/PrinterTest.java +++ b/src/test/java/spoon/test/prettyprinter/PrinterTest.java @@ -25,7 +25,10 @@ import spoon.Launcher; import spoon.compiler.SpoonResourceHelper; +import spoon.reflect.code.CtBinaryOperator; import spoon.reflect.code.CtComment; +import spoon.reflect.code.CtInvocation; +import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtType; import spoon.reflect.factory.Factory; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; @@ -34,6 +37,7 @@ import spoon.reflect.visitor.PrinterHelper; import spoon.reflect.visitor.TokenWriter; import spoon.reflect.visitor.DefaultTokenWriter; +import spoon.reflect.visitor.filter.TypeFilter; import spoon.test.prettyprinter.testclasses.MissingVariableDeclaration; import spoon.testing.utils.ModelUtils; @@ -554,4 +558,18 @@ public void testListPrinter() { String expectedResult = " start un next deux next trois end"; assertEquals(expectedResult, pp.toString()); } + + @Test + public void testMethodParentheses() { + //contract: there should not be any redundant parentheses + //https://github.com/INRIA/spoon/issues/2330 + CtClass c1 = Launcher.parseClass("class C1 { int count ; void m() { logger.info(\"Value declared in if:\" + c); }"); + assertEquals("\"Value declared in if:\" + c", c1.getElements(new TypeFilter<>(CtBinaryOperator.class)).get(0).toString()); + + CtClass c2 = Launcher.parseClass("class C2 { int count ; void m() { (i++).toString(); (a+b).toString(); }"); + List invocations = c2.getElements(new TypeFilter<>(CtInvocation.class)); + assertEquals("super()", invocations.get(0).toString()); + assertEquals("(i++).toString()", invocations.get(1).toString()); + assertEquals("(a + b).toString()", invocations.get(2).toString()); + } } diff --git a/src/test/java/spoon/test/query_function/VariableReferencesTest.java b/src/test/java/spoon/test/query_function/VariableReferencesTest.java index 10adca1ade8..459a7f63037 100644 --- a/src/test/java/spoon/test/query_function/VariableReferencesTest.java +++ b/src/test/java/spoon/test/query_function/VariableReferencesTest.java @@ -359,7 +359,7 @@ public void testPotentialVariableAccessFromStaticMethod() throws Exception { CtClass clazz = factory.Class().get(VariableReferencesFromStaticMethod.class); CtMethod staticMethod = clazz.getMethodsByName("staticMethod").get(0); CtStatement stmt = staticMethod.getBody().getStatements().get(1); - assertEquals("org.junit.Assert.assertTrue((field == 2))", stmt.toString()); + assertEquals("org.junit.Assert.assertTrue(field == 2)", stmt.toString()); CtLocalVariableReference varRef = stmt.filterChildren(new TypeFilter<>(CtLocalVariableReference.class)).first(); List vars = varRef.map(new PotentialVariableDeclarationFunction()).list(); assertEquals("Found unexpected variable declaration.", 1, vars.size()); diff --git a/src/test/java/spoon/test/strings/StringTest.java b/src/test/java/spoon/test/strings/StringTest.java index 6f403c3ce29..147f6c872bf 100644 --- a/src/test/java/spoon/test/strings/StringTest.java +++ b/src/test/java/spoon/test/strings/StringTest.java @@ -37,7 +37,7 @@ public void testModelBuildingInitializer() throws Exception { CtBinaryOperator op = type.getElements( new TypeFilter>(CtBinaryOperator.class)) .get(0); - assertEquals("(\"a\" + \"b\")", op.toString()); + assertEquals("\"a\" + \"b\"", op.toString()); assertEquals(BinaryOperatorKind.PLUS, op.getKind()); assertTrue(op.getLeftHandOperand() instanceof CtLiteral); assertTrue(op.getRightHandOperand() instanceof CtLiteral); diff --git a/src/test/java/spoon/test/template/TemplateReplaceReturnTest.java b/src/test/java/spoon/test/template/TemplateReplaceReturnTest.java index a749adc870b..8cfac018cd8 100644 --- a/src/test/java/spoon/test/template/TemplateReplaceReturnTest.java +++ b/src/test/java/spoon/test/template/TemplateReplaceReturnTest.java @@ -48,7 +48,7 @@ public void testReturnReplaceTemplate() { CtClass resultKlass = factory.Class().create(factory.Package().getOrCreate("spoon.test.template"), "ReturnReplaceResult"); new ReturnReplaceTemplate(model).apply(resultKlass); - assertEquals("{ if (((java.lang.System.currentTimeMillis()) % 2L) == 0) { return \"Panna\"; } else { return \"Orel\"; }}", getOptimizedString(resultKlass.getMethod("method").getBody())); + assertEquals("{ if ((java.lang.System.currentTimeMillis() % 2L) == 0) { return \"Panna\"; } else { return \"Orel\"; }}", getOptimizedString(resultKlass.getMethod("method").getBody())); launcher.setSourceOutputDirectory(new File("./target/spooned/")); launcher.getModelBuilder().generateProcessedSourceFiles(OutputType.CLASSES); ModelUtils.canBeBuilt(new File("./target/spooned/spoon/test/template/ReturnReplaceResult.java"), 8); diff --git a/src/test/java/spoon/test/template/TemplateTest.java b/src/test/java/spoon/test/template/TemplateTest.java index 60e63dbcecd..8e10de820f5 100644 --- a/src/test/java/spoon/test/template/TemplateTest.java +++ b/src/test/java/spoon/test/template/TemplateTest.java @@ -431,7 +431,7 @@ public void testCheckBoundTemplate() throws Exception { CtIf ifStmt = (CtIf) injectedCode; // contains the replaced code - assertEquals("(l.size()) > 10", ifStmt.getCondition().toString()); + assertEquals("l.size() > 10", ifStmt.getCondition().toString()); // adds the bound check at the beginning of a method method.getBody().insertBegin(injectedCode); @@ -484,8 +484,8 @@ public void testTemplateMatcher() throws Exception { assertEquals(2, matcher.find(klass).size()); assertThat(asList("foo","fbar"), is(klass.filterChildren(matcher).map((CtElement e)->getMethodName(e)).list())) ; matcher.forEachMatch(klass, (match) -> { - assertTrue(checkParameters("foo", match, "_x_", "(new java.util.ArrayList<>().size())") - ||checkParameters("fbar", match, "_x_", "(l.size())") + assertTrue(checkParameters("foo", match, "_x_", "new java.util.ArrayList<>().size()") + ||checkParameters("fbar", match, "_x_", "l.size()") ); }); } @@ -500,13 +500,13 @@ public void testTemplateMatcher() throws Exception { matcher.forEachMatch(klass, (match) -> { assertTrue( checkParameters("foo", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10") ||checkParameters("foo2", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "11") ||checkParameters("fbar", match, - "_x_", "(l.size())", + "_x_", "l.size()", "_y_", "10") ); }); @@ -522,27 +522,27 @@ public void testTemplateMatcher() throws Exception { matcher.forEachMatch(klass, (match) -> { assertTrue( checkParameters("foo", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10", "_block_", "throw new java.lang.IndexOutOfBoundsException();") ||checkParameters("foo2", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "11", "_block_", "throw new java.lang.IndexOutOfBoundsException();") ||checkParameters("fbar", match, - "_x_", "(l.size())", + "_x_", "l.size()", "_y_", "10", "_block_", "throw new java.lang.IndexOutOfBoundsException();") ||checkParameters("baz", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10", "_block_", "{}") ||checkParameters("bou", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10", "_block_", "{ java.lang.System.out.println();}") ||checkParameters("bov", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10", "_block_", "java.lang.System.out.println();") ); @@ -559,27 +559,27 @@ public void testTemplateMatcher() throws Exception { matcher.forEachMatch(klass, (match) -> { assertTrue( checkParameters("foo", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10", "_stmt_", "throw new java.lang.IndexOutOfBoundsException()") ||checkParameters("foo2", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "11", "_stmt_", "throw new java.lang.IndexOutOfBoundsException()") ||checkParameters("fbar", match, - "_x_", "(l.size())", + "_x_", "l.size()", "_y_", "10", "_stmt_", "throw new java.lang.IndexOutOfBoundsException()") ||checkParameters("baz", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10", "_stmt_", "null") ||checkParameters("bou", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10", "_stmt_", "java.lang.System.out.println()") ||checkParameters("bov", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_y_", "10", "_stmt_", "java.lang.System.out.println()") ); @@ -596,7 +596,7 @@ public void testTemplateMatcher() throws Exception { matcher.forEachMatch(klass, (match) -> { assertTrue( checkParameters("bos", match, - "_x_", "(new java.util.ArrayList<>().size())", + "_x_", "new java.util.ArrayList<>().size()", "_block_", "java.lang.System.out.println();") ); }); @@ -678,7 +678,7 @@ public void testExtensionBlock() { assertTrue(aTry.getBody().getStatement(0) instanceof CtInvocation); assertEquals("spoon.test.template.testclasses.logger.Logger.enter(\"Logger\", \"enter\")", aTry.getBody().getStatement(0).toString()); assertTrue(aTry.getBody().getStatements().size() > 1); - assertEquals("java.lang.System.out.println((((\"enter: \" + className) + \" - \") + methodName))", aTry.getBody().getStatement(1).toString()); + assertEquals("java.lang.System.out.println(((\"enter: \" + className) + \" - \") + methodName)", aTry.getBody().getStatement(1).toString()); } @Test @@ -1142,7 +1142,7 @@ public void testAnotherFieldAccessNameSubstitution() { assertEquals("int x;", result.getField("x").toString()); assertEquals("int m_x;", result.getField("m_x").toString()); - assertEquals("java.lang.System.out.println(((x) + (m_x)))", result.getAnonymousExecutables().get(0).getBody().getStatement(0).toString()); + assertEquals("java.lang.System.out.println(x + m_x)", result.getAnonymousExecutables().get(0).getBody().getStatement(0).toString()); } } @@ -1171,7 +1171,7 @@ public void substituteTypeAccessReference() { assertEquals("spoon.test.template.TypeReferenceClassAccess.Example ret = new spoon.test.template.TypeReferenceClassAccess.Example()", method.getBody().getStatement(1).toString()); assertEquals("o = spoon.test.template.TypeReferenceClassAccess.Example.currentTimeMillis()", method.getBody().getStatement(2).toString()); assertEquals("o = spoon.test.template.TypeReferenceClassAccess.Example.class", method.getBody().getStatement(3).toString()); - assertEquals("o = (o) instanceof spoon.test.template.TypeReferenceClassAccess.Example", method.getBody().getStatement(4).toString()); + assertEquals("o = o instanceof spoon.test.template.TypeReferenceClassAccess.Example", method.getBody().getStatement(4).toString()); assertEquals("java.util.function.Supplier p = spoon.test.template.TypeReferenceClassAccess.Example::currentTimeMillis", method.getBody().getStatement(5).toString()); } diff --git a/src/test/java/spoon/test/visibility/VisibilityTest.java b/src/test/java/spoon/test/visibility/VisibilityTest.java index 24550308f51..910828839c3 100644 --- a/src/test/java/spoon/test/visibility/VisibilityTest.java +++ b/src/test/java/spoon/test/visibility/VisibilityTest.java @@ -102,7 +102,7 @@ public void testFullyQualifiedNameOfTypeReferenceWithGeneric() { CtType nestedB = aClass.getNestedType("B"); List elements = nestedB.getElements(new TypeFilter<>(CtFieldAccess.class)); assertEquals(1, elements.size()); - assertEquals("(spoon.test.visibility.testclasses.A.B.i)", elements.get(0).toString()); + assertEquals("spoon.test.visibility.testclasses.A.B.i", elements.get(0).toString()); CtMethod instanceOf = aClass.getMethodsByName("instanceOf").get(0); List elements1 = instanceOf.getElements(new TypeFilter<>(CtBinaryOperator.class)); @@ -116,7 +116,7 @@ public void testFullyQualifiedNameOfTypeReferenceWithGeneric() { nestedB = secondClass.getNestedType("B"); elements = nestedB.getElements(new TypeFilter<>(CtFieldAccess.class)); assertEquals(1, elements.size()); - assertEquals("(spoon.test.visibility.testclasses.A2.B.i)", elements.get(0).toString()); + assertEquals("spoon.test.visibility.testclasses.A2.B.i", elements.get(0).toString()); instanceOf = secondClass.getMethodsByName("instanceOf").get(0); elements1 = instanceOf.getElements(new TypeFilter<>(CtBinaryOperator.class));