Skip to content

Commit

Permalink
fix referenced class objects not detected in lambdas
Browse files Browse the repository at this point in the history
With a404fb4 we changed the import of lambdas, such that accesses declared within lambdas are not associated with the synthetic lambda method anymore, but instead with the real method surrounding the lambda. Unfortunately we overlooked non-access dependencies declared within lambdas, like referenced class objects. Since we filter out synthetic lambda methods completely, this meant that those dependencies now completely vanished from the import.
We now fix this by applying the same non-synthetic origin resolution mechanism to `RawReferencedClassObject` that we only applied to `RawAccessRecord` before. This also means, that we can't simply add `RawReferencedClassObject` directly to the `JavaCodeUnitBuilder` anymore and in particular can't resolve it directly on `JavaCodeUnit` creation, but instead supply it later on like the accesses.

Signed-off-by: Peter Gafert <[email protected]>
  • Loading branch information
codecholeric committed Nov 21, 2022
1 parent 710b565 commit 4f5698b
Show file tree
Hide file tree
Showing 19 changed files with 211 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ public static Source createSource(URI uri, Optional<String> sourceFileName, bool
return new Source(uri, sourceFileName, md5InClassSourcesEnabled);
}

public static ReferencedClassObject createReferencedClassObject(JavaCodeUnit codeUnit, JavaClass javaClass, int lineNumber) {
return ReferencedClassObject.from(codeUnit, javaClass, lineNumber);
public static ReferencedClassObject createReferencedClassObject(JavaCodeUnit codeUnit, JavaClass javaClass, int lineNumber, boolean declaredInLambda) {
return ReferencedClassObject.from(codeUnit, javaClass, lineNumber, declaredInLambda);
}

public static <CODE_UNIT extends JavaCodeUnit> ThrowsClause<CODE_UNIT> createThrowsClause(CODE_UNIT codeUnit, List<JavaClass> types) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,7 @@ public interface ImportContext {

Set<TryCatchBlockBuilder> createTryCatchBlockBuilders(JavaCodeUnit codeUnit);

Set<ReferencedClassObject> createReferencedClassObjectsFor(JavaCodeUnit codeUnit);

JavaClass resolveClass(String fullyQualifiedClassName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -1480,7 +1480,7 @@ void completeAnnotations(ImportContext context) {

JavaClassDependencies completeFrom(ImportContext context) {
completeComponentType(context);
members.completeAccessesFrom(context);
members.completeFrom(context);
javaClassDependencies = new JavaClassDependencies(this);
return javaClassDependencies;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,9 +318,9 @@ void completeAnnotations(ImportContext context) {
}
}

void completeAccessesFrom(ImportContext context) {
void completeFrom(ImportContext context) {
for (JavaCodeUnit codeUnit : codeUnits) {
codeUnit.completeAccessesFrom(context);
codeUnit.completeFrom(context);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public abstract class JavaCodeUnit
private final Parameters parameters;
private final String fullName;
private final List<JavaTypeVariable<JavaCodeUnit>> typeParameters;
private final Set<ReferencedClassObject> referencedClassObjects;
private final Set<InstanceofCheck> instanceofChecks;

private Set<JavaFieldAccess> fieldAccesses = Collections.emptySet();
Expand All @@ -72,14 +71,14 @@ public abstract class JavaCodeUnit
private Set<JavaMethodReference> methodReferences = Collections.emptySet();
private Set<JavaConstructorReference> constructorReferences = Collections.emptySet();
private Set<TryCatchBlock> tryCatchBlocks = Collections.emptySet();
private Set<ReferencedClassObject> referencedClassObjects;

JavaCodeUnit(JavaCodeUnitBuilder<?, ?> builder) {
super(builder);
typeParameters = builder.getTypeParameters(this);
returnType = new ReturnType(this, builder);
parameters = new Parameters(this, builder);
fullName = formatMethod(getOwner().getName(), getName(), namesOf(getRawParameterTypes()));
referencedClassObjects = ImmutableSet.copyOf(builder.getReferencedClassObjects(this));
instanceofChecks = ImmutableSet.copyOf(builder.getInstanceofChecks(this));
}

Expand Down Expand Up @@ -271,7 +270,7 @@ public List<Set<JavaAnnotation<JavaParameter>>> getParameterAnnotations() {
return parameters.getAnnotations();
}

void completeAccessesFrom(ImportContext context) {
void completeFrom(ImportContext context) {
Set<TryCatchBlockBuilder> tryCatchBlockBuilders = context.createTryCatchBlockBuilders(this);
fieldAccesses = context.createFieldAccessesFor(this, tryCatchBlockBuilders);
methodCalls = context.createMethodCallsFor(this, tryCatchBlockBuilders);
Expand All @@ -281,6 +280,7 @@ void completeAccessesFrom(ImportContext context) {
tryCatchBlocks = tryCatchBlockBuilders.stream()
.map(builder -> builder.build(this, context))
.collect(toImmutableSet());
referencedClassObjects = context.createReferencedClassObjectsFor(this);
}

@ResolvesTypesViaReflection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ public final class ReferencedClassObject implements HasType, HasOwner<JavaCodeUn
private final JavaClass value;
private final int lineNumber;
private final SourceCodeLocation sourceCodeLocation;
private final boolean declaredInLambda;

private ReferencedClassObject(JavaCodeUnit owner, JavaClass value, int lineNumber) {
private ReferencedClassObject(JavaCodeUnit owner, JavaClass value, int lineNumber, boolean declaredInLambda) {
this.owner = checkNotNull(owner);
this.value = checkNotNull(value);
this.lineNumber = lineNumber;
sourceCodeLocation = SourceCodeLocation.of(owner.getOwner(), lineNumber);
this.declaredInLambda = declaredInLambda;
}

@Override
Expand Down Expand Up @@ -73,17 +75,23 @@ public SourceCodeLocation getSourceCodeLocation() {
return sourceCodeLocation;
}

@PublicAPI(usage = ACCESS)
public boolean isDeclaredInLambda() {
return declaredInLambda;
}

@Override
public String toString() {
return toStringHelper(this)
.add("owner", owner)
.add("value", value)
.add("sourceCodeLocation", sourceCodeLocation)
.add("declaredInLambda", declaredInLambda)
.toString();
}

static ReferencedClassObject from(JavaCodeUnit owner, JavaClass javaClass, int lineNumber) {
return new ReferencedClassObject(owner, javaClass, lineNumber);
static ReferencedClassObject from(JavaCodeUnit owner, JavaClass javaClass, int lineNumber, boolean declaredInLambda) {
return new ReferencedClassObject(owner, javaClass, lineNumber, declaredInLambda);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,17 +291,7 @@ public AccessType getAccessType() {
}

private static Supplier<JavaCodeUnit> createOriginSupplier(CodeUnit origin, ImportedClasses classes) {
return Suppliers.memoize(() -> Factory.getOrigin(origin, classes));
}

private static JavaCodeUnit getOrigin(CodeUnit rawOrigin, ImportedClasses classes) {
for (JavaCodeUnit method : classes.getOrResolve(rawOrigin.getDeclaringClassName()).getCodeUnits()) {
if (rawOrigin.is(method)) {
return method;
}
}
throw new IllegalStateException("Never found a " + JavaCodeUnit.class.getSimpleName() +
" that matches supposed origin " + rawOrigin);
return Suppliers.memoize(() -> origin.resolveFrom(classes));
}

private static List<JavaClass> getArgumentTypesFrom(String descriptor, ImportedClasses classes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class ClassFileImportRecord {
private final Set<RawAccessRecord> rawConstructorCallRecords = new HashSet<>();
private final Set<RawAccessRecord> rawMethodReferenceRecords = new HashSet<>();
private final Set<RawAccessRecord> rawConstructorReferenceRecords = new HashSet<>();
private final Set<RawReferencedClassObject> rawReferencedClassObjects = new HashSet<>();
private final SyntheticAccessRecorder syntheticLambdaAccessRecorder = createSyntheticLambdaAccessRecorder();
private final SyntheticAccessRecorder syntheticPrivateAccessRecorder = createSyntheticPrivateAccessRecorder();

Expand Down Expand Up @@ -246,6 +247,10 @@ void registerLambdaInvocation(RawAccessRecord record) {
syntheticLambdaAccessRecorder.registerSyntheticMethodInvocation(record);
}

void registerReferencedClassObject(RawReferencedClassObject referencedClassObject) {
rawReferencedClassObjects.add(referencedClassObject);
}

void forEachRawFieldAccessRecord(Consumer<RawAccessRecord.ForField> doWithRecord) {
fixSyntheticOrigins(
rawFieldAccessRecords, COPY_RAW_FIELD_ACCESS_RECORD,
Expand Down Expand Up @@ -281,6 +286,13 @@ void forEachRawConstructorReferenceRecord(Consumer<RawAccessRecord> doWithRecord
).forEach(doWithRecord);
}

void forEachRawReferencedClassObject(Consumer<RawReferencedClassObject> doWithReferencedClassObject) {
fixSyntheticOrigins(
rawReferencedClassObjects, COPY_RAW_REFERENCED_CLASS_OBJECT,
syntheticLambdaAccessRecorder
).forEach(doWithReferencedClassObject);
}

private <ACCESS extends RawCodeUnitDependency<?>> Stream<ACCESS> fixSyntheticOrigins(
Set<ACCESS> rawAccessRecordsIncludingSyntheticAccesses,
Function<ACCESS, ? extends RawCodeUnitDependencyBuilder<ACCESS, ?>> createAccessWithNewOrigin,
Expand Down Expand Up @@ -309,6 +321,9 @@ Map<String, JavaClass> getClasses() {
access -> copyInto(new RawAccessRecord.ForField.Builder(), access)
.withAccessType(access.accessType);

private static final Function<RawReferencedClassObject, RawReferencedClassObject.Builder> COPY_RAW_REFERENCED_CLASS_OBJECT =
referencedClassObject -> copyInto(new RawReferencedClassObject.Builder(), referencedClassObject);

private static <TARGET, BUILDER extends RawCodeUnitDependencyBuilder<?, TARGET>> BUILDER copyInto(BUILDER builder, RawCodeUnitDependency<TARGET> referencedClassObject) {
builder
.withOrigin(referencedClassObject.getOrigin())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,16 @@ public void handleLambdaInstruction(String owner, String name, String desc) {
importRecord.registerLambdaInvocation(filled(new RawAccessRecord.Builder(), target).build());
}

@Override
public void handleReferencedClassObject(JavaClassDescriptor type, int lineNumber) {
importRecord.registerReferencedClassObject(new RawReferencedClassObject.Builder()
.withOrigin(codeUnit)
.withTarget(type)
.withLineNumber(lineNumber)
.withDeclaredInLambda(false)
.build());
}

@Override
public void handleTryCatchBlock(Label start, Label end, Label handler, JavaClassDescriptor throwableType) {
LOG.trace("Found try/catch block between {} and {} for throwable {}", start, end, throwableType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import com.tngtech.archunit.core.domain.JavaStaticInitializer;
import com.tngtech.archunit.core.domain.JavaType;
import com.tngtech.archunit.core.domain.JavaTypeVariable;
import com.tngtech.archunit.core.domain.ReferencedClassObject;
import com.tngtech.archunit.core.importer.AccessRecord.FieldAccessRecord;
import com.tngtech.archunit.core.importer.DomainBuilders.JavaClassTypeParametersBuilder;
import com.tngtech.archunit.core.importer.DomainBuilders.JavaConstructorCallBuilder;
Expand All @@ -72,6 +73,7 @@
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.completeMembers;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.completeTypeParameters;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createJavaClasses;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createReferencedClassObject;
import static com.tngtech.archunit.core.importer.DomainBuilders.BuilderWithBuildParameter.BuildFinisher.build;
import static com.tngtech.archunit.core.importer.DomainBuilders.buildAnnotations;
import static com.tngtech.archunit.core.importer.JavaClassDescriptorImporter.isLambdaMethodName;
Expand All @@ -88,6 +90,7 @@ class ClassGraphCreator implements ImportContext {
private final SetMultimap<JavaCodeUnit, AccessRecord<ConstructorCallTarget>> processedConstructorCallRecords = HashMultimap.create();
private final SetMultimap<JavaCodeUnit, AccessRecord<MethodReferenceTarget>> processedMethodReferenceRecords = HashMultimap.create();
private final SetMultimap<JavaCodeUnit, AccessRecord<ConstructorReferenceTarget>> processedConstructorReferenceRecords = HashMultimap.create();
private final SetMultimap<JavaCodeUnit, ReferencedClassObject> processedReferencedClassObjects = HashMultimap.create();

ClassGraphCreator(ClassFileImportRecord importRecord, DependencyResolutionProcess dependencyResolutionProcess, ClassResolver classResolver) {
this.importRecord = importRecord;
Expand All @@ -98,7 +101,7 @@ class ClassGraphCreator implements ImportContext {
JavaClasses complete() {
dependencyResolutionProcess.resolve(classes);
completeClasses();
completeAccesses();
completeCodeUnitDependencies();
return createJavaClasses(classes.getDirectlyImported(), classes.getAllWithOuterClassesSortedBeforeInnerClasses(), this);
}

Expand All @@ -114,7 +117,7 @@ private void completeClasses() {
}
}

private void completeAccesses() {
private void completeCodeUnitDependencies() {
importRecord.forEachRawFieldAccessRecord(record ->
tryProcess(record, AccessRecord.Factory.forFieldAccessRecord(), processedFieldAccessRecords));
importRecord.forEachRawMethodCallRecord(record ->
Expand All @@ -125,6 +128,7 @@ private void completeAccesses() {
tryProcess(record, AccessRecord.Factory.forMethodReferenceRecord(), processedMethodReferenceRecords));
importRecord.forEachRawConstructorReferenceRecord(record ->
tryProcess(record, AccessRecord.Factory.forConstructorReferenceRecord(), processedConstructorReferenceRecords));
importRecord.forEachRawReferencedClassObject(this::processReferencedClassObject);
}

private <T extends AccessRecord<?>, B extends RawAccessRecord> void tryProcess(
Expand All @@ -136,6 +140,17 @@ private <T extends AccessRecord<?>, B extends RawAccessRecord> void tryProcess(
processedAccessRecords.put(processed.getOrigin(), processed);
}

private void processReferencedClassObject(RawReferencedClassObject rawReferencedClassObject) {
JavaCodeUnit origin = rawReferencedClassObject.getOrigin().resolveFrom(classes);
ReferencedClassObject referencedClassObject = createReferencedClassObject(
origin,
classes.getOrResolve(rawReferencedClassObject.getClassName()),
rawReferencedClassObject.getLineNumber(),
rawReferencedClassObject.isDeclaredInLambda()
);
processedReferencedClassObjects.put(origin, referencedClassObject);
}

@Override
public Set<JavaFieldAccess> createFieldAccessesFor(JavaCodeUnit codeUnit, Set<TryCatchBlockBuilder> tryCatchBlockBuilders) {
ImmutableSet.Builder<JavaFieldAccess> result = ImmutableSet.builder();
Expand Down Expand Up @@ -334,6 +349,11 @@ public Set<TryCatchBlockBuilder> createTryCatchBlockBuilders(JavaCodeUnit codeUn
return importRecord.getTryCatchBlockBuildersFor(codeUnit);
}

@Override
public Set<ReferencedClassObject> createReferencedClassObjectsFor(JavaCodeUnit codeUnit) {
return ImmutableSet.copyOf(processedReferencedClassObjects.get(codeUnit));
}

@Override
public JavaClass resolveClass(String fullyQualifiedClassName) {
return classes.getOrResolve(fullyQualifiedClassName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
import com.tngtech.archunit.core.domain.JavaType;
import com.tngtech.archunit.core.domain.JavaTypeVariable;
import com.tngtech.archunit.core.domain.JavaWildcardType;
import com.tngtech.archunit.core.domain.ReferencedClassObject;
import com.tngtech.archunit.core.domain.Source;
import com.tngtech.archunit.core.domain.SourceCodeLocation;
import com.tngtech.archunit.core.domain.ThrowsClause;
Expand All @@ -82,7 +81,6 @@
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.completeTypeVariable;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createGenericArrayType;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createInstanceofCheck;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createReferencedClassObject;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createSource;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createThrowsClause;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createTryCatchBlock;
Expand Down Expand Up @@ -250,7 +248,6 @@ public abstract static class JavaCodeUnitBuilder<OUTPUT, SELF extends JavaCodeUn
private SetMultimap<Integer, JavaAnnotationBuilder> parameterAnnotationsByIndex;
private JavaCodeUnitTypeParametersBuilder typeParametersBuilder;
private List<JavaClassDescriptor> throwsDeclarations;
private final Set<RawReferencedClassObject> rawReferencedClassObjects = new HashSet<>();
private final List<RawInstanceofCheck> instanceOfChecks = new ArrayList<>();

private JavaCodeUnitBuilder() {
Expand Down Expand Up @@ -283,11 +280,6 @@ SELF withThrowsClause(List<JavaClassDescriptor> throwsDeclarations) {
return self();
}

SELF addReferencedClassObject(RawReferencedClassObject rawReferencedClassObject) {
rawReferencedClassObjects.add(rawReferencedClassObject);
return self();
}

SELF addInstanceOfCheck(RawInstanceofCheck rawInstanceOfChecks) {
this.instanceOfChecks.add(rawInstanceOfChecks);
return self();
Expand Down Expand Up @@ -339,14 +331,6 @@ public <CODE_UNIT extends JavaCodeUnit> ThrowsClause<CODE_UNIT> getThrowsClause(
return createThrowsClause(codeUnit, asJavaClasses(this.throwsDeclarations));
}

public Set<ReferencedClassObject> getReferencedClassObjects(JavaCodeUnit codeUnit) {
ImmutableSet.Builder<ReferencedClassObject> result = ImmutableSet.builder();
for (RawReferencedClassObject rawReferencedClassObject : this.rawReferencedClassObjects) {
result.add(createReferencedClassObject(codeUnit, get(rawReferencedClassObject.getClassName()), rawReferencedClassObject.getLineNumber()));
}
return result.build();
}

public Set<InstanceofCheck> getInstanceofChecks(JavaCodeUnit codeUnit) {
ImmutableSet.Builder<InstanceofCheck> result = ImmutableSet.builder();
for (RawInstanceofCheck instanceOfCheck : this.instanceOfChecks) {
Expand Down Expand Up @@ -733,7 +717,7 @@ public List<JavaType> getUpperBounds(Iterable<? extends JavaTypeVariable<?>> all
}
}

private static abstract class AbstractTypeParametersBuilder<OWNER extends HasDescription> {
private abstract static class AbstractTypeParametersBuilder<OWNER extends HasDescription> {
private final List<JavaTypeParameterBuilder<OWNER>> typeParameterBuilders;

AbstractTypeParametersBuilder(List<JavaTypeParameterBuilder<OWNER>> typeParameterBuilders) {
Expand Down Expand Up @@ -1096,7 +1080,7 @@ JavaConstructorReference build() {
}

@Internal
public static abstract class AccessTargetBuilder<MEMBER extends JavaMember, TARGET extends AccessTarget, SELF extends AccessTargetBuilder<MEMBER, TARGET, SELF>> {
public abstract static class AccessTargetBuilder<MEMBER extends JavaMember, TARGET extends AccessTarget, SELF extends AccessTargetBuilder<MEMBER, TARGET, SELF>> {
private final Function<SELF, TARGET> createTarget;

private JavaClass owner;
Expand Down
Loading

0 comments on commit 4f5698b

Please sign in to comment.