fix: Remove parentheses around targeted expr in binary or unary op (#…

…3024)

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) {

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);

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

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<count;i++){}}}");
+		assertEquals("count", c1.getElements(new TypeFilter<>(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());
+	}
 }

src/test/java/spoon/test/lambda/LambdaTest.java

@@ -175,7 +175,7 @@ public void testLambdaExpressionWithExpressionBodyAndWithoutTypeForParameter() {
 		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);
 	}
 
@@ -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>) ((spoon.test.lambda.testclasses.Foo.Person p) -> (p.age) > 10))",
+				"((java.util.function.Predicate<spoon.test.lambda.testclasses.Foo.Person>) ((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<spoon.test.lambda.testclasses.Foo.Person>) (( 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<spoon.test.lambda.testclasses.Foo.Person>) (( p) -> (p.age) > 18)).test(new spoon.test.lambda.testclasses.Foo.Person(10))) {"
+				"if (((java.util.function.Predicate<spoon.test.lambda.testclasses.Foo.Person>) (( 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()

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<CtInvocation> 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());
+		}
 }

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<CtVariable> vars = varRef.map(new PotentialVariableDeclarationFunction()).list();
 		assertEquals("Found unexpected variable declaration.", 1, vars.size());

src/test/java/spoon/test/strings/StringTest.java

@@ -37,7 +37,7 @@ public void testModelBuildingInitializer() throws Exception {
 		CtBinaryOperator<?> op = type.getElements(
 				new TypeFilter<CtBinaryOperator<?>>(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);

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);

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<java.util.Date> ret = new spoon.test.template.TypeReferenceClassAccess.Example<java.util.Date>()", 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<java.lang.Long> p = spoon.test.template.TypeReferenceClassAccess.Example::currentTimeMillis", method.getBody().getStatement(5).toString());
 	}
 

src/test/java/spoon/test/visibility/VisibilityTest.java

@@ -102,7 +102,7 @@ public void testFullyQualifiedNameOfTypeReferenceWithGeneric() {
 		CtType<?> nestedB = aClass.getNestedType("B");
 		List<CtFieldAccess> 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<CtBinaryOperator> 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));