Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

review: fix issue related with overridden methods retrieved from getAllMethods #1375

Merged
merged 11 commits into from
Jun 22, 2017
2 changes: 1 addition & 1 deletion src/main/java/spoon/reflect/factory/ExecutableFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private <T> CtExecutableReference<T> createReferenceInternal(CtExecutable<T> e)
return createReference(((CtMethod<T>) e).getDeclaringType().getReference(), isStatic, ((CtMethod<T>) e).getType().clone(), executableName, refs);
} else if (e instanceof CtLambda) {
CtMethod<T> lambdaMethod = ((CtLambda) e).getOverriddenMethod();
return createReference(e.getParent(CtType.class).getReference(), lambdaMethod == null ? null : lambdaMethod.getType(), executableName, refs);
return createReference(e.getParent(CtType.class).getReference(), lambdaMethod == null ? null : lambdaMethod.getType().clone(), executableName, refs);
} else if (e instanceof CtAnonymousExecutable) {
return createReference(((CtAnonymousExecutable) e).getDeclaringType().getReference(), e.getType().clone(), executableName);
}
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import spoon.support.compiler.SnippetCompilationHelper;
import spoon.support.util.QualifiedNameBasedSortedSet;
import spoon.support.util.SignatureBasedSortedSet;
import spoon.support.visitor.ClassTypingContext;

import java.lang.annotation.Annotation;
import java.util.ArrayList;
Expand Down Expand Up @@ -895,14 +896,18 @@ public Collection<CtExecutableReference<?>> getAllExecutables() {

@Override
public Set<CtMethod<?>> getAllMethods() {
final Set<String> distinctSignatures = new HashSet<>();
final Set<CtMethod<?>> l = new SignatureBasedSortedSet<>();
final Set<CtMethod<?>> l = new HashSet<>();
final ClassTypingContext ctc = new ClassTypingContext(this);
map(new AllTypeMembersFunction(CtMethod.class)).forEach(new CtConsumer<CtMethod<?>>() {
@Override
public void accept(CtMethod<?> method) {
if (distinctSignatures.add(method.getSignature())) {
l.add(method);
public void accept(CtMethod<?> currentMethod) {
for (CtMethod<?> alreadyVisitedMethod : l) {
if (ctc.isSameSignature(currentMethod, alreadyVisitedMethod)) {
return;
}
}

l.add(currentMethod);
}
});
return Collections.unmodifiableSet(l);
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/spoon/support/visitor/ClassTypingContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,9 @@ private CtTypeReference<?> getEnclosingType(CtTypeReference<?> typeRef) {
*/
@Override
protected CtTypeReference<?> adaptTypeParameter(CtTypeParameter typeParam) {
if (typeParam == null) {
throw new SpoonException("You cannot adapt a null type parameter.");
}
CtFormalTypeDeclarer declarer = typeParam.getTypeParameterDeclarer();
if ((declarer instanceof CtType<?>) == false) {
return null;
Expand Down Expand Up @@ -643,6 +646,10 @@ private CtTypeReference<?> adaptTypeForNewMethod(CtTypeReference<?> typeRef) {
if (typeRef instanceof CtTypeParameterReference) {
CtTypeParameterReference typeParamRef = (CtTypeParameterReference) typeRef;
CtTypeParameter typeParam = typeParamRef.getDeclaration();
if (typeParam == null) {
throw new SpoonException("Declaration of the CtTypeParameter should not be null.");
}

if (typeParam.getTypeParameterDeclarer() instanceof CtExecutable) {
//the parameter is declared in scope of Method or Constructor
return typeRef.clone();
Expand Down
31 changes: 28 additions & 3 deletions src/test/java/spoon/reflect/declaration/CtTypeInformationTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package spoon.reflect.declaration;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static spoon.testing.utils.ModelUtils.build;

import java.lang.reflect.Field;
Expand Down Expand Up @@ -135,13 +137,18 @@ public void testGetSuperclass() throws Exception {

// + 48 of ArrayList (in library)
// + 12 of java.lang.Object
Assert.assertEquals(1+12+48, extendObject.getAllMethods().size());
// 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());

final CtType<?> subClass = this.factory.Type().get(Subclass.class);
assertEquals(2, subClass.getMethods().size());

// the abstract method from Comparable which is overridden is also present in the model
assertEquals(61+3, subClass.getAllMethods().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());

CtTypeReference<?> superclass = subClass.getSuperclass();
Assert.assertEquals(ExtendsObject.class.getName(), superclass.getQualifiedName());
Expand Down Expand Up @@ -172,4 +179,22 @@ public void testGetSuperclass() throws Exception {
type2.getMethodsByName("foo").get(0).getSignature());

}

@Test
public void testGetAllMethodsWontReturnOverriddenMethod() {
final CtType<?> subClass = this.factory.Type().get(Subclass.class);
Set<CtMethod<?>> listCtMethods = subClass.getAllMethods();

boolean detectedCompareTo = false;
for (CtMethod<?> ctMethod : listCtMethods) {
if (ctMethod.getSimpleName().equals("compareTo")) {
assertFalse(ctMethod.hasModifier(ModifierKind.ABSTRACT));
assertFalse(ctMethod.getParameters().get(0).getType() instanceof CtTypeParameter);
assertEquals("Object", ctMethod.getParameters().get(0).getType().getSimpleName());
detectedCompareTo = true;
}
}

assertTrue(detectedCompareTo);
}
}