fix(ThisAccess): the type access associated to a this must be the typ…

…e containing the this (#1008)

fixes #1006.

Note that #1006 was neither in clone, nor in pretty-printer, but in the model.

This is the problem when implicit=true, the model can be incorrect and one does not see it.

src/main/java/spoon/reflect/factory/CodeFactory.java

@@ -107,7 +107,13 @@ public <T> CtBinaryOperator<T> createBinaryOperator(CtExpression<?> left, CtExpr
 	 * @return a accessed type expression.
 	 */
 	public <T> CtTypeAccess<T> createTypeAccess(CtTypeReference<T> accessedType) {
-		return createTypeAccessWithoutCloningReference(accessedType == null ? null : accessedType.clone());
+		if (accessedType == null) {
+			return factory.Core().createTypeAccess();
+		}
+		CtTypeReference<T> access = accessedType.clone();
+		// a type access doesn't contain actual type parameters
+		access.setActualTypeArguments(null);
+		return createTypeAccessWithoutCloningReference(access);
 	}
 
 	/**
@@ -365,7 +371,7 @@ public <R> CtStatementList createStatementList(CtBlock<R> block) {
 	}
 
 	/**
-	 * Creates an access to a <code>this</code> variable (of the form
+	 * Creates an explicit access to a <code>this</code> variable (of the form
 	 * <code>type.this</code>).
 	 *
 	 * @param <T>
@@ -376,12 +382,7 @@ public <R> CtStatementList createStatementList(CtBlock<R> block) {
 	 * @return a <code>type.this</code> expression
 	 */
 	public <T> CtThisAccess<T> createThisAccess(CtTypeReference<T> type) {
-		CtThisAccess<T> thisAccess = factory.Core().<T>createThisAccess();
-		thisAccess.setType(type);
-		CtTypeAccess<T> typeAccess = factory.Code().createTypeAccess(type);
-		typeAccess.setImplicit(true);
-		thisAccess.setTarget(typeAccess);
-		return thisAccess;
+		return createThisAccess(type, false);
 	}
 
 	/**
@@ -402,7 +403,6 @@ public <T> CtThisAccess<T> createThisAccess(CtTypeReference<T> type, boolean isI
 		thisAccess.setImplicit(isImplicit);
 		thisAccess.setType(type);
 		CtTypeAccess<T> typeAccess = factory.Code().createTypeAccess(type);
-		typeAccess.setImplicit(isImplicit);
 		thisAccess.setTarget(typeAccess);
 		return thisAccess;
 	}

src/main/java/spoon/support/compiler/jdt/ParentExiter.java

@@ -610,9 +610,6 @@ public <T> void visitCtInvocation(CtInvocation<T> invocation) {
 					final CtTypeReference<?> declaringType = invocation.getExecutable().getDeclaringType();
 					if (declaringType != null && invocation.getExecutable().isStatic() && child.isImplicit()) {
 						invocation.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(declaringType, declaringType.isAnonymous()));
-					} else if (declaringType != null && !invocation.getExecutable().isStatic() && child.isImplicit()) {
-						((CtThisAccess) child).setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(declaringType, true));
-						invocation.setTarget((CtThisAccess<?>) child);
 					} else {
 						invocation.setTarget((CtThisAccess<?>) child);
 					}

src/test/java/spoon/test/delete/DeleteTest.java

@@ -150,11 +150,12 @@ public void testDeleteMethod() throws Exception {
 
 		final CtMethod method = adobada.getMethod("m4", factory.Type().INTEGER_PRIMITIVE, factory.Type().FLOAT_PRIMITIVE, factory.Type().STRING);
 
-		assertEquals(4, adobada.getMethods().size());
+		int n = adobada.getMethods().size();
 
+		// deleting m4
 		method.delete();
 
-		assertEquals(3, adobada.getMethods().size());
+		assertEquals(n - 1, adobada.getMethods().size());
 		assertFalse(adobada.getMethods().contains(method));
 	}
 

src/test/java/spoon/test/delete/testclasses/Adobada.java

@@ -50,4 +50,8 @@ public void m4(int i, float j, String s) {
 		int k;
 		j = i = k = 3;
 	}
+
+	public void methodUsingjlObjectMethods() {
+		notify();
+	}
 }

src/test/java/spoon/test/prettyprinter/QualifiedThisRefTest.java

@@ -5,16 +5,28 @@
 import org.junit.Test;
 import spoon.Launcher;
 import spoon.compiler.SpoonResourceHelper;
+import spoon.reflect.code.CtInvocation;
+import spoon.reflect.code.CtStatement;
+import spoon.reflect.code.CtThisAccess;
+import spoon.reflect.code.CtTypeAccess;
+import spoon.reflect.declaration.CtClass;
+import spoon.reflect.declaration.CtMethod;
 import spoon.reflect.declaration.CtType;
 import spoon.reflect.factory.Factory;
 import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.DefaultJavaPrettyPrinter;
+import spoon.reflect.visitor.filter.TypeFilter;
+import spoon.test.delete.testclasses.Adobada;
 import spoon.test.prettyprinter.testclasses.QualifiedThisRef;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static spoon.testing.utils.ModelUtils.build;
+
 public class QualifiedThisRefTest {
 
 	Factory factory;
@@ -44,4 +56,28 @@ public void testQualifiedThisRef() {
 		printer.scan(ctClass);
 		Assert.assertTrue(printer.getResult().contains("Object o = QualifiedThisRef.Sub.this"));
 	}
+
+	@Test
+	public void testCloneThisAccess() throws Exception {
+		// contract: the target of "this" is correct and can be cloned
+		final Factory factory = build(Adobada.class);
+		final CtClass<Adobada> adobada = factory.Class().get(Adobada.class);
+		final CtMethod<?> m2 = adobada.getMethod("methodUsingjlObjectMethods");
+
+		CtThisAccess th = (CtThisAccess) m2.getElements(new TypeFilter(CtThisAccess.class)).get(0);
+		assertEquals(true,th.isImplicit());
+		assertEquals("notify()",th.getParent().toString());
+		assertEquals("spoon.test.delete.testclasses.Adobada", ((CtTypeAccess)th.getTarget()).getAccessedType().toString());
+		CtMethod<?> clone = m2.clone();
+		CtInvocation<?> stat = clone.getBody().getStatement(0);
+		assertNotEquals("java.lang.Object.this.notify()", stat.toString()); // the original bug
+		assertEquals("spoon.test.delete.testclasses.Adobada.this.notify()", stat.toString());
+		stat.getTarget().setImplicit(true);
+		assertEquals("notify()", stat.toString());
+
+		// note that this behavior means that you can only keep cloned "this" in the same class,
+		// and you cannot "transplant" a cloned "this" to another class
+		// it makes perfectly sense about the meaning of this.
+		// to "transplant" a this, you have to first set the target to null
+	}
 }

src/test/java/spoon/test/targeted/TargetedExpressionTest.java

@@ -103,7 +103,7 @@ public void testNotTargetedExpression() throws Exception {
 		CtFieldAccess<?> fieldAccess = factory.Core().createFieldRead();
 		fieldAccess.setVariable((CtFieldReference) iField.getReference());
 		fieldAccess.setTarget(factory.Code().createThisAccess(fooClass.getReference()));
-		assertEquals("this.i", fieldAccess.toString());
+		assertEquals("spoon.test.targeted.testclasses.Foo.this.i", fieldAccess.toString());
 		// this test is made for this line. Check that we can setTarget(null)
 		// without NPE
 		fieldAccess.setTarget(null);
@@ -293,7 +293,7 @@ public void testTargetsOfInv() throws Exception {
 
 		assertEquals(fooTypeAccess.getType().getQualifiedName(), ((CtThisAccess) elements.get(2).getTarget()).getTarget().getType().getQualifiedName());
 		assertEquals(fooTypeAccess.getType().getQualifiedName(), ((CtThisAccess) elements.get(3).getTarget()).getTarget().getType().getQualifiedName());
-		assertEquals(superClassTypeAccess, ((CtThisAccess) elements.get(6).getTarget()).getTarget());
+		assertEquals(fooTypeAccess, ((CtThisAccess) elements.get(6).getTarget()).getTarget());
 	}
 
 	@Test

src/test/java/spoon/test/targeted/testclasses/Foo.java

@@ -18,7 +18,8 @@ public class Foo<T> extends SuperClass {
 
 	public void m() {
 		int x;
-		x= this.k;
+		// checking that this is correct Java and is correctly parsed
+		x= spoon.test.targeted.testclasses.Foo.this.k;
 		x= Foo.k;
 		x= k;
 		this.k = x;