fix: fix top definitions filtering (#4873)

src/main/java/spoon/support/reflect/declaration/CtMethodImpl.java

@@ -29,6 +29,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * The implementation for {@link spoon.reflect.declaration.CtMethod}.
@@ -223,19 +224,18 @@ public Collection<CtMethod<?>> getTopDefinitions() {
 		});
 
 		// now removing the intermediate methods for which there exists a definition upper in the hierarchy
-		List<CtMethod<?>> finalMeths = new ArrayList<>(s);
-		for (CtMethod m1 : s) {
-			boolean m1IsIntermediate = false;
-			for (CtMethod m2 : s) {
-				if (context.isOverriding(m1, m2)) {
-					m1IsIntermediate = true;
-				}
-			}
-			if (!m1IsIntermediate) {
-				finalMeths.add(m1);
-			}
-		}
-		return finalMeths;
+		return s.stream()
+				.filter(ctMethod ->  isTopDefinition(ctMethod, s, context))
+				.collect(Collectors.toList());
+	}
+
+	/**
+	 * Returns {@code true} if the {@code method} parameter is a top definition,
+	 * i.e. no other candidate overrides it.
+	 */
+	private boolean isTopDefinition(CtMethod<?> method, Collection<CtMethod<?>> allCandidates, TypeAdaptor context) {
+		return allCandidates.stream()
+				.noneMatch(candidate -> candidate != method && context.isOverriding(method, candidate));
 	}
 
 	@Override

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

@@ -459,9 +459,8 @@ public void testgetTopDefinitions() {
 		List<CtMethod<?>> methods;
 
 		methods = orderByName(aTostada.getMethodsByName("make").get(0).getTopDefinitions());
-		assertEquals(2, methods.size());
-		assertEquals("AbstractTostada", methods.get(0).getDeclaringType().getSimpleName());
-		assertEquals("ITostada", methods.get(1).getDeclaringType().getSimpleName());
+		assertEquals(1, methods.size());
+		assertEquals("ITostada", methods.get(0).getDeclaringType().getSimpleName());
 
 		methods = orderByName(aTostada.getMethodsByName("prepare").get(0).getTopDefinitions());
 		assertEquals(1, methods.size());

src/test/java/spoon/test/method/MethodTest.java

@@ -16,8 +16,10 @@
  */
 package spoon.test.method;
 
+import org.hamcrest.CoreMatchers;
 import spoon.reflect.factory.Factory;
 import spoon.reflect.declaration.CtParameter;
+import spoon.test.method.testclasses.Hierarchy;
 import spoon.test.method.testclasses.Tacos;
 import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.filter.NamedElementFilter;
@@ -30,13 +32,19 @@
 import spoon.reflect.declaration.CtMethod;
 import spoon.test.method.testclasses.Methods;
 import org.junit.jupiter.api.Test;
+import spoon.testing.utils.ModelTest;
 
+import java.util.Collection;
+import java.util.List;
 import java.util.Set;
 import java.util.ConcurrentModificationException;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Arrays;
 
+import static org.hamcrest.CoreMatchers.anyOf;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.sameInstance;
 import static spoon.testing.utils.ModelUtils.buildClass;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -218,4 +226,23 @@ public void test_addFormalCtTypeParameterAt_throwsOutOfBoundsException_whenPosit
 		assertThrows(IndexOutOfBoundsException.class,
 				() -> method.addFormalCtTypeParameterAt(1, typeParam));
 	}
+
+	@ModelTest("src/test/java/spoon/test/method/testclasses/Hierarchy.java")
+	void test_getTopDefinitions_findsTopOnly(Factory factory) {
+		// contract: getTopDefinitions should find top-level definitions only
+		CtMethod<?> method = factory.Interface().get(Hierarchy.D.class).getMethods().iterator().next();
+		List<CtMethod<?>> topDefinitions = new ArrayList<>(method.getTopDefinitions());
+		assertThat(topDefinitions.size(), equalTo(2));
+		CtMethod<?> m0 = topDefinitions.get(0);
+		CtMethod<?> m1 = topDefinitions.get(1);
+		// two distinct elements
+		assertThat(m0, not(sameInstance(m1)));
+		// A1 and A2 declare the top-level methods
+		assertThat(m0.getDeclaringType(), not(equalTo(m1.getDeclaringType())));
+		assertThat(m0.getDeclaringType().getSimpleName(), anyOf(equalTo("A1"), equalTo("A2")));
+		assertThat(m1.getDeclaringType().getSimpleName(), anyOf(equalTo("A1"), equalTo("A2")));
+		// top-level definitions don't have top-level definitions
+		assertThat(m0.getTopDefinitions().size(), equalTo(0));
+		assertThat(m1.getTopDefinitions().size(), equalTo(0));
+	}
 }

src/test/java/spoon/test/method/testclasses/Hierarchy.java

@@ -0,0 +1,19 @@
+package spoon.test.method.testclasses;
+
+public class Hierarchy {
+	interface A1 {
+		void m();
+	}
+	interface A2 {
+		void m();
+	}
+	interface B extends A1 {
+		void m();
+	}
+	interface C extends B, A1 {
+		void m();
+	}
+	public interface D extends C, A2 {
+		void m();
+	}
+}