Skip to content

Commit

Permalink
fix importing inner class using string concat of outer field (#1203)
Browse files Browse the repository at this point in the history
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 generates bytecode instantiating and using a
`StringBuilder`. 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.

Resolves: #1146
Resolves: #1194
  • Loading branch information
codecholeric authored Dec 3, 2023
2 parents 47b5e29 + cabbe13 commit 4b0a225
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ void forEachRawMethodCallRecord(Consumer<RawAccessRecord> doWithRecord) {
}

void forEachRawConstructorCallRecord(Consumer<RawAccessRecord> doWithRecord) {
resolveSyntheticOrigins(rawConstructorCallRecords, COPY_RAW_ACCESS_RECORD, syntheticLambdaAccessRecorder)
resolveSyntheticOrigins(rawConstructorCallRecords, COPY_RAW_ACCESS_RECORD, syntheticPrivateAccessRecorder, syntheticLambdaAccessRecorder)
.forEach(doWithRecord);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@

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;
import static com.tngtech.archunit.core.domain.JavaFieldAccess.AccessType.GET;
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 {
Expand Down Expand Up @@ -422,6 +424,40 @@ Supplier<Supplier<Data_of_imports_private_constructor_reference_from_lambda.Targ
);
}

/**
* This is a special case, because for += concatenation of an outer string from an inner class
* the compiler may create a synthetic `access$123` method that creates a new `StringBuilder()` (depending on the JDK version).
* Before this we wrongly assumed that all that happens from such `access$123` methods
* are field accesses and method calls, but no constructor calls.
*/
@Test
public void imports_synthetic_access_from_string_concatenation() {
@SuppressWarnings("unused")
class Outer {
private String outerPrivateString = "Hello";

class Inner {
private void stringConcat() {
outerPrivateString += ", world!";
}
}
}

Set<JavaFieldAccess> 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<JavaAccess<?>> importRelevantAccesses(Class<?> origin, Class<?> target) {
return new ClassFileImporter().importClasses(origin, target).get(origin).getMethods().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -35,7 +37,9 @@ public final AccessesAssertion containOnly(Condition<? super JavaAccess<?>>... c
}
assertThat(actualRemaining).as("Unexpected " + JavaAccess.class.getSimpleName()).isEmpty();
return this;
}public static AccessCondition access() {
}

public static AccessCondition access() {
return new AccessCondition();
}

Expand Down Expand Up @@ -69,6 +73,15 @@ public AccessCondition toTarget(Class<?> owner, String name) {
.as("%s to target %s.%s", predicate.getDescription(), owner.getName(), name));
}

public Condition<? super JavaAccess<?>> withAccessType(AccessType accessType) {
DescribedPredicate<JavaAccess<?>> 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))
Expand Down

0 comments on commit 4b0a225

Please sign in to comment.