review: Fix issue with isOverriding #1407

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

@@ -632,6 +632,7 @@ <T> CtTypeReference<T> getTypeReference(TypeBinding binding) {
 				ref = getTypeReference(binding.actualType());
 			} else {
 				ref = this.jdtTreeBuilder.getFactory().Core().createTypeReference();
+				this.exploringParameterizedBindings.put(binding, ref);
 				if (binding.isAnonymousType()) {
 					ref.setSimpleName("");
 				} else {
@@ -702,7 +703,13 @@ <T> CtTypeReference<T> getTypeReference(TypeBinding binding) {
 				// if the type parameter has a super class other than java.lang.Object, we get it
 				// superClass.superclass() is null if it's java.lang.Object
 				if (superClass != null && !(superClass.superclass() == null)) {
-					refSuperClass = this.getTypeReference(superClass);
+
+					// this case could happen with Enum<E extends Enum<E>> for example:
+					// in that case we only want to have E -> Enum -> E
+					// to conserve the same behavior as JavaReflectionTreeBuilder
+					if (!(superClass instanceof ParameterizedTypeBinding) || !this.exploringParameterizedBindings.containsKey(superClass)) {
+						refSuperClass = this.getTypeReference(superClass);
+					}
 
 				// if the type parameter has a super interface, then we'll get it too, as a superclass
 				// type parameter can only extends an interface or a class, so we don't make the distinction

src/main/java/spoon/support/reflect/reference/CtExecutableReferenceImpl.java

@@ -145,7 +145,10 @@ public boolean isConstructor() {
 	@SuppressWarnings("unchecked")
 	public CtExecutable<T> getDeclaration() {
 		final CtTypeReference<?> typeRef = getDeclaringType();
-		return typeRef == null ? null : getCtExecutable(typeRef.getDeclaration());
+		if (typeRef == null || typeRef.getDeclaration() == null) {
+			return null;
+		}
+		return getCtExecutable(typeRef.getDeclaration());
 	}
 
 	@Override

src/main/java/spoon/support/visitor/AbstractTypingContext.java

@@ -71,9 +71,6 @@ private CtTypeReference<?> adaptTypeParameterReference(CtTypeParameterReference
 			return adaptTypeParameterReferenceBoundingType(typeParamRef, typeParamRef.getBoundingType());
 		}
 		CtTypeReference<?> typeRefAdapted = adaptTypeParameter(typeParamRef.getDeclaration());
-		if (typeRefAdapted instanceof CtTypeParameterReference) {
-			return adaptTypeParameterReferenceBoundingType((CtTypeParameterReference) typeRefAdapted, typeParamRef.getBoundingType());
-		}
 		return typeRefAdapted;
 	}
 

src/main/java/spoon/support/visitor/java/JavaReflectionTreeBuilder.java

@@ -329,7 +329,12 @@ public <T extends GenericDeclaration> void visitTypeParameterReference(TypeVaria
 
 	@Override
 	public void visitType(Type type) {
-		final CtTypeReference<?> ctTypeReference = factory.Core().createTypeReference();
+		CtTypeReference<?> ctTypeReference;
+		if (type instanceof TypeVariable) {
+			ctTypeReference = factory.Core().createTypeParameterReference();
+		} else {
+			ctTypeReference = factory.Core().createTypeReference();
+		}
 		enter(new TypeReferenceRuntimeBuilderContext(ctTypeReference));
 		ctTypeReference.setSimpleName(getTypeName(type));
 

src/test/java/spoon/reflect/declaration/CtTypeInformationTest.java

@@ -137,18 +137,13 @@ public void testGetSuperclass() throws Exception {
 
 		// + 48 of ArrayList (in library)
 		// + 12 of java.lang.Object
-		// The final +1 is the result of the only usage of `ClassTypingContext#isSameSignature` in `getAllMethods`
-		// (see: https://github.com/INRIA/spoon/pull/1375)
-		// now it gets both `ArrayList#forEach` and `Iterable#forEach`
-		// this has been spotted as an issue in https://github.com/INRIA/spoon/issues/1407
-		Assert.assertEquals(1+12+48+1, extendObject.getAllMethods().size());
+		Assert.assertEquals(1+12+48, extendObject.getAllMethods().size());
 
 		final CtType<?> subClass = this.factory.Type().get(Subclass.class);
 		assertEquals(2, subClass.getMethods().size());
 
 		// the abstract method from Comparable which is overridden should not be present in the model
-		// The +1 happens for the same reason as below
-		assertEquals(61+2+1, subClass.getAllMethods().size());
+		assertEquals(61+2, subClass.getAllMethods().size());
 
 		CtTypeReference<?> superclass = subClass.getSuperclass();
 		Assert.assertEquals(ExtendsObject.class.getName(), superclass.getQualifiedName());

src/test/java/spoon/reflect/declaration/UnknownDeclarationTest.java

@@ -6,6 +6,7 @@
 import spoon.reflect.visitor.CtScanner;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
 /**
@@ -21,6 +22,7 @@ private static class ExecutableReferenceVisitor extends CtScanner {
         public <T> void visitCtExecutableReference(final CtExecutableReference<T> reference) {
             final CtExecutable executable = reference.getDeclaration();
             assertNull(executable);
+
             referenceCounter++;
         }
     }

src/test/java/spoon/test/filters/FilterTest.java

@@ -447,7 +447,11 @@ public boolean matches(CtInvocation<?> element) {
 		final CtExecutableReference<?> expectedExecutable = expectedInv.getExecutable();
 		assertNotNull(expectedExecutable);
 		assertEquals("size", expectedExecutable.getSimpleName());
+
 		assertNull(expectedExecutable.getDeclaration());
+		CtExecutable<?> exec = expectedExecutable.getExecutableDeclaration();
+		assertEquals("size", exec.getSimpleName());
+		assertEquals("ArrayList", ((CtClass)exec.getParent()).getSimpleName());
 		final CtExecutable<?> declaration = expectedExecutable.getExecutableDeclaration();
 		assertNotNull(declaration);
 		assertEquals("size", declaration.getSimpleName());

src/test/java/spoon/test/generics/GenericsTest.java

@@ -14,6 +14,7 @@
 import spoon.reflect.declaration.CtClass;
 import spoon.reflect.declaration.CtConstructor;
 import spoon.reflect.declaration.CtElement;
+import spoon.reflect.declaration.CtExecutable;
 import spoon.reflect.declaration.CtField;
 import spoon.reflect.declaration.CtInterface;
 import spoon.reflect.declaration.CtMethod;
@@ -43,25 +44,29 @@
 import spoon.test.generics.testclasses.Banana;
 import spoon.test.generics.testclasses.CelebrationLunch;
 import spoon.test.generics.testclasses.CelebrationLunch.WeddingLunch;
+import spoon.test.generics.testclasses.EnumSetOf;
 import spoon.test.generics.testclasses.FakeTpl;
 import spoon.test.generics.testclasses.Lunch;
 import spoon.test.generics.testclasses.Mole;
 import spoon.test.generics.testclasses.Orange;
 import spoon.test.generics.testclasses.Paella;
 import spoon.test.generics.testclasses.Panini;
+import spoon.test.generics.testclasses.SameSignature;
 import spoon.test.generics.testclasses.Spaghetti;
 import spoon.test.generics.testclasses.Tacos;
 import spoon.testing.utils.ModelUtils;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.EnumSet;
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static spoon.testing.utils.ModelUtils.build;
@@ -1094,4 +1099,53 @@ public void testGetDeclarationOfTypeParameterReference() {
 		assertSame(innerTotoFormatCtType.get(0), paramInnerToto.getDeclaration());
 		assertSame(innerTypeParametersList.get(0), returnInnerToto.getDeclaration());
 	}
+
+	@Test
+	public void testIsSameSignatureWithGenerics() {
+		Launcher launcher = new Launcher();
+		launcher.addInputResource("./src/test/java/spoon/test/generics/testclasses/SameSignature.java");
+		launcher.buildModel();
+
+		CtClass ctClass = launcher.getFactory().Class().get(SameSignature.class);
+
+		List<CtMethod> methods = ctClass.getMethodsByName("forEach");
+		assertEquals(1, methods.size());
+
+		CtType<?> iterableItf = launcher.getFactory().Type().get(Iterable.class);
+
+		List<CtMethod<?>> methodsItf = iterableItf.getMethodsByName("forEach");
+		assertEquals(1, methodsItf.size());
+
+		ClassTypingContext ctc = new ClassTypingContext(ctClass.getReference());
+		assertTrue(ctc.isOverriding(methods.get(0), methodsItf.get(0)));
+		assertTrue(ctc.isSubSignature(methods.get(0), methodsItf.get(0)));
+		assertTrue(ctc.isSameSignature(methods.get(0), methodsItf.get(0)));
+	}
+	@Test
+	public void testGetExecDeclarationOfEnumSetOf() {
+		Launcher launcher = new Launcher();
+		launcher.addInputResource("./src/test/java/spoon/test/generics/testclasses/EnumSetOf.java");
+		launcher.buildModel();
+
+		CtClass<?> ctClass = launcher.getFactory().Class().get(EnumSetOf.class);
+
+		CtInvocation invocation = ctClass.getMethodsByName("m").get(0).getBody().getStatement(0);
+		CtExecutable<?> decl = invocation.getExecutable().getDeclaration();
+		assertNull(decl);
+
+		CtClass<?> enumClass = launcher.getFactory().Class().get(EnumSet.class);
+		List<CtMethod<?>> methods = enumClass.getMethodsByName("of");
+
+		CtMethod rightOfMethod = null;
+		for (CtMethod method : methods) {
+			if (method.getParameters().size() == 1) {
+				rightOfMethod = method;
+			}
+		}
+
+		assertNotNull(rightOfMethod);
+
+		decl = invocation.getExecutable().getExecutableDeclaration();
+		assertEquals(rightOfMethod, decl);
+	}
 }

src/test/java/spoon/test/generics/testclasses/EnumSetOf.java

@@ -0,0 +1,13 @@
+package spoon.test.generics.testclasses;
+
+import java.util.EnumSet;
+
+import spoon.reflect.declaration.ModifierKind;
+
+public class EnumSetOf {
+
+	public void m() {
+		EnumSet.of(ModifierKind.STATIC);
+	}
+
+}

src/test/java/spoon/test/generics/testclasses/SameSignature.java

@@ -0,0 +1,19 @@
+package spoon.test.generics.testclasses;
+
+import java.util.Iterator;
+import java.util.function.Consumer;
+
+/**
+ * Created by urli on 21/06/2017.
+ */
+public class SameSignature<T extends String> implements Iterable<T> {
+    @Override
+    public Iterator<T> iterator() {
+        return null;
+    }
+
+    @Override
+    public void forEach(Consumer<? super T> action) {
+
+    }
+}

src/test/java/spoon/test/reference/ExecutableReferenceGenericTest.java

@@ -86,7 +86,11 @@ public void testReferencesBetweenConstructorsInOtherClass() throws Exception {
 		assertEquals(3, refConstructors.size());
 		assertEquals(0, emptyConstructorClass1.getParameters().size());
 		assertEquals(0, emptyConstructorClass3.getParameters().size());
-		assertNull(refConstructors.get(0).getDeclaration()); // reference to Object constructor.
+
+		assertNull(refConstructors.get(0).getDeclaration());
+		CtExecutable<?> decl = refConstructors.get(0).getExecutableDeclaration();
+		assertEquals("Object", decl.getType().getSimpleName());
+		assertEquals(0, decl.getParameters().size());
 		assertNotNull(refConstructors.get(0).getExecutableDeclaration());
 		assertEquals(emptyConstructorClass1, refConstructors.get(1).getDeclaration());
 		assertEquals(emptyConstructorClass3, refConstructors.get(2).getDeclaration());