fix non-deterministic return value of JavaCodeUnit.getMethod() #717

In some cases (like bridge methods) there would be more than one method matching the same name and parameter type names. Since we used a `Set` and just iterated until we found some match the result was not deterministic in these cases, i.e. depended on the JVM implementation or similar uncertain factors.
We now take all code units into consideration that match name and parameters. If this result should not be deterministic we make sure to pick the non-synthetic code unit first because users are usually interested in that one (coming from their source code) instead of some synthetic code added by the compiler.

Resolves: #256

archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassMembers.java

@@ -16,18 +16,25 @@
 package com.tngtech.archunit.core.domain;
 
 import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
+import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.tngtech.archunit.base.Optional;
 
 import static com.google.common.collect.Iterables.concat;
+import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.tngtech.archunit.core.domain.JavaConstructor.CONSTRUCTOR_NAME;
 import static com.tngtech.archunit.core.domain.JavaModifier.ENUM;
+import static com.tngtech.archunit.core.domain.JavaModifier.SYNTHETIC;
 import static com.tngtech.archunit.core.domain.properties.HasName.Utils.namesOf;
 
 class JavaClassMembers {
@@ -256,12 +263,29 @@ private <T extends JavaCodeUnit> T findMatchingCodeUnit(Set<T> codeUnits, String
     }
 
     private <T extends JavaCodeUnit> Optional<T> tryFindMatchingCodeUnit(Set<T> codeUnits, String name, List<String> parameters) {
+        Set<T> matching = findCodeUnitsWithMatchingNameAndParameters(codeUnits, name, parameters);
+
+        if (matching.isEmpty()) {
+            return Optional.empty();
+        } else if (matching.size() == 1) {
+            return Optional.of(getOnlyElement(matching));
+        } else {
+            // In this case we have some synthetic methods like bridge methods making name and parameters alone ambiguous
+            // We want to return the non-synthetic method first because that is usually the relevant one for users
+            SortedSet<T> sortedByPriority = new TreeSet<>(SORTED_BY_SYNTHETIC_LAST_THEN_FULL_NAME);
+            sortedByPriority.addAll(matching);
+            return Optional.of(sortedByPriority.first());
+        }
+    }
+
+    private <T extends JavaCodeUnit> Set<T> findCodeUnitsWithMatchingNameAndParameters(Set<T> codeUnits, String name, List<String> parameters) {
+        Set<T> matching = new HashSet<>();
         for (T codeUnit : codeUnits) {
             if (name.equals(codeUnit.getName()) && parameters.equals(namesOf(codeUnit.getRawParameterTypes()))) {
-                return Optional.of(codeUnit);
+                matching.add(codeUnit);
             }
         }
-        return Optional.empty();
+        return matching;
     }
 
     void completeAnnotations(ImportContext context) {
@@ -282,6 +306,16 @@ void setReverseDependencies(ReverseDependencies reverseDependencies) {
         }
     }
 
+    private static final Comparator<JavaCodeUnit> SORTED_BY_SYNTHETIC_LAST_THEN_FULL_NAME = new Comparator<JavaCodeUnit>() {
+        @Override
+        public int compare(JavaCodeUnit codeUnit1, JavaCodeUnit codeUnit2) {
+            return ComparisonChain.start()
+                    .compareTrueFirst(!codeUnit1.getModifiers().contains(SYNTHETIC), !codeUnit2.getModifiers().contains(SYNTHETIC))
+                    .compare(codeUnit1.getFullName(), codeUnit2.getFullName())
+                    .result();
+        }
+    };
+
     static JavaClassMembers empty(JavaClass owner) {
         return new JavaClassMembers(
                 owner,

archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java

@@ -81,6 +81,7 @@
 import static com.tngtech.archunit.core.domain.JavaClass.Predicates.type;
 import static com.tngtech.archunit.core.domain.JavaConstructor.CONSTRUCTOR_NAME;
 import static com.tngtech.archunit.core.domain.JavaFieldAccess.AccessType.SET;
+import static com.tngtech.archunit.core.domain.JavaModifier.SYNTHETIC;
 import static com.tngtech.archunit.core.domain.TestUtils.importClassWithContext;
 import static com.tngtech.archunit.core.domain.TestUtils.importClasses;
 import static com.tngtech.archunit.core.domain.TestUtils.importClassesWithContext;
@@ -469,6 +470,28 @@ public void tryGetCodeUnitWithParameterTypes() {
         assertThat(clazz.tryGetCodeUnitWithParameterTypeNames(CONSTRUCTOR_NAME, Collections.<String>emptyList())).isAbsent();
     }
 
+    @Test
+    public void getMethod_returns_non_synthetic_method_if_method_name_and_parameters_are_ambiguous() {
+        class Parent {
+            @SuppressWarnings("unused")
+            Object covariantlyOverriddenCausingBridgeMethod() {
+                return null;
+            }
+        }
+        class Child extends Parent {
+            @Override
+            String covariantlyOverriddenCausingBridgeMethod() {
+                return null;
+            }
+        }
+
+        JavaClass javaClass = new ClassFileImporter().importClasses(Parent.class, Child.class).get(Child.class);
+        JavaMethod method = javaClass.getMethod("covariantlyOverriddenCausingBridgeMethod");
+
+        assertThatType(method.getReturnType()).matches(String.class);
+        assertThat(method.getModifiers()).doesNotContain(SYNTHETIC);
+    }
+
     @Test
     public void has_no_dependencies_to_primitives() {
         JavaClass javaClass = importClassWithContext(AllPrimitiveDependencies.class);