Skip to content

Commit

Permalink
fix importing inner class using string concat of outer field
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.

Signed-off-by: Peter Gafert <[email protected]>
  • Loading branch information
codecholeric committed Nov 26, 2023
1 parent 789706f commit a6fdc9d
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 creates a synthetic `access$123` method that creates a new `StringBuilder()`.
* 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 a6fdc9d

Please sign in to comment.