From 262880301a049d08694879809d9dc0759d6724ce Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Fri, 18 Feb 2022 21:06:19 +0700 Subject: [PATCH 1/4] fix `compileJdk9mainJava` task dependencies This task depends on the compiled output of the `compileJava` task, so we should explicitly declare this dependency. Otherwise, if you just run `compileJdk9mainJava` on a clean build it breaks. Signed-off-by: Peter Gafert --- archunit/build.gradle | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/archunit/build.gradle b/archunit/build.gradle index 44370c6845..4754ef02b0 100644 --- a/archunit/build.gradle +++ b/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 From a98c4d65b795e7a52327e1b21d5a151825fab075 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Fri, 25 Feb 2022 13:48:09 +0700 Subject: [PATCH 2/4] log dependency resolution process configuration on debug In the end this is not relevant for most users which will be happy with the default configuration and do not need to get into the depths of tweaking specific resolutions. Signed-off-by: Peter Gafert --- .../archunit/core/importer/DependencyResolutionProcess.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/DependencyResolutionProcess.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/DependencyResolutionProcess.java index d032c3c42c..c8adafe049 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/DependencyResolutionProcess.java +++ b/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), From 63618cb70238b96a11591410e1c0fae409e26a56 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Thu, 24 Feb 2022 14:57:41 +0700 Subject: [PATCH 3/4] fix call origin cannot be found if descriptor and signature mismatch 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). Note that for parameter types we have already separated this a while ago, so there it never was a problem. Signed-off-by: Peter Gafert --- .../archunit/core/domain/JavaCodeUnit.java | 26 +++++++++++++++--- .../core/importer/DomainBuilders.java | 8 ++++-- .../ClassFileImporterAccessesTest.java | 26 ++++++++++++++++++ .../core/importer/ImportOptionsTest.java | 7 ++--- .../tngtech/archunit/testutil/TestUtils.java | 8 ++++++ ...ismatchBetweenDescriptorAndSignature.class | Bin 0 -> 2334 bytes 6 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 archunit/src/test/resources/com/tngtech/archunit/core/importer/testexamples/MismatchBetweenDescriptorAndSignature.class diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java index 3cde461bab..e654e44e4f 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java +++ b/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, HasThrowsClause { - private final JavaType returnType; + private final ReturnType returnType; private final Parameters parameters; private final String fullName; private final List> 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 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 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() { } diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java index fabd004a71..d3bb39e92a 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java +++ b/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> allTypeParametersInContextOf(JavaCodeUnit codeUnit) { diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterAccessesTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterAccessesTest.java index 25e99d22cb..c90e2f914c 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterAccessesTest.java +++ b/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) = 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 withoutJavaLangTargets(Set dependencies) { Set result = new HashSet<>(); for (Dependency dependency : dependencies) { diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ImportOptionsTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ImportOptionsTest.java index ef6502a212..fc141e6859 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/importer/ImportOptionsTest.java +++ b/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)); } diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java b/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java index 0636c73474..6aaf7fcd50 100644 --- a/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java +++ b/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); + } + } } diff --git a/archunit/src/test/resources/com/tngtech/archunit/core/importer/testexamples/MismatchBetweenDescriptorAndSignature.class b/archunit/src/test/resources/com/tngtech/archunit/core/importer/testexamples/MismatchBetweenDescriptorAndSignature.class new file mode 100644 index 0000000000000000000000000000000000000000..c4012f04c58cf6b62bac8c6d3e73a83039640e84 GIT binary patch literal 2334 zcmb_dOHb`#`?U5{FL!D$Y?dD#cb3dKuchCLv-S2+@xQl&;CuO51i&Lfc zQWJ8{RIHI2Ai1KG9@4-e1DA%q!tH!p@_qBkU4m`X_#XIULLYVr%OPL|xM zSUNlVp|rCRa9m;tuBuexjA(!gYqQ>V*@*7nrz&&oRXi zI1#V*UNFq`7TzPQ7jiF$8E*9gJ*xHsJ6t`-)HW`JFR9g<(5v>5+njs^w~*kN!7Rh& zp0L||4`GfW)KIj#a6%kSj|#MQVGU+9u#;ZtvPnt|mRB-N%u_VCNg zl;RPZs0sbTu~#w5qC@u`VJj8IR;VFTt>L3_!<;4PEFdQ*q(4<1DMeou@>^;(ukcor zOJfq3!hSBr5+o*Bs~P5Ft!8Fu*gq^Sx0>$8QtZ*ym^2|RvrI7NqrKI9IQr(hpcL~z z8j1xb!X7#mB=z^f(3{u6R@3p6VYv6H!{FR}W4PW)vfn79gekwlwu=SJu4rnBVf?*U zE`%+H8y}su46?^O_X-tM%l_zWc?fD{w0ExEKB30>b4EqbL4ij5ZoO6#<}+uMISn_; zingzqsv~>KfudT~6}xT{VDf2Qx78Xw3k}s$3A5h4feewnt_w4#DV8N@%tSZ!vSwH` z%ZUp|Muo;{(H#q0s^e^=E!-ESdUf;E7P>_*0u{=0vHvdzNYhjcP;KdfiN%~TN96H2 zwZ;&w1(MbXA<@p0b3bD^`75seNRosdT7!fM1m0nnzDL_A+OLsBc#M%XVkO2gK{g#* zi>~b0`U%6o;ReGGr0v>5;BVJfgm8|9wyixEGl|EvJ03ryo#7^?=}*V%0ZD$^qjN3H z?D*E^e?js!QeD&B2`vbO2LE!+CkakZ93`Kjzyfr1heC6bjGl!b#S*?E!)@nr!aSjK zP8tUXO6n=CemuY<5933TG!nM)jP|$1o%P530#Eq1`&{#6ajNsR#pkBdQx|dn=`xYU z%+$FdMHJ=ly(~Og;OXh~{GFx6)JmE!PG?r=GtHBm9M36}9OZnMs Date: Fri, 25 Feb 2022 13:39:10 +0700 Subject: [PATCH 4/4] compare descriptor to find call origins Since we meanwhile have the descriptor both on the search target and the code unit candidates, we can simply compare that instead of comparing raw parameter types and return type individually. Note that this by itself would have already fixed the bug where call origins cannot be identified if descriptor and signature mismatch, but nevertheless the API would have been behaving wrongly when deriving the raw return type from the erasure of the generic return type. Signed-off-by: Peter Gafert --- .../com/tngtech/archunit/core/importer/RawAccessRecord.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/RawAccessRecord.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/RawAccessRecord.java index c4c3ab7e92..4e1f6bbc6d 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/RawAccessRecord.java +++ b/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()); } }