Skip to content

Commit

Permalink
support type cast detection TNG#710
Browse files Browse the repository at this point in the history
Detect *checkcast* asm instructions as TypeCast (similar to
InstanceofCheck)
Implicit checkcast instructions generated by compiler due to method
invocations are ignored

Signed-off-by: Allan Jones <[email protected]>
  • Loading branch information
ratoaq2 committed Mar 12, 2023
1 parent 4ef8299 commit bf83238
Show file tree
Hide file tree
Showing 34 changed files with 691 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ Method <com.tngtech.archunit.example.layers.persistence.first.dao.jpa.SomeJpa.fi
Method <com.tngtech.archunit.example.layers.persistence.second.dao.OtherDao.getEntityManager()> has return type <javax.persistence.EntityManager> in (OtherDao.java:0)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.findById(long)> calls method <javax.persistence.EntityManager.find(java.lang.Class, java.lang.Object)> in (OtherJpa.java:19)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.getEntityManager()> has return type <javax.persistence.EntityManager> in (OtherJpa.java:0)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> calls method <javax.persistence.EntityManager.unwrap(java.lang.Class)> in (OtherJpa.java:24)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> calls method <javax.persistence.EntityManager.unwrap(java.lang.Class)> in (OtherJpa.java:24)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> casts <com.tngtech.archunit.example.layers.service.ProxiedConnection> in (OtherJpa.java:27)
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ Method <com.tngtech.archunit.example.layers.persistence.first.dao.jpa.SomeJpa.fi
Method <com.tngtech.archunit.example.layers.persistence.second.dao.OtherDao.getEntityManager()> has return type <javax.persistence.EntityManager> in (OtherDao.java:0)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.findById(long)> calls method <javax.persistence.EntityManager.find(java.lang.Class, java.lang.Object)> in (OtherJpa.java:19)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.getEntityManager()> has return type <javax.persistence.EntityManager> in (OtherJpa.java:0)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> calls method <javax.persistence.EntityManager.unwrap(java.lang.Class)> in (OtherJpa.java:24)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> calls method <javax.persistence.EntityManager.unwrap(java.lang.Class)> in (OtherJpa.java:24)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> casts <com.tngtech.archunit.example.layers.service.ProxiedConnection> in (OtherJpa.java:27)
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ Method <com.tngtech.archunit.example.layers.persistence.first.dao.jpa.SomeJpa.fi
Method <com.tngtech.archunit.example.layers.persistence.second.dao.OtherDao.getEntityManager()> has return type <javax.persistence.EntityManager> in (OtherDao.java:0)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.findById(long)> calls method <javax.persistence.EntityManager.find(java.lang.Class, java.lang.Object)> in (OtherJpa.java:19)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.getEntityManager()> has return type <javax.persistence.EntityManager> in (OtherJpa.java:0)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> calls method <javax.persistence.EntityManager.unwrap(java.lang.Class)> in (OtherJpa.java:24)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> calls method <javax.persistence.EntityManager.unwrap(java.lang.Class)> in (OtherJpa.java:24)
Method <com.tngtech.archunit.example.layers.persistence.second.dao.jpa.OtherJpa.testConnection()> casts <com.tngtech.archunit.example.layers.service.ProxiedConnection> in (OtherJpa.java:27)
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,9 @@ Stream<DynamicTest> FrozenRulesTest() {
.by(method(OtherJpa.class, "testConnection")
.checkingInstanceOf(ProxiedConnection.class)
.inLine(26))
.by(method(OtherJpa.class, "testConnection")
.casting(ProxiedConnection.class)
.inLine(27))

.ofRule("no classes should depend on classes that are assignable to javax.persistence.EntityManager")
.by(callFromMethod(ServiceViolatingDaoRules.class, "illegallyUseEntityManager").
Expand Down Expand Up @@ -847,6 +850,9 @@ Stream<DynamicTest> LayerDependencyRulesTest() {
.by(method(OtherJpa.class, "testConnection")
.checkingInstanceOf(ProxiedConnection.class)
.inLine(26))
.by(method(OtherJpa.class, "testConnection")
.casting(ProxiedConnection.class)
.inLine(27))

.ofRule("classes that reside in a package '..service..' should " +
"only have dependent classes that reside in any package ['..controller..', '..service..']")
Expand All @@ -866,6 +872,9 @@ Stream<DynamicTest> LayerDependencyRulesTest() {
.by(method(OtherJpa.class, "testConnection")
.checkingInstanceOf(ProxiedConnection.class)
.inLine(26))
.by(method(OtherJpa.class, "testConnection")
.casting(ProxiedConnection.class)
.inLine(27))

.ofRule("classes that reside in a package '..service..' should "
+ "only depend on classes that reside in any package ['..service..', '..persistence..', 'java..', 'javax..']")
Expand Down Expand Up @@ -972,6 +981,9 @@ Stream<DynamicTest> LayeredArchitectureTest() {
.toMethod(ProxiedConnection.class, "refresh")
.inLine(27)
.asDependency())
.by(method(OtherJpa.class, "testConnection")
.casting(ProxiedConnection.class)
.inLine(27))

.by(typeParameter(ServiceHelper.class, "TYPE_PARAMETER_VIOLATING_LAYER_RULE").dependingOn(SomeUtility.class))
.by(typeParameter(ServiceHelper.class, "ANOTHER_TYPE_PARAMETER_VIOLATING_LAYER_RULE").dependingOn(SomeEnum.class))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,10 @@ public AddsLineNumber checkingInstanceOf(Class<?> target) {
return new AddsLineNumber(owner, getOriginName(), "checks instanceof", target);
}

public AddsLineNumber casting(Class<?> target) {
return new AddsLineNumber(owner, getOriginName(), "casts", target);
}

public AddsLineNumber referencingClassObject(Class<?> target) {
return new AddsLineNumber(owner, getOriginName(), "references class object", target);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ static Set<Dependency> tryCreateFromInstanceofCheck(InstanceofCheck instanceofCh
instanceofCheck.getRawType(), instanceofCheck.getSourceCodeLocation());
}

static Set<Dependency> tryCreateFromTypeCast(TypeCast typeCast) {
return tryCreateDependency(
typeCast.getOwner(), "casts",
typeCast.getRawType(), typeCast.getSourceCodeLocation());
}

static Set<Dependency> tryCreateFromReferencedClassObject(ReferencedClassObject referencedClassObject) {
return tryCreateDependency(
referencedClassObject.getOwner(), "references class object",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ public static InstanceofCheck createInstanceofCheck(JavaCodeUnit codeUnit, JavaC
return InstanceofCheck.from(codeUnit, type, lineNumber, declaredInLambda);
}

public static TypeCast createTypeCast(JavaCodeUnit codeUnit, JavaClass type, int lineNumber, boolean declaredInLambda) {
return TypeCast.from(codeUnit, type, lineNumber, declaredInLambda);
}

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

Set<InstanceofCheck> createInstanceofChecksFor(JavaCodeUnit codeUnit);

Set<TypeCast> createTypeCastsFor(JavaCodeUnit codeUnit);

JavaClass resolveClass(String fullyQualifiedClassName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,11 @@ public Set<InstanceofCheck> getInstanceofChecks() {
return members.getInstanceofChecks();
}

@PublicAPI(usage = ACCESS)
public Set<TypeCast> getTypeCasts() {
return members.getTypeCasts();
}

@PublicAPI(usage = ACCESS)
public Set<ReferencedClassObject> getReferencedClassObjects() {
return members.getReferencedClassObjects();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ private Supplier<Set<Dependency>> createDirectDependenciesFromClassSupplier() {
throwsDeclarationDependenciesFromSelf(),
annotationDependenciesFromSelf(),
instanceofCheckDependenciesFromSelf(),
typeCastDependenciesFromSelf(),
referencedClassObjectDependenciesFromSelf(),
typeParameterDependenciesFromSelf()
).collect(toImmutableSet())
Expand Down Expand Up @@ -182,6 +183,11 @@ private Stream<Dependency> instanceofCheckDependenciesFromSelf() {
.flatMap(instanceofCheck -> Dependency.tryCreateFromInstanceofCheck(instanceofCheck).stream());
}

private Stream<Dependency> typeCastDependenciesFromSelf() {
return javaClass.getTypeCasts().stream()
.flatMap(typeCast -> Dependency.tryCreateFromTypeCast(typeCast).stream());
}

private Stream<Dependency> referencedClassObjectDependenciesFromSelf() {
return javaClass.getReferencedClassObjects().stream()
.flatMap(referencedClassObject -> Dependency.tryCreateFromReferencedClassObject(referencedClassObject).stream());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,14 @@ Set<InstanceofCheck> getInstanceofChecks() {
return result.build();
}

Set<TypeCast> getTypeCasts() {
ImmutableSet.Builder<TypeCast> result = ImmutableSet.builder();
for (JavaCodeUnit codeUnit : codeUnits) {
result.addAll(codeUnit.getTypeCasts());
}
return result.build();
}

Set<ReferencedClassObject> getReferencedClassObjects() {
ImmutableSet.Builder<ReferencedClassObject> result = ImmutableSet.builder();
for (JavaCodeUnit codeUnit : codeUnits) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public abstract class JavaCodeUnit
private Set<TryCatchBlock> tryCatchBlocks = Collections.emptySet();
private Set<ReferencedClassObject> referencedClassObjects;
private Set<InstanceofCheck> instanceofChecks;
private Set<TypeCast> typeCasts;

JavaCodeUnit(JavaCodeUnitBuilder<?, ?> builder) {
super(builder);
Expand Down Expand Up @@ -207,6 +208,11 @@ public Set<InstanceofCheck> getInstanceofChecks() {
return instanceofChecks;
}

@PublicAPI(usage = ACCESS)
public Set<TypeCast> getTypeCasts() {
return typeCasts;
}

@PublicAPI(usage = ACCESS)
public Set<TryCatchBlock> getTryCatchBlocks() {
return tryCatchBlocks;
Expand Down Expand Up @@ -281,6 +287,7 @@ void completeFrom(ImportContext context) {
.collect(toImmutableSet());
referencedClassObjects = context.createReferencedClassObjectsFor(this);
instanceofChecks = context.createInstanceofChecksFor(this);
typeCasts = context.createTypeCastsFor(this);
}

@ResolvesTypesViaReflection
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright 2014-2023 TNG Technology Consulting GmbH
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.tngtech.archunit.core.domain;

import com.tngtech.archunit.PublicAPI;
import com.tngtech.archunit.core.domain.properties.HasOwner;
import com.tngtech.archunit.core.domain.properties.HasSourceCodeLocation;
import com.tngtech.archunit.core.domain.properties.HasType;

import static com.google.common.base.MoreObjects.toStringHelper;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;

@PublicAPI(usage = ACCESS)
public final class TypeCast implements HasType, HasOwner<JavaCodeUnit>, HasSourceCodeLocation {

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

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

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

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

@Override
@PublicAPI(usage = ACCESS)
public JavaCodeUnit getOwner() {
return owner;
}

@PublicAPI(usage = ACCESS)
public int getLineNumber() {
return lineNumber;
}

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

@Override
public SourceCodeLocation getSourceCodeLocation() {
return sourceCodeLocation;
}

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

static TypeCast from(JavaCodeUnit owner, JavaClass type, int lineNumber, boolean declaredInLambda) {
return new TypeCast(owner, type, lineNumber, declaredInLambda);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class ClassFileImportRecord {
private final Set<RawAccessRecord> rawConstructorReferenceRecords = new HashSet<>();
private final Set<RawReferencedClassObject> rawReferencedClassObjects = new HashSet<>();
private final Set<RawInstanceofCheck> rawInstanceofChecks = new HashSet<>();
private final Set<RawTypeCast> rawTypeCasts = new HashSet<>();
private final SyntheticAccessRecorder syntheticLambdaAccessRecorder = createSyntheticLambdaAccessRecorder();
private final SyntheticAccessRecorder syntheticPrivateAccessRecorder = createSyntheticPrivateAccessRecorder();

Expand Down Expand Up @@ -256,6 +257,10 @@ void registerInstanceofCheck(RawInstanceofCheck instanceofCheck) {
rawInstanceofChecks.add(instanceofCheck);
}

void registerTypeCast(RawTypeCast typeCast) {
rawTypeCasts.add(typeCast);
}

void forEachRawFieldAccessRecord(Consumer<RawAccessRecord.ForField> doWithRecord) {
fixSyntheticOrigins(
rawFieldAccessRecords, COPY_RAW_FIELD_ACCESS_RECORD,
Expand Down Expand Up @@ -305,6 +310,13 @@ void forEachRawInstanceofCheck(Consumer<RawInstanceofCheck> doWithInstanceofChec
).forEach(doWithInstanceofCheck);
}

void forEachRawTypeCast(Consumer<RawTypeCast> doWithTypeCast) {
fixSyntheticOrigins(
rawTypeCasts, COPY_RAW_TYPE_CAST,
syntheticLambdaAccessRecorder
).forEach(doWithTypeCast);
}

private <ACCESS extends RawCodeUnitDependency<?>> Stream<ACCESS> fixSyntheticOrigins(
Set<ACCESS> rawAccessRecordsIncludingSyntheticAccesses,
Function<ACCESS, ? extends RawCodeUnitDependencyBuilder<ACCESS, ?>> createAccessWithNewOrigin,
Expand Down Expand Up @@ -339,6 +351,9 @@ Map<String, JavaClass> getClasses() {
private static final Function<RawInstanceofCheck, RawInstanceofCheck.Builder> COPY_RAW_INSTANCEOF_CHECK =
instanceofCheck -> copyInto(new RawInstanceofCheck.Builder(), instanceofCheck);

private static final Function<RawTypeCast, RawTypeCast.Builder> COPY_RAW_TYPE_CAST =
typeCast -> copyInto(new RawTypeCast.Builder(), typeCast);

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 @@ -189,6 +189,11 @@ public void onDeclaredInstanceofCheck(String typeName) {
dependencyResolutionProcess.registerAccessToType(typeName);
}

@Override
public void onDeclaredTypeCast(String typeName) {
dependencyResolutionProcess.registerAccessToType(typeName);
}

@Override
public void onDeclaredThrowsClause(Collection<String> exceptionTypeNames) {
dependencyResolutionProcess.registerMemberTypes(exceptionTypeNames);
Expand All @@ -208,6 +213,7 @@ private static class RecordAccessHandler implements AccessHandler, TryCatchBlock
private CodeUnit codeUnit;
private int lineNumber;
private final TryCatchRecorder tryCatchRecorder = new TryCatchRecorder(this);
private final TypeCastRecorder typeCastRecorder = new TypeCastRecorder();

private RecordAccessHandler(ClassFileImportRecord importRecord, DependencyResolutionProcess dependencyResolutionProcess) {
this.importRecord = importRecord;
Expand All @@ -230,6 +236,11 @@ public void onLabel(Label label) {
tryCatchRecorder.onEncounteredLabel(label);
}

@Override
public void handleVariableInstruction(int opcode, int varIndex) {
typeCastRecorder.setImplicit(false);
}

@Override
public void handleFieldInstruction(int opcode, String owner, String name, String desc) {
AccessType accessType = AccessType.forOpCode(opcode);
Expand All @@ -240,6 +251,7 @@ public void handleFieldInstruction(int opcode, String owner, String name, String
.build();
importRecord.registerFieldAccess(accessRecord);
tryCatchRecorder.registerAccess(accessRecord);
typeCastRecorder.setImplicit(false);
dependencyResolutionProcess.registerAccessToType(target.owner.getFullyQualifiedClassName());
}

Expand All @@ -254,6 +266,7 @@ public void handleMethodInstruction(String owner, String name, String desc) {
importRecord.registerMethodCall(accessRecord);
}
tryCatchRecorder.registerAccess(accessRecord);
typeCastRecorder.setImplicit(true);
dependencyResolutionProcess.registerAccessToType(target.owner.getFullyQualifiedClassName());
}

Expand Down Expand Up @@ -297,6 +310,18 @@ public void handleInstanceofCheck(JavaClassDescriptor instanceOfCheckType, int l
.build());
}

@Override
public void handleTypeCast(JavaClassDescriptor typeCastType, int lineNumber) {
if (!typeCastRecorder.isImplicit()) {
importRecord.registerTypeCast(new RawTypeCast.Builder()
.withOrigin(codeUnit)
.withTarget(typeCastType)
.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
Loading

0 comments on commit bf83238

Please sign in to comment.