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

…e containing the this

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;