From 77ba5a03c9bcd43f06627aa93cff5924102df67f Mon Sep 17 00:00:00 2001 From: Timo Thomas Date: Thu, 11 Jul 2024 16:09:17 +0200 Subject: [PATCH 1/2] analyze local variable instantiations So far local variable instantiations were not analyzed at all, leaving gaps in the evaluation of dependency rules. If a local variable of a certain type was instantiated in a method, but no other use of that type was present (e.g. a method call on the type, or a field access), the dependency from the method to that type was undetected. Here, if enabled by a new configuration property (to preserve backward compatibility and performance), local variable instantiations add class dependencies from the surrounding method to the type of the variable and, if that type has generic type parameters, also from the method to any concrete generic type. Issue: #768 Signed-off-by: Timo Thomas --- .../tngtech/archunit/ArchConfiguration.java | 15 +- .../core/importer/JavaClassProcessor.java | 128 +++++++++++++++++- ...ImporterLocalVariableDependenciesTest.java | 54 ++++++++ ...erencingClassObjectsFromLocalVariable.java | 56 ++++++++ .../userguide/010_Advanced_Configuration.adoc | 37 +++++ 5 files changed, 286 insertions(+), 4 deletions(-) create mode 100644 archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/referencedclassobjects/ReferencingClassObjectsFromLocalVariable.java diff --git a/archunit/src/main/java/com/tngtech/archunit/ArchConfiguration.java b/archunit/src/main/java/com/tngtech/archunit/ArchConfiguration.java index cd7ff0329c..da9f2cfe7e 100644 --- a/archunit/src/main/java/com/tngtech/archunit/ArchConfiguration.java +++ b/archunit/src/main/java/com/tngtech/archunit/ArchConfiguration.java @@ -53,6 +53,8 @@ public final class ArchConfiguration { static final String CLASS_RESOLVER_ARGS = "classResolver.args"; @Internal public static final String ENABLE_MD5_IN_CLASS_SOURCES = "enableMd5InClassSources"; + @Internal + public static final String ANALYZE_LOCAL_VARIABLE_INSTANTIATIONS = "analyzeLocalVariableInstantiations"; private static final String EXTENSION_PREFIX = "extension"; private static final Logger LOG = LoggerFactory.getLogger(ArchConfiguration.class); @@ -124,6 +126,16 @@ public void setMd5InClassSourcesEnabled(boolean enabled) { properties.setProperty(ENABLE_MD5_IN_CLASS_SOURCES, String.valueOf(enabled)); } + @PublicAPI(usage = ACCESS) + public boolean analyzeLocalVariableInstantiations() { + return Boolean.parseBoolean(properties.getProperty(ANALYZE_LOCAL_VARIABLE_INSTANTIATIONS)); + } + + @PublicAPI(usage = ACCESS) + public void setAnalyzeLocalVariableInstantiations(boolean enabled) { + properties.setProperty(ANALYZE_LOCAL_VARIABLE_INSTANTIATIONS, String.valueOf(enabled)); + } + @PublicAPI(usage = ACCESS) public Optional getClassResolver() { return Optional.ofNullable(properties.getProperty(CLASS_RESOLVER)); @@ -328,7 +340,8 @@ private ArchConfiguration copy() { private static class PropertiesOverwritableBySystemProperties { private static final Properties PROPERTY_DEFAULTS = createProperties(ImmutableMap.of( RESOLVE_MISSING_DEPENDENCIES_FROM_CLASS_PATH, Boolean.TRUE.toString(), - ENABLE_MD5_IN_CLASS_SOURCES, Boolean.FALSE.toString() + ENABLE_MD5_IN_CLASS_SOURCES, Boolean.FALSE.toString(), + ANALYZE_LOCAL_VARIABLE_INSTANTIATIONS, Boolean.FALSE.toString() )); private final Properties baseProperties; diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java index 7a1dea1d44..0a57602968 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java @@ -19,8 +19,10 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -36,6 +38,7 @@ import com.google.common.primitives.Ints; import com.google.common.primitives.Longs; import com.google.common.primitives.Shorts; +import com.tngtech.archunit.ArchConfiguration; import com.tngtech.archunit.Internal; import com.tngtech.archunit.base.HasDescription; import com.tngtech.archunit.base.MayResolveTypesViaReflection; @@ -74,9 +77,9 @@ class JavaClassProcessor extends ClassVisitor { private static final Logger LOG = LoggerFactory.getLogger(JavaClassProcessor.class); - private static final AccessHandler NO_OP = new AccessHandler.NoOp(); + private final boolean analyzeLocalVariableInstantiations = ArchConfiguration.get().analyzeLocalVariableInstantiations(); private DomainBuilders.JavaClassBuilder javaClassBuilder; private final Set annotations = new HashSet<>(); private final SourceDescriptor sourceDescriptor; @@ -258,7 +261,7 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si .withThrowsClause(throwsDeclarations); declarationHandler.onDeclaredThrowsClause(fullyQualifiedClassNamesOf(throwsDeclarations)); - return new MethodProcessor(className, accessHandler, codeUnitBuilder, declarationHandler); + return new MethodProcessor(className, accessHandler, codeUnitBuilder, declarationHandler, analyzeLocalVariableInstantiations); } private Collection fullyQualifiedClassNamesOf(List classDescriptors) { @@ -318,14 +321,18 @@ private static class MethodProcessor extends MethodVisitor { private final Set annotations = new HashSet<>(); private final SetMultimap parameterAnnotationsByIndex = HashMultimap.create(); private int actualLineNumber; + private final Map labelToLineNumber; + private final boolean analyzeLocalVariableInstantiations; - MethodProcessor(String declaringClassName, AccessHandler accessHandler, DomainBuilders.JavaCodeUnitBuilder codeUnitBuilder, DeclarationHandler declarationHandler) { + MethodProcessor(String declaringClassName, AccessHandler accessHandler, DomainBuilders.JavaCodeUnitBuilder codeUnitBuilder, DeclarationHandler declarationHandler, boolean analyzeLocalVariableInstantiations) { super(ASM_API_VERSION); this.declaringClassName = declaringClassName; this.accessHandler = accessHandler; this.codeUnitBuilder = codeUnitBuilder; this.declarationHandler = declarationHandler; codeUnitBuilder.withParameterAnnotations(parameterAnnotationsByIndex); + this.analyzeLocalVariableInstantiations = analyzeLocalVariableInstantiations; + labelToLineNumber = analyzeLocalVariableInstantiations ? new HashMap<>() : null; } @Override @@ -351,6 +358,9 @@ public void visitLineNumber(int line, Label label) { public void visitLabel(Label label) { LOG.trace("Examining label {}", label); accessHandler.onLabel(label); + if (analyzeLocalVariableInstantiations) { + labelToLineNumber.put(label, actualLineNumber); + } } @Override @@ -381,6 +391,117 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, accessHandler.handleMethodInstruction(owner, name, desc); } + @Override + public void visitLocalVariable(String name, String desc, String signature, Label start, Label end, int index) { + if (!analyzeLocalVariableInstantiations) { + return; + } + if (name.equals("this") || + this.codeUnitBuilder.getModifiers().contains(JavaModifier.SYNTHETIC) || + this.codeUnitBuilder.getName().equals("")) { + return; + } + int lineNumber = start != null ? labelToLineNumber.getOrDefault(start, actualLineNumber) : actualLineNumber; + JavaClassDescriptor type = JavaClassDescriptorImporter.importAsmTypeFromDescriptor(desc); + accessHandler.handleReferencedClassObject(type, lineNumber); + JavaFieldTypeSignatureImporter.parseAsmFieldTypeSignature(signature, new DeclarationHandler() { + @Override + public boolean isNew(String className) { + return false; + } + + @Override + public void onNewClass(String className, Optional superclassName, List interfaceNames) { + + } + + @Override + public void onDeclaredTypeParameters(DomainBuilders.JavaClassTypeParametersBuilder typeParametersBuilder) { + + } + + @Override + public void onGenericSuperclass(DomainBuilders.JavaParameterizedTypeBuilder genericSuperclassBuilder) { + + } + + @Override + public void onGenericInterfaces(List> genericInterfaceBuilders) { + + } + + @Override + public void onDeclaredField(DomainBuilders.JavaFieldBuilder fieldBuilder, String fieldTypeName) { + + } + + @Override + public void onDeclaredConstructor(DomainBuilders.JavaConstructorBuilder constructorBuilder, Collection rawParameterTypeNames) { + + } + + @Override + public void onDeclaredMethod(DomainBuilders.JavaMethodBuilder methodBuilder, Collection rawParameterTypeNames, String rawReturnTypeName) { + + } + + @Override + public void onDeclaredStaticInitializer(DomainBuilders.JavaStaticInitializerBuilder staticInitializerBuilder) { + + } + + @Override + public void onDeclaredClassAnnotations(Set annotationBuilders) { + + } + + @Override + public void onDeclaredMemberAnnotations(String memberName, String descriptor, Set annotations) { + + } + + @Override + public void onDeclaredAnnotationValueType(String valueTypeName) { + + } + + @Override + public void onDeclaredAnnotationDefaultValue(String methodName, String methodDescriptor, ValueBuilder valueBuilder) { + + } + + @Override + public void registerEnclosingClass(String ownerName, String enclosingClassName) { + + } + + @Override + public void registerEnclosingCodeUnit(String ownerName, CodeUnit enclosingCodeUnit) { + + } + + @Override + public void onDeclaredClassObject(String typeName) { + + } + + @Override + public void onDeclaredInstanceofCheck(String typeName) { + + } + + @Override + public void onDeclaredThrowsClause(Collection exceptionTypeNames) { + + } + + @Override + public void onDeclaredGenericSignatureType(String typeName) { + accessHandler.handleReferencedClassObject(JavaClassDescriptor.From.name(typeName), lineNumber); + } + }); + } + @Override public void visitTypeInsn(int opcode, String type) { if (opcode == Opcodes.INSTANCEOF) { @@ -578,6 +699,7 @@ public void handleTryFinallyBlock(Label start, Label end, Label handler) { @Override public void onMethodEnd() { } + } } diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java new file mode 100644 index 0000000000..611d03125d --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java @@ -0,0 +1,54 @@ +package com.tngtech.archunit.core.importer; + +import com.tngtech.archunit.ArchConfiguration; +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.domain.ReferencedClassObject; +import com.tngtech.archunit.core.importer.testexamples.referencedclassobjects.ReferencingClassObjectsFromLocalVariable; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.io.FilterInputStream; +import java.io.InputStream; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.Stack; + +import static com.tngtech.archunit.testutil.Assertions.assertThatReferencedClassObjects; +import static com.tngtech.archunit.testutil.assertion.ReferencedClassObjectsAssertion.referencedClassObject; + +@RunWith(DataProviderRunner.class) +public class ClassFileImporterLocalVariableDependenciesTest { + + @Test + public void imports_referenced_class_object_in_LocalVariable() { + ArchConfiguration.get().setAnalyzeLocalVariableInstantiations(true); + + JavaClasses classes = new ClassFileImporter().importClasses(ReferencingClassObjectsFromLocalVariable.class); + Set referencedClassObjects = classes.get(ReferencingClassObjectsFromLocalVariable.class).getReferencedClassObjects(); + + assertThatReferencedClassObjects(referencedClassObjects).containReferencedClassObjects( + referencedClassObject(FilterInputStream.class, 14), + referencedClassObject(List.class, 15), + referencedClassObject(Double.class, 15), + referencedClassObject(FilterInputStream.class, 21), + referencedClassObject(InputStream.class, 22), + referencedClassObject(FilterInputStream.class, 26), + referencedClassObject(InputStream.class, 27), + referencedClassObject(FilterInputStream.class, 32), + referencedClassObject(InputStream.class, 33), + referencedClassObject(FilterInputStream.class, 40), + referencedClassObject(InputStream.class, 41), + referencedClassObject(Map.class, 43), + referencedClassObject(FilterInputStream.class, 43), + referencedClassObject(InputStream.class, 43), + referencedClassObject(Map.class, 45), + referencedClassObject(Set.class, 45), + referencedClassObject(FilterInputStream.class, 45), + referencedClassObject(InputStream.class, 45), + referencedClassObject(Number.class, 53) + ); + } + +} diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/referencedclassobjects/ReferencingClassObjectsFromLocalVariable.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/referencedclassobjects/ReferencingClassObjectsFromLocalVariable.java new file mode 100644 index 0000000000..665a25d5d2 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/referencedclassobjects/ReferencingClassObjectsFromLocalVariable.java @@ -0,0 +1,56 @@ +package com.tngtech.archunit.core.importer.testexamples.referencedclassobjects; + +import java.io.FilterInputStream; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; + +@SuppressWarnings("unused") +public class ReferencingClassObjectsFromLocalVariable { + + static { + FilterInputStream streamStatic = null; + @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") List listStatic = new ArrayList<>(); + //noinspection ResultOfMethodCallIgnored + listStatic.size(); // if listStatic is not used it is optimized away. Surprisingly this is not the case for streamStatic above + } + + void reference() { + FilterInputStream stream = null; + InputStream stream2 = null; + System.out.println(stream); + System.out.println(stream2); + // after statement and comment + FilterInputStream stream3 = null; + InputStream stream4 = null; + System.out.println(stream3); + System.out.println(stream4); + { + // in block + FilterInputStream stream5 = null; + InputStream stream6 = null; + System.out.println(stream5); + System.out.println(stream6); + } + } + + void referenceByGeneric() { + List list = null; + List list2 = null; + // multiple generic parameters + Map map = null; + // nested generic parameters + Map, InputStream> map2 = null; + System.out.println(list); + System.out.println(list2); + System.out.println(map); + System.out.println(map2); + } + + void referenceToOwnGenericType() { + T myType = null; + System.out.println(myType); + } +} diff --git a/docs/userguide/010_Advanced_Configuration.adoc b/docs/userguide/010_Advanced_Configuration.adoc index b626d7fbf4..6ec63706d1 100644 --- a/docs/userguide/010_Advanced_Configuration.adoc +++ b/docs/userguide/010_Advanced_Configuration.adoc @@ -223,3 +223,40 @@ in particular by custom predicates and conditions, there is at the moment no more sophisticated way than plain text parsing. Users can tailor this to their specific environments where they know which sorts of failure formats can appear in practice. + +=== Analysis of Local Variable Instantiations + +ArchUnit does not analyze local variables by default. There is limited support for it by adding class dependencies +for every local variable instantiation: from the surrounding method to the type of the variable and, if that type has +generic type parameters, also from the method to any concrete generic type. Since this has +a performance impact, it is disabled by default, but it can be activated the following way: + +[source,options="nowrap"] +.archunit.properties +---- +analyzeLocalVariableInstantiations=true +---- + +If this feature is enabled, the following code would add class dependencies + +* A.m() -> B +* A.m() -> C +* A.m() -> D +* A.m() -> E + +which otherwise wouldn't be the case: + +[source,java,options="nowrap"] +---- +public class A { + + void m() { + B var1 = null; + D var2 = null; + T var3 = null; + } + +} +---- + +Note that there are no other uses (method calls, field accesses) of types B, C and D in the code. \ No newline at end of file From 8120aad3ec50173eb45ad3a3f2caceb949f97790 Mon Sep 17 00:00:00 2001 From: Timo Thomas Date: Thu, 1 Aug 2024 12:25:27 +0200 Subject: [PATCH 2/2] fix spotless bugs Issue: #768 Signed-off-by: Timo Thomas --- .../ClassFileImporterLocalVariableDependenciesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java index 611d03125d..0129fabe8c 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java @@ -13,7 +13,7 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.Stack; + import static com.tngtech.archunit.testutil.Assertions.assertThatReferencedClassObjects; import static com.tngtech.archunit.testutil.assertion.ReferencedClassObjectsAssertion.referencedClassObject;