Fix bug where call origins cannot be found on descriptor signature mi…

…smatch #812

With ArchUnit `0.23.0` we fixed an ambiguity problem when resolving method call origins (bridge methods would cause two methods with same name and parameter types, so we would pick one randomly, which could be the wrong/synthetic one). Unfortunately, this broke some Kotlin use cases, because inline functions would cause the compiler to create synthetic classes with methods where the signature and the descriptor do not match (e.g. the signature says the return type is String, but the descriptor claims Object). In these cases the erasure of the generic type does not match the raw type. But so far for the return type we derived the raw return type as erasure from the generic return type.
This is now fixed by keeping raw and generic return type completely separate, since we obviously cannot derive one from the other (the JVM spec also clearly states that this is a valid case, that descriptor and signature do not need to match exactly and are not validated against each other).

archunit/build.gradle

@@ -88,7 +88,8 @@ dependencies {
     runtimeOnly sourceSets.jdk9main.output
 }
 
-compileJdk9mainJava.with {
+compileJdk9mainJava {
+    dependsOn(compileJava)
     ext.minimumJavaVersion = JavaVersion.VERSION_1_9
 
     destinationDir = compileJava.destinationDir

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

@@ -55,7 +55,7 @@ public abstract class JavaCodeUnit
         extends JavaMember
         implements HasParameterTypes, HasReturnType, HasTypeParameters<JavaCodeUnit>, HasThrowsClause<JavaCodeUnit> {
 
-    private final JavaType returnType;
+    private final ReturnType returnType;
     private final Parameters parameters;
     private final String fullName;
     private final List<JavaTypeVariable<JavaCodeUnit>> typeParameters;
@@ -71,7 +71,7 @@ public abstract class JavaCodeUnit
     JavaCodeUnit(JavaCodeUnitBuilder<?, ?> builder) {
         super(builder);
         typeParameters = builder.getTypeParameters(this);
-        returnType = builder.getReturnType(this);
+        returnType = new ReturnType(this, builder);
         parameters = new Parameters(this, builder);
         fullName = formatMethod(getOwner().getName(), getName(), namesOf(getRawParameterTypes()));
         referencedClassObjects = ImmutableSet.copyOf(builder.getReferencedClassObjects(this));
@@ -157,13 +157,13 @@ public List<JavaClass> getExceptionTypes() {
     @Override
     @PublicAPI(usage = ACCESS)
     public JavaType getReturnType() {
-        return returnType;
+        return returnType.get();
     }
 
     @Override
     @PublicAPI(usage = ACCESS)
     public JavaClass getRawReturnType() {
-        return returnType.toErasure();
+        return returnType.getRaw();
     }
 
     @PublicAPI(usage = ACCESS)
@@ -323,6 +323,24 @@ protected List<JavaParameter> delegate() {
         }
     }
 
+    private static class ReturnType {
+        private final JavaClass rawReturnType;
+        private final JavaType returnType;
+
+        ReturnType(JavaCodeUnit owner, JavaCodeUnitBuilder<?, ?> builder) {
+            rawReturnType = builder.getRawReturnType();
+            returnType = builder.getGenericReturnType(owner);
+        }
+
+        JavaClass getRaw() {
+            return rawReturnType;
+        }
+
+        JavaType get() {
+            return returnType;
+        }
+    }
+
     public static final class Predicates {
         private Predicates() {
         }

archunit/src/main/java/com/tngtech/archunit/core/importer/DependencyResolutionProcess.java

@@ -124,7 +124,7 @@ void resolve(ImportedClasses classes) {
     }
 
     private void logConfiguration() {
-        log.info("Automatically resolving transitive class dependencies with the following configuration:{}{}{}{}{}{}",
+        log.debug("Automatically resolving transitive class dependencies with the following configuration:{}{}{}{}{}{}",
                 formatConfigProperty(MAX_ITERATIONS_FOR_MEMBER_TYPES_PROPERTY_NAME, maxRunsForMemberTypes),
                 formatConfigProperty(MAX_ITERATIONS_FOR_ACCESSES_TO_TYPES_PROPERTY_NAME, maxRunsForAccessesToTypes),
                 formatConfigProperty(MAX_ITERATIONS_FOR_SUPERTYPES_PROPERTY_NAME, maxRunsForSupertypes),

archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java

@@ -305,10 +305,14 @@ boolean hasNoParameters() {
             return rawParameterTypes.isEmpty();
         }
 
-        public JavaType getReturnType(JavaCodeUnit codeUnit) {
+        public JavaClass getRawReturnType() {
+            return get(rawReturnType.getFullyQualifiedClassName());
+        }
+
+        public JavaType getGenericReturnType(JavaCodeUnit codeUnit) {
             return genericReturnType.isPresent()
                     ? genericReturnType.get().finish(codeUnit, allTypeParametersInContextOf(codeUnit), importedClasses)
-                    : get(rawReturnType.getFullyQualifiedClassName());
+                    : getRawReturnType();
         }
 
         private Iterable<JavaTypeVariable<?>> allTypeParametersInContextOf(JavaCodeUnit codeUnit) {

archunit/src/main/java/com/tngtech/archunit/core/importer/RawAccessRecord.java

@@ -22,7 +22,6 @@
 import com.tngtech.archunit.core.domain.JavaClassDescriptor;
 import com.tngtech.archunit.core.domain.JavaCodeUnit;
 import com.tngtech.archunit.core.domain.JavaFieldAccess.AccessType;
-import com.tngtech.archunit.core.domain.properties.HasName;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -139,8 +138,7 @@ public String toString() {
 
         public boolean is(JavaCodeUnit method) {
             return getName().equals(method.getName())
-                    && getRawParameterTypeNames().equals(HasName.Utils.namesOf(method.getRawParameterTypes()))
-                    && returnType.getFullyQualifiedClassName().equals(method.getRawReturnType().getName())
+                    && descriptor.equals(method.getDescriptor())
                     && getDeclaringClassName().equals(method.getOwner().getName());
         }
     }

archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterAccessesTest.java

@@ -101,6 +101,8 @@
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.constructor;
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.field;
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.method;
+import static com.tngtech.archunit.testutil.TestUtils.relativeResourceUri;
+import static java.util.Collections.singleton;
 
 @RunWith(DataProviderRunner.class)
 public class ClassFileImporterAccessesTest {
@@ -898,6 +900,30 @@ void call(Target target) {
                 .isEqualTo(call.getTarget().getRawParameterTypes());
     }
 
+    @Test
+    public void identifies_call_origin_if_signature_and_descriptor_deviate() {
+        // Kotlin inline functions cause the creation of extra classes where the signature of the respective method shows
+        // the real types while the descriptor shows the erased types. The erasure of the real types from the signature
+        // does then not match the descriptor in some cases.
+        //
+        // The file `MismatchBetweenDescriptorAndSignature.class` is a byproduct of the following source code:
+        // --------------------
+        // package com.example
+        //
+        // object CrashArchUnit {
+        //     fun useInlineFunctionCrashingArchUnit(strings: List<String>) = strings.groupingBy { it.length }
+        // }
+        // --------------------
+        // With the current Kotlin version this creates a synthetic class file `CrashArchUnit$useInlineFunctionCrashingArchUnit$$inlined$groupingBy$1.class`
+        // which has been copied and renamed to `MismatchBetweenDescriptorAndSignature.class`.
+
+        JavaClass javaClass = getOnlyElement(new ClassFileImporter().importLocations(singleton(Location.of(
+                relativeResourceUri(getClass(), "testexamples/MismatchBetweenDescriptorAndSignature.class")))));
+
+        // this method in the problematic compiled class has a mismatch between return type of descriptor and signature
+        assertThat(javaClass.getMethod("keyOf", Object.class).getMethodCallsFromSelf()).isNotEmpty();
+    }
+
     private Set<Dependency> withoutJavaLangTargets(Set<Dependency> dependencies) {
         Set<Dependency> result = new HashSet<>();
         for (Dependency dependency : dependencies) {

archunit/src/test/java/com/tngtech/archunit/core/importer/ImportOptionsTest.java

@@ -29,6 +29,7 @@
 import static com.tngtech.archunit.core.importer.ImportOption.Predefined.DO_NOT_INCLUDE_PACKAGE_INFOS;
 import static com.tngtech.archunit.core.importer.ImportOption.Predefined.DO_NOT_INCLUDE_TESTS;
 import static com.tngtech.archunit.core.importer.ImportOption.Predefined.ONLY_INCLUDE_TESTS;
+import static com.tngtech.archunit.testutil.TestUtils.relativeResourceUri;
 import static com.tngtech.java.junit.dataprovider.DataProviders.$;
 import static com.tngtech.java.junit.dataprovider.DataProviders.crossProduct;
 import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach;
@@ -168,7 +169,7 @@ public static Object[][] do_not_include_package_info_classes() {
     @Test
     @UseDataProvider("do_not_include_package_info_classes")
     public void detect_package_info_class(ImportOption doNotIncludePackageInfoClasses) throws URISyntaxException {
-        Location packageInfoLocation = Location.of(getClass().getResource(testExampleResourcePath("package-info.class")).toURI());
+        Location packageInfoLocation = Location.of(relativeResourceUri(getClass(), "testexamples/package-info.class"));
         assertThat(doNotIncludePackageInfoClasses.includes(packageInfoLocation))
                 .as("doNotIncludePackageInfoClasses includes package-info.class")
                 .isFalse();
@@ -179,10 +180,6 @@ public void detect_package_info_class(ImportOption doNotIncludePackageInfoClasse
                 .isTrue();
     }
 
-    private String testExampleResourcePath(String resourceName) {
-        return "/" + getClass().getPackage().getName().replace(".", "/") + "/testexamples/" + resourceName;
-    }
-
     private static Location locationOf(Class<?> clazz) {
         return getLast(Locations.ofClass(clazz));
     }

archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java

@@ -84,4 +84,12 @@ public static URL urlOf(Class<?> clazz) {
     public static URI uriOf(Class<?> clazz) {
         return toUri(urlOf(clazz));
     }
+
+    public static URI relativeResourceUri(Class<?> relativeToClass, String resourceName) {
+        try {
+            return relativeToClass.getResource("/" + relativeToClass.getPackage().getName().replace(".", "/") + "/" + resourceName).toURI();
+        } catch (URISyntaxException e) {
+            throw new RuntimeException(e);
+        }
+    }
 }