support type cast detection #710

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]>
TNG · Mar 12, 2023 · 2730507 · 2730507
1 parent 4ef8299
commit 2730507
Show file tree

Hide file tree

Showing 34 changed files with 691 additions and 3 deletions.
diff --git a/...nit-example/example-junit4/src/test/resources/frozen/e77ec262-4d5c-4a7b-b41f-362a71e5a1d8 b/...nit-example/example-junit4/src/test/resources/frozen/e77ec262-4d5c-4a7b-b41f-362a71e5a1d8
@@ -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)
diff --git a/...nit-example/example-junit5/src/test/resources/frozen/e77ec262-4d5c-4a7b-b41f-362a71e5a1d8 b/...nit-example/example-junit5/src/test/resources/frozen/e77ec262-4d5c-4a7b-b41f-362a71e5a1d8
@@ -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)
diff --git a/...unit-example/example-plain/src/test/resources/frozen/e77ec262-4d5c-4a7b-b41f-362a71e5a1d8 b/...unit-example/example-plain/src/test/resources/frozen/e77ec262-4d5c-4a7b-b41f-362a71e5a1d8
@@ -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)
diff --git a/...egration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java b/...egration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java
@@ -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").
@@ -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..']")
@@ -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..']")
@@ -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))

diff --git a/...nit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedDependency.java b/...nit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedDependency.java
@@ -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);
         }

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/Dependency.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/Dependency.java
@@ -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",

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/DomainObjectCreationContext.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/DomainObjectCreationContext.java
@@ -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);
     }

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/ImportContext.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/ImportContext.java
@@ -67,5 +67,7 @@ public interface ImportContext {
 
     Set<InstanceofCheck> createInstanceofChecksFor(JavaCodeUnit codeUnit);
 
+    Set<TypeCast> createTypeCastsFor(JavaCodeUnit codeUnit);
+
     JavaClass resolveClass(String fullyQualifiedClassName);
 }
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java
@@ -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();

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassDependencies.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassDependencies.java
@@ -49,6 +49,7 @@ private Supplier<Set<Dependency>> createDirectDependenciesFromClassSupplier() {
                         throwsDeclarationDependenciesFromSelf(),
                         annotationDependenciesFromSelf(),
                         instanceofCheckDependenciesFromSelf(),
+                        typeCastDependenciesFromSelf(),
                         referencedClassObjectDependenciesFromSelf(),
                         typeParameterDependenciesFromSelf()
                 ).collect(toImmutableSet())
@@ -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());

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassMembers.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassMembers.java
@@ -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) {

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaCodeUnit.java
@@ -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);
@@ -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;
@@ -281,6 +287,7 @@ void completeFrom(ImportContext context) {
                 .collect(toImmutableSet());
         referencedClassObjects = context.createReferencedClassObjectsFor(this);
         instanceofChecks = context.createInstanceofChecksFor(this);
+        typeCasts = context.createTypeCastsFor(this);
     }
 
     @ResolvesTypesViaReflection

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/TypeCast.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/TypeCast.java
@@ -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);
+    }
+}
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImportRecord.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImportRecord.java
@@ -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();
 
@@ -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,
@@ -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,
@@ -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())

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileProcessor.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileProcessor.java
@@ -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);
@@ -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;
@@ -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);
@@ -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());
         }
 
@@ -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());
         }
 
@@ -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);