Skip to content

Commit

Permalink
fix instanceof checks 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 instanceof checks. 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 `RawInstanceofCheck` that we only applied to `RawAccessRecord` before. This also means, that we can't simply add `RawInstanceofCheck` 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 4f5698b commit ef153bf
Show file tree
Hide file tree
Showing 17 changed files with 305 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ public static <CODE_UNIT extends JavaCodeUnit> ThrowsClause<CODE_UNIT> createThr
return ThrowsClause.from(codeUnit, types);
}

public static InstanceofCheck createInstanceofCheck(JavaCodeUnit codeUnit, JavaClass target, int lineNumber) {
return InstanceofCheck.from(codeUnit, target, lineNumber);
public static InstanceofCheck createInstanceofCheck(JavaCodeUnit codeUnit, JavaClass type, int lineNumber, boolean declaredInLambda) {
return InstanceofCheck.from(codeUnit, type, lineNumber, declaredInLambda);
}

public static <OWNER extends HasDescription> JavaTypeVariable<OWNER> createTypeVariable(String name, OWNER owner, JavaClass erasure) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,7 @@ public interface ImportContext {

Set<ReferencedClassObject> createReferencedClassObjectsFor(JavaCodeUnit codeUnit);

Set<InstanceofCheck> createInstanceofChecksFor(JavaCodeUnit codeUnit);

JavaClass resolveClass(String fullyQualifiedClassName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,29 @@
public final class InstanceofCheck implements HasType, HasOwner<JavaCodeUnit>, HasSourceCodeLocation {

private final JavaCodeUnit owner;
private final JavaClass target;
private final JavaClass type;
private final int lineNumber;
private final boolean declaredInLambda;
private final SourceCodeLocation sourceCodeLocation;

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

@Override
@PublicAPI(usage = ACCESS)
public JavaClass getRawType() {
return target;
return type;
}

@Override
@PublicAPI(usage = ACCESS)
public JavaType getType() {
return target;
return type;
}

@Override
Expand All @@ -62,6 +64,11 @@ public int getLineNumber() {
return lineNumber;
}

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

@Override
public SourceCodeLocation getSourceCodeLocation() {
return sourceCodeLocation;
Expand All @@ -71,12 +78,13 @@ public SourceCodeLocation getSourceCodeLocation() {
public String toString() {
return toStringHelper(this)
.add("owner", owner)
.add("target", target)
.add("type", type)
.add("lineNumber", lineNumber)
.add("declaredInLambda", declaredInLambda)
.toString();
}

static InstanceofCheck from(JavaCodeUnit owner, JavaClass target, int lineNumber) {
return new InstanceofCheck(owner, target, lineNumber);
static InstanceofCheck from(JavaCodeUnit owner, JavaClass type, int lineNumber, boolean declaredInLambda) {
return new InstanceofCheck(owner, type, lineNumber, declaredInLambda);
}
}
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<InstanceofCheck> instanceofChecks;

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

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()));
instanceofChecks = ImmutableSet.copyOf(builder.getInstanceofChecks(this));
}

/**
Expand Down Expand Up @@ -281,6 +280,7 @@ void completeFrom(ImportContext context) {
.map(builder -> builder.build(this, context))
.collect(toImmutableSet());
referencedClassObjects = context.createReferencedClassObjectsFor(this);
instanceofChecks = context.createInstanceofChecksFor(this);
}

@ResolvesTypesViaReflection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class ClassFileImportRecord {
private final Set<RawAccessRecord> rawMethodReferenceRecords = new HashSet<>();
private final Set<RawAccessRecord> rawConstructorReferenceRecords = new HashSet<>();
private final Set<RawReferencedClassObject> rawReferencedClassObjects = new HashSet<>();
private final Set<RawInstanceofCheck> rawInstanceofChecks = new HashSet<>();
private final SyntheticAccessRecorder syntheticLambdaAccessRecorder = createSyntheticLambdaAccessRecorder();
private final SyntheticAccessRecorder syntheticPrivateAccessRecorder = createSyntheticPrivateAccessRecorder();

Expand Down Expand Up @@ -251,6 +252,10 @@ void registerReferencedClassObject(RawReferencedClassObject referencedClassObjec
rawReferencedClassObjects.add(referencedClassObject);
}

void registerInstanceofCheck(RawInstanceofCheck instanceofCheck) {
rawInstanceofChecks.add(instanceofCheck);
}

void forEachRawFieldAccessRecord(Consumer<RawAccessRecord.ForField> doWithRecord) {
fixSyntheticOrigins(
rawFieldAccessRecords, COPY_RAW_FIELD_ACCESS_RECORD,
Expand Down Expand Up @@ -293,6 +298,13 @@ void forEachRawReferencedClassObject(Consumer<RawReferencedClassObject> doWithRe
).forEach(doWithReferencedClassObject);
}

void forEachRawInstanceofCheck(Consumer<RawInstanceofCheck> doWithInstanceofCheck) {
fixSyntheticOrigins(
rawInstanceofChecks, COPY_RAW_INSTANCEOF_CHECK,
syntheticLambdaAccessRecorder
).forEach(doWithInstanceofCheck);
}

private <ACCESS extends RawCodeUnitDependency<?>> Stream<ACCESS> fixSyntheticOrigins(
Set<ACCESS> rawAccessRecordsIncludingSyntheticAccesses,
Function<ACCESS, ? extends RawCodeUnitDependencyBuilder<ACCESS, ?>> createAccessWithNewOrigin,
Expand Down Expand Up @@ -324,6 +336,9 @@ Map<String, JavaClass> getClasses() {
private static final Function<RawReferencedClassObject, RawReferencedClassObject.Builder> COPY_RAW_REFERENCED_CLASS_OBJECT =
referencedClassObject -> copyInto(new RawReferencedClassObject.Builder(), referencedClassObject);

private static final Function<RawInstanceofCheck, RawInstanceofCheck.Builder> COPY_RAW_INSTANCEOF_CHECK =
instanceofCheck -> copyInto(new RawInstanceofCheck.Builder(), instanceofCheck);

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 @@ -287,6 +287,16 @@ public void handleReferencedClassObject(JavaClassDescriptor type, int lineNumber
.build());
}

@Override
public void handleInstanceofCheck(JavaClassDescriptor instanceOfCheckType, int lineNumber) {
importRecord.registerInstanceofCheck(new RawInstanceofCheck.Builder()
.withOrigin(codeUnit)
.withTarget(instanceOfCheckType)
.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 @@ -34,6 +34,7 @@
import com.tngtech.archunit.core.domain.AccessTarget.MethodCallTarget;
import com.tngtech.archunit.core.domain.AccessTarget.MethodReferenceTarget;
import com.tngtech.archunit.core.domain.ImportContext;
import com.tngtech.archunit.core.domain.InstanceofCheck;
import com.tngtech.archunit.core.domain.JavaAccess;
import com.tngtech.archunit.core.domain.JavaAnnotation;
import com.tngtech.archunit.core.domain.JavaClass;
Expand Down Expand Up @@ -72,6 +73,7 @@
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.completeGenericSuperclass;
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.createInstanceofCheck;
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;
Expand All @@ -91,6 +93,7 @@ class ClassGraphCreator implements ImportContext {
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();
private final SetMultimap<JavaCodeUnit, InstanceofCheck> processedInstanceofChecks = HashMultimap.create();

ClassGraphCreator(ClassFileImportRecord importRecord, DependencyResolutionProcess dependencyResolutionProcess, ClassResolver classResolver) {
this.importRecord = importRecord;
Expand Down Expand Up @@ -129,6 +132,7 @@ private void completeCodeUnitDependencies() {
importRecord.forEachRawConstructorReferenceRecord(record ->
tryProcess(record, AccessRecord.Factory.forConstructorReferenceRecord(), processedConstructorReferenceRecords));
importRecord.forEachRawReferencedClassObject(this::processReferencedClassObject);
importRecord.forEachRawInstanceofCheck(this::processInstanceofCheck);
}

private <T extends AccessRecord<?>, B extends RawAccessRecord> void tryProcess(
Expand All @@ -151,6 +155,17 @@ private void processReferencedClassObject(RawReferencedClassObject rawReferenced
processedReferencedClassObjects.put(origin, referencedClassObject);
}

private void processInstanceofCheck(RawInstanceofCheck rawInstanceofCheck) {
JavaCodeUnit origin = rawInstanceofCheck.getOrigin().resolveFrom(classes);
InstanceofCheck instanceofCheck = createInstanceofCheck(
origin,
classes.getOrResolve(rawInstanceofCheck.getTarget().getFullyQualifiedClassName()),
rawInstanceofCheck.getLineNumber(),
rawInstanceofCheck.isDeclaredInLambda()
);
processedInstanceofChecks.put(origin, instanceofCheck);
}

@Override
public Set<JavaFieldAccess> createFieldAccessesFor(JavaCodeUnit codeUnit, Set<TryCatchBlockBuilder> tryCatchBlockBuilders) {
ImmutableSet.Builder<JavaFieldAccess> result = ImmutableSet.builder();
Expand Down Expand Up @@ -354,6 +369,11 @@ public Set<ReferencedClassObject> createReferencedClassObjectsFor(JavaCodeUnit c
return ImmutableSet.copyOf(processedReferencedClassObjects.get(codeUnit));
}

@Override
public Set<InstanceofCheck> createInstanceofChecksFor(JavaCodeUnit codeUnit) {
return ImmutableSet.copyOf(processedInstanceofChecks.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 @@ -44,7 +44,6 @@
import com.tngtech.archunit.core.domain.DomainObjectCreationContext;
import com.tngtech.archunit.core.domain.Formatters;
import com.tngtech.archunit.core.domain.ImportContext;
import com.tngtech.archunit.core.domain.InstanceofCheck;
import com.tngtech.archunit.core.domain.JavaAccess;
import com.tngtech.archunit.core.domain.JavaAnnotation;
import com.tngtech.archunit.core.domain.JavaClass;
Expand Down Expand Up @@ -80,7 +79,6 @@
import static com.google.common.collect.Sets.union;
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.createSource;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createThrowsClause;
import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createTryCatchBlock;
Expand Down Expand Up @@ -248,7 +246,6 @@ public abstract static class JavaCodeUnitBuilder<OUTPUT, SELF extends JavaCodeUn
private SetMultimap<Integer, JavaAnnotationBuilder> parameterAnnotationsByIndex;
private JavaCodeUnitTypeParametersBuilder typeParametersBuilder;
private List<JavaClassDescriptor> throwsDeclarations;
private final List<RawInstanceofCheck> instanceOfChecks = new ArrayList<>();

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

SELF addInstanceOfCheck(RawInstanceofCheck rawInstanceOfChecks) {
this.instanceOfChecks.add(rawInstanceOfChecks);
return self();
}

String getReturnTypeName() {
return rawReturnType.getFullyQualifiedClassName();
}
Expand Down Expand Up @@ -331,14 +323,6 @@ public <CODE_UNIT extends JavaCodeUnit> ThrowsClause<CODE_UNIT> getThrowsClause(
return createThrowsClause(codeUnit, asJavaClasses(this.throwsDeclarations));
}

public Set<InstanceofCheck> getInstanceofChecks(JavaCodeUnit codeUnit) {
ImmutableSet.Builder<InstanceofCheck> result = ImmutableSet.builder();
for (RawInstanceofCheck instanceOfCheck : this.instanceOfChecks) {
result.add(createInstanceofCheck(codeUnit, get(instanceOfCheck.getTarget().getFullyQualifiedClassName()), instanceOfCheck.getLineNumber()));
}
return result.build();
}

private List<JavaClass> asJavaClasses(List<JavaClassDescriptor> descriptors) {
ImmutableList.Builder<JavaClass> result = ImmutableList.builder();
for (JavaClassDescriptor javaClassDescriptor : descriptors) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
import static com.tngtech.archunit.core.importer.JavaClassDescriptorImporter.isAsmMethodHandle;
import static com.tngtech.archunit.core.importer.JavaClassDescriptorImporter.isLambdaMetafactory;
import static com.tngtech.archunit.core.importer.JavaClassDescriptorImporter.isLambdaMethod;
import static com.tngtech.archunit.core.importer.RawInstanceofCheck.from;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toList;

Expand Down Expand Up @@ -401,7 +400,7 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc,
public void visitTypeInsn(int opcode, String type) {
if (opcode == Opcodes.INSTANCEOF) {
JavaClassDescriptor instanceOfCheckType = JavaClassDescriptorImporter.createFromAsmObjectTypeName(type);
codeUnitBuilder.addInstanceOfCheck(from(instanceOfCheckType, actualLineNumber));
accessHandler.handleInstanceofCheck(instanceOfCheckType, actualLineNumber);
declarationHandler.onDeclaredInstanceofCheck(instanceOfCheckType.getFullyQualifiedClassName());
}
}
Expand Down Expand Up @@ -537,6 +536,8 @@ interface AccessHandler {

void handleReferencedClassObject(JavaClassDescriptor type, int lineNumber);

void handleInstanceofCheck(JavaClassDescriptor instanceOfCheckType, int lineNumber);

void handleTryCatchBlock(Label start, Label end, Label handler, JavaClassDescriptor throwableType);

void handleTryFinallyBlock(Label start, Label end, Label handler);
Expand Down Expand Up @@ -577,6 +578,10 @@ public void handleLambdaInstruction(String owner, String name, String desc) {
public void handleReferencedClassObject(JavaClassDescriptor type, int lineNumber) {
}

@Override
public void handleInstanceofCheck(JavaClassDescriptor instanceOfCheckType, int lineNumber) {
}

@Override
public void handleTryCatchBlock(Label start, Label end, Label handler, JavaClassDescriptor throwableType) {
}
Expand Down
Loading

0 comments on commit ef153bf

Please sign in to comment.