Skip to content

Commit

Permalink
analyze local variable instantiations
Browse files Browse the repository at this point in the history 
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: TNG#768
  • Loading branch information
@ttho
ttho committed Jul 31, 2024
1 parent 771fe26 commit 3b3383f
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 16 deletions.
15 changes: 14 additions & 1 deletion archunit/src/main/java/com/tngtech/archunit/ArchConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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<String> getClassResolver() {
return Optional.ofNullable(properties.getProperty(CLASS_RESOLVER));
Expand Down Expand Up @@ -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;
Expand Down
27 changes: 20 additions & 7 deletions archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -57,7 +60,6 @@
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -75,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<JavaAnnotationBuilder> annotations = new HashSet<>();
private final SourceDescriptor sourceDescriptor;
Expand Down Expand Up @@ -259,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<String> fullyQualifiedClassNamesOf(List<JavaClassDescriptor> classDescriptors) {
Expand Down Expand Up @@ -319,14 +321,18 @@ private static class MethodProcessor extends MethodVisitor {
private final Set<JavaAnnotationBuilder> annotations = new HashSet<>();
private final SetMultimap<Integer, JavaAnnotationBuilder> parameterAnnotationsByIndex = HashMultimap.create();
private int actualLineNumber;
private final Map<Label, Integer> 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
Expand All @@ -352,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
Expand Down Expand Up @@ -384,13 +393,17 @@ public void visitMethodInsn(int opcode, String owner, String name, String 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("<init>")) {
return;
}
JavaClassDescriptor type = JavaClassDescriptorImporter.importAsmType(Type.getType(desc));
accessHandler.handleReferencedClassObject(type, actualLineNumber);
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) {
Expand Down Expand Up @@ -484,7 +497,7 @@ public void onDeclaredThrowsClause(Collection<String> exceptionTypeNames) {

@Override
public void onDeclaredGenericSignatureType(String typeName) {
accessHandler.handleReferencedClassObject(JavaClassDescriptor.From.name(typeName), actualLineNumber);
accessHandler.handleReferencedClassObject(JavaClassDescriptor.From.name(typeName), lineNumber);
}
});
}
Expand Down
30 changes: 26 additions & 4 deletions ...va/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
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;
Expand All @@ -8,7 +9,11 @@
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;
Expand All @@ -18,14 +23,31 @@ public class ClassFileImporterLocalVariableDependenciesTest {

@Test
public void imports_referenced_class_object_in_LocalVariable() {
ArchConfiguration.get().setAnalyzeLocalVariableInstantiations(true);

JavaClasses classes = new ClassFileImporter().importClasses(ReferencingClassObjectsFromLocalVariable.class);
Set<ReferencedClassObject> referencedClassObjects = classes.get(ReferencingClassObjectsFromLocalVariable.class).getReferencedClassObjects();

assertThatReferencedClassObjects(referencedClassObjects).containReferencedClassObjects(
referencedClassObject(FilterInputStream.class, 16)
, referencedClassObject(FilterInputStream.class, 18)
, referencedClassObject(FilterInputStream.class, 14)
, referencedClassObject(Double.class, 14)
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)
);
}

Expand Down
45 changes: 41 additions & 4 deletions ...mporter/testexamples/referencedclassobjects/ReferencingClassObjectsFromLocalVariable.java
View file
Original file line number Diff line number Diff line change
@@ -1,19 +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 {
public class ReferencingClassObjectsFromLocalVariable<T extends Number> {

static {
FilterInputStream streamStatic = null;
List<Double> listStatic = new ArrayList<>();
@SuppressWarnings("MismatchedQueryAndUpdateOfCollection") List<Double> 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; }
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<FilterInputStream> list = null;
List<InputStream> list2 = null;
// multiple generic parameters
Map<FilterInputStream, InputStream> map = null;
// nested generic parameters
Map<Set<FilterInputStream>, InputStream> map2 = null;
System.out.println(list);
System.out.println(list2);
System.out.println(map);
System.out.println(map2);
}

void referenceByGeneric() { List<FilterInputStream> list = null; }
void referenceToOwnGenericType() {
T myType = null;
System.out.println(myType);
}
}
37 changes: 37 additions & 0 deletions docs/userguide/010_Advanced_Configuration.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T extends E> {
void m() {
B var1 = null;
D<C> 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.

0 comments on commit 3b3383f

Please sign in to comment.