From cabbe13c8b7f04d11332ca0433a1f51c03c7236e Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Sun, 26 Nov 2023 23:14:28 +0100 Subject: [PATCH] fix importing inner class using string concat of outer field We weren't aware that the compiler occasionally generates synthetic `access$123` methods that call constructors. More precisely for the following constellation ``` class Outer { String field = ""; class Inner { void concat() { field += "any"; } } } ``` the compiler may generate bytecode instantiating and using a `StringBuilder` (depending on the JDK version). But for constructor calls the synthetic access resolution was not hooked in, because we assumed that those methods would never call a constructor. This in turn lead to the bug ``` java.lang.IllegalStateException: Never found a JavaCodeUnit that matches supposed origin CodeUnit{name='access$123'... ``` I.e. the method `access$123` was filtered out as synthetic, but the origin of the constructor call had not been resolved to the actual `Inner.concat` method. Signed-off-by: Peter Gafert --- .../core/importer/ClassFileImportRecord.java | 2 +- ...eImporterSyntheticPrivateAccessesTest.java | 36 +++++++++++++++++++ .../testutil/assertion/AccessesAssertion.java | 15 +++++++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImportRecord.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImportRecord.java index 0df029f4fd..f82699dd4f 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImportRecord.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImportRecord.java @@ -264,7 +264,7 @@ void forEachRawMethodCallRecord(Consumer doWithRecord) { } void forEachRawConstructorCallRecord(Consumer doWithRecord) { - resolveSyntheticOrigins(rawConstructorCallRecords, COPY_RAW_ACCESS_RECORD, syntheticLambdaAccessRecorder) + resolveSyntheticOrigins(rawConstructorCallRecords, COPY_RAW_ACCESS_RECORD, syntheticPrivateAccessRecorder, syntheticLambdaAccessRecorder) .forEach(doWithRecord); } diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterSyntheticPrivateAccessesTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterSyntheticPrivateAccessesTest.java index 9ff4fc9c53..781e53bd7e 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterSyntheticPrivateAccessesTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterSyntheticPrivateAccessesTest.java @@ -5,6 +5,7 @@ import com.tngtech.archunit.core.domain.JavaAccess; import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaFieldAccess; import org.junit.Test; import static com.tngtech.archunit.core.domain.JavaConstructor.CONSTRUCTOR_NAME; @@ -12,6 +13,7 @@ import static com.tngtech.archunit.core.domain.JavaFieldAccess.AccessType.SET; import static com.tngtech.archunit.testutil.Assertions.assertThatAccesses; import static com.tngtech.archunit.testutil.Assertions.expectedAccess; +import static com.tngtech.archunit.testutil.assertion.AccessesAssertion.access; import static java.util.stream.Collectors.toSet; public class ClassFileImporterSyntheticPrivateAccessesTest { @@ -422,6 +424,40 @@ Supplier fieldAccesses = new ClassFileImporter().importClasses(Outer.class, Outer.Inner.class) + .get(Outer.Inner.class) + .getFieldAccessesFromSelf(); + + assertThatAccesses(fieldAccesses) + .contain(access() + .fromOrigin(Outer.Inner.class, "stringConcat") + .toTarget(Outer.class, "outerPrivateString") + .withAccessType(GET)) + .contain(access() + .fromOrigin(Outer.Inner.class, "stringConcat") + .toTarget(Outer.class, "outerPrivateString") + .withAccessType(SET)); + } + @SuppressWarnings("OptionalGetWithoutIsPresent") private Set> importRelevantAccesses(Class origin, Class target) { return new ClassFileImporter().importClasses(origin, target).get(origin).getMethods().stream() diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/AccessesAssertion.java b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/AccessesAssertion.java index 35c6284020..3e1dfc88f3 100644 --- a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/AccessesAssertion.java +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/AccessesAssertion.java @@ -7,6 +7,8 @@ import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.core.domain.JavaAccess; +import com.tngtech.archunit.core.domain.JavaFieldAccess; +import com.tngtech.archunit.core.domain.JavaFieldAccess.AccessType; import org.assertj.core.api.Condition; import static org.assertj.core.api.Assertions.assertThat; @@ -35,7 +37,9 @@ public final AccessesAssertion containOnly(Condition>... c } assertThat(actualRemaining).as("Unexpected " + JavaAccess.class.getSimpleName()).isEmpty(); return this; - }public static AccessCondition access() { + } + + public static AccessCondition access() { return new AccessCondition(); } @@ -69,6 +73,15 @@ public AccessCondition toTarget(Class owner, String name) { .as("%s to target %s.%s", predicate.getDescription(), owner.getName(), name)); } + public Condition> withAccessType(AccessType accessType) { + DescribedPredicate> withAccessType = DescribedPredicate.describe("", access -> + access instanceof JavaFieldAccess + && ((JavaFieldAccess) access).getAccessType().equals(accessType)); + return new AccessCondition( + predicate.and(withAccessType) + .as("%s with access type %s", predicate.getDescription(), accessType)); + } + public AccessCondition declaredInLambda() { return new AccessCondition( predicate.and(DescribedPredicate.describe("", JavaAccess::isDeclaredInLambda))