Skip to content

Commit 77ba5a0

Browse files
tthottho
committed
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 <work@timothomas.de>
1 parent d1332ca commit 77ba5a0

File tree

5 files changed

+286
-4
lines changed

5 files changed

+286
-4
lines changed

‎archunit/src/main/java/com/tngtech/archunit/ArchConfiguration.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ public final class ArchConfiguration {
5353
static final String CLASS_RESOLVER_ARGS = "classResolver.args";
5454
@Internal
5555
public static final String ENABLE_MD5_IN_CLASS_SOURCES = "enableMd5InClassSources";
56+
@Internal
57+
public static final String ANALYZE_LOCAL_VARIABLE_INSTANTIATIONS = "analyzeLocalVariableInstantiations";
5658
private static final String EXTENSION_PREFIX = "extension";
5759

5860
private static final Logger LOG = LoggerFactory.getLogger(ArchConfiguration.class);
@@ -124,6 +126,16 @@ public void setMd5InClassSourcesEnabled(boolean enabled) {
124126
properties.setProperty(ENABLE_MD5_IN_CLASS_SOURCES, String.valueOf(enabled));
125127
}
126128

129+
@PublicAPI(usage = ACCESS)
130+
public boolean analyzeLocalVariableInstantiations() {
131+
return Boolean.parseBoolean(properties.getProperty(ANALYZE_LOCAL_VARIABLE_INSTANTIATIONS));
132+
}
133+
134+
@PublicAPI(usage = ACCESS)
135+
public void setAnalyzeLocalVariableInstantiations(boolean enabled) {
136+
properties.setProperty(ANALYZE_LOCAL_VARIABLE_INSTANTIATIONS, String.valueOf(enabled));
137+
}
138+
127139
@PublicAPI(usage = ACCESS)
128140
public Optional<String> getClassResolver() {
129141
return Optional.ofNullable(properties.getProperty(CLASS_RESOLVER));
@@ -328,7 +340,8 @@ private ArchConfiguration copy() {
328340
private static class PropertiesOverwritableBySystemProperties {
329341
private static final Properties PROPERTY_DEFAULTS = createProperties(ImmutableMap.of(
330342
RESOLVE_MISSING_DEPENDENCIES_FROM_CLASS_PATH, Boolean.TRUE.toString(),
331-
ENABLE_MD5_IN_CLASS_SOURCES, Boolean.FALSE.toString()
343+
ENABLE_MD5_IN_CLASS_SOURCES, Boolean.FALSE.toString(),
344+
ANALYZE_LOCAL_VARIABLE_INSTANTIATIONS, Boolean.FALSE.toString()
332345
));
333346

334347
private final Properties baseProperties;

‎archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java‎

Lines changed: 125 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
import java.util.ArrayList;
2020
import java.util.Collection;
2121
import java.util.Collections;
22+
import java.util.HashMap;
2223
import java.util.HashSet;
2324
import java.util.List;
25+
import java.util.Map;
2426
import java.util.Optional;
2527
import java.util.Set;
2628
import java.util.stream.Stream;
@@ -36,6 +38,7 @@
3638
import com.google.common.primitives.Ints;
3739
import com.google.common.primitives.Longs;
3840
import com.google.common.primitives.Shorts;
41+
import com.tngtech.archunit.ArchConfiguration;
3942
import com.tngtech.archunit.Internal;
4043
import com.tngtech.archunit.base.HasDescription;
4144
import com.tngtech.archunit.base.MayResolveTypesViaReflection;
@@ -74,9 +77,9 @@
7477

7578
class JavaClassProcessor extends ClassVisitor {
7679
private static final Logger LOG = LoggerFactory.getLogger(JavaClassProcessor.class);
77-
7880
private static final AccessHandler NO_OP = new AccessHandler.NoOp();
7981

82+
private final boolean analyzeLocalVariableInstantiations = ArchConfiguration.get().analyzeLocalVariableInstantiations();
8083
private DomainBuilders.JavaClassBuilder javaClassBuilder;
8184
private final Set<JavaAnnotationBuilder> annotations = new HashSet<>();
8285
private final SourceDescriptor sourceDescriptor;
@@ -258,7 +261,7 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si
258261
.withThrowsClause(throwsDeclarations);
259262
declarationHandler.onDeclaredThrowsClause(fullyQualifiedClassNamesOf(throwsDeclarations));
260263

261-
return new MethodProcessor(className, accessHandler, codeUnitBuilder, declarationHandler);
264+
return new MethodProcessor(className, accessHandler, codeUnitBuilder, declarationHandler, analyzeLocalVariableInstantiations);
262265
}
263266

264267
private Collection<String> fullyQualifiedClassNamesOf(List<JavaClassDescriptor> classDescriptors) {
@@ -318,14 +321,18 @@ private static class MethodProcessor extends MethodVisitor {
318321
private final Set<JavaAnnotationBuilder> annotations = new HashSet<>();
319322
private final SetMultimap<Integer, JavaAnnotationBuilder> parameterAnnotationsByIndex = HashMultimap.create();
320323
private int actualLineNumber;
324+
private final Map<Label, Integer> labelToLineNumber;
325+
private final boolean analyzeLocalVariableInstantiations;
321326

322-
MethodProcessor(String declaringClassName, AccessHandler accessHandler, DomainBuilders.JavaCodeUnitBuilder<?, ?> codeUnitBuilder, DeclarationHandler declarationHandler) {
327+
MethodProcessor(String declaringClassName, AccessHandler accessHandler, DomainBuilders.JavaCodeUnitBuilder<?, ?> codeUnitBuilder, DeclarationHandler declarationHandler, boolean analyzeLocalVariableInstantiations) {
323328
super(ASM_API_VERSION);
324329
this.declaringClassName = declaringClassName;
325330
this.accessHandler = accessHandler;
326331
this.codeUnitBuilder = codeUnitBuilder;
327332
this.declarationHandler = declarationHandler;
328333
codeUnitBuilder.withParameterAnnotations(parameterAnnotationsByIndex);
334+
this.analyzeLocalVariableInstantiations = analyzeLocalVariableInstantiations;
335+
labelToLineNumber = analyzeLocalVariableInstantiations ? new HashMap<>() : null;
329336
}
330337

331338
@Override
@@ -351,6 +358,9 @@ public void visitLineNumber(int line, Label label) {
351358
public void visitLabel(Label label) {
352359
LOG.trace("Examining label {}", label);
353360
accessHandler.onLabel(label);
361+
if (analyzeLocalVariableInstantiations) {
362+
labelToLineNumber.put(label, actualLineNumber);
363+
}
354364
}
355365

356366
@Override
@@ -381,6 +391,117 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc,
381391
accessHandler.handleMethodInstruction(owner, name, desc);
382392
}
383393

394+
@Override
395+
public void visitLocalVariable(String name, String desc, String signature, Label start, Label end, int index) {
396+
if (!analyzeLocalVariableInstantiations) {
397+
return;
398+
}
399+
if (name.equals("this") ||
400+
this.codeUnitBuilder.getModifiers().contains(JavaModifier.SYNTHETIC) ||
401+
this.codeUnitBuilder.getName().equals("<init>")) {
402+
return;
403+
}
404+
int lineNumber = start != null ? labelToLineNumber.getOrDefault(start, actualLineNumber) : actualLineNumber;
405+
JavaClassDescriptor type = JavaClassDescriptorImporter.importAsmTypeFromDescriptor(desc);
406+
accessHandler.handleReferencedClassObject(type, lineNumber);
407+
JavaFieldTypeSignatureImporter.parseAsmFieldTypeSignature(signature, new DeclarationHandler() {
408+
@Override
409+
public boolean isNew(String className) {
410+
return false;
411+
}
412+
413+
@Override
414+
public void onNewClass(String className, Optional<String> superclassName, List<String> interfaceNames) {
415+
416+
}
417+
418+
@Override
419+
public void onDeclaredTypeParameters(DomainBuilders.JavaClassTypeParametersBuilder typeParametersBuilder) {
420+
421+
}
422+
423+
@Override
424+
public void onGenericSuperclass(DomainBuilders.JavaParameterizedTypeBuilder<JavaClass> genericSuperclassBuilder) {
425+
426+
}
427+
428+
@Override
429+
public void onGenericInterfaces(List<DomainBuilders.JavaParameterizedTypeBuilder<JavaClass>> genericInterfaceBuilders) {
430+
431+
}
432+
433+
@Override
434+
public void onDeclaredField(DomainBuilders.JavaFieldBuilder fieldBuilder, String fieldTypeName) {
435+
436+
}
437+
438+
@Override
439+
public void onDeclaredConstructor(DomainBuilders.JavaConstructorBuilder constructorBuilder, Collection<String> rawParameterTypeNames) {
440+
441+
}
442+
443+
@Override
444+
public void onDeclaredMethod(DomainBuilders.JavaMethodBuilder methodBuilder, Collection<String> rawParameterTypeNames, String rawReturnTypeName) {
445+
446+
}
447+
448+
@Override
449+
public void onDeclaredStaticInitializer(DomainBuilders.JavaStaticInitializerBuilder staticInitializerBuilder) {
450+
451+
}
452+
453+
@Override
454+
public void onDeclaredClassAnnotations(Set<JavaAnnotationBuilder> annotationBuilders) {
455+
456+
}
457+
458+
@Override
459+
public void onDeclaredMemberAnnotations(String memberName, String descriptor, Set<JavaAnnotationBuilder> annotations) {
460+
461+
}
462+
463+
@Override
464+
public void onDeclaredAnnotationValueType(String valueTypeName) {
465+
466+
}
467+
468+
@Override
469+
public void onDeclaredAnnotationDefaultValue(String methodName, String methodDescriptor, ValueBuilder valueBuilder) {
470+
471+
}
472+
473+
@Override
474+
public void registerEnclosingClass(String ownerName, String enclosingClassName) {
475+
476+
}
477+
478+
@Override
479+
public void registerEnclosingCodeUnit(String ownerName, CodeUnit enclosingCodeUnit) {
480+
481+
}
482+
483+
@Override
484+
public void onDeclaredClassObject(String typeName) {
485+
486+
}
487+
488+
@Override
489+
public void onDeclaredInstanceofCheck(String typeName) {
490+
491+
}
492+
493+
@Override
494+
public void onDeclaredThrowsClause(Collection<String> exceptionTypeNames) {
495+
496+
}
497+
498+
@Override
499+
public void onDeclaredGenericSignatureType(String typeName) {
500+
accessHandler.handleReferencedClassObject(JavaClassDescriptor.From.name(typeName), lineNumber);
501+
}
502+
});
503+
}
504+
384505
@Override
385506
public void visitTypeInsn(int opcode, String type) {
386507
if (opcode == Opcodes.INSTANCEOF) {
@@ -578,6 +699,7 @@ public void handleTryFinallyBlock(Label start, Label end, Label handler) {
578699
@Override
579700
public void onMethodEnd() {
580701
}
702+
581703
}
582704
}
583705

‎archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterLocalVariableDependenciesTest.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package com.tngtech.archunit.core.importer;
2+
3+
import com.tngtech.archunit.ArchConfiguration;
4+
import com.tngtech.archunit.core.domain.JavaClasses;
5+
import com.tngtech.archunit.core.domain.ReferencedClassObject;
6+
import com.tngtech.archunit.core.importer.testexamples.referencedclassobjects.ReferencingClassObjectsFromLocalVariable;
7+
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
8+
import org.junit.Test;
9+
import org.junit.runner.RunWith;
10+
11+
import java.io.FilterInputStream;
12+
import java.io.InputStream;
13+
import java.util.List;
14+
import java.util.Map;
15+
import java.util.Set;
16+
import java.util.Stack;
17+
18+
import static com.tngtech.archunit.testutil.Assertions.assertThatReferencedClassObjects;
19+
import static com.tngtech.archunit.testutil.assertion.ReferencedClassObjectsAssertion.referencedClassObject;
20+
21+
@RunWith(DataProviderRunner.class)
22+
public class ClassFileImporterLocalVariableDependenciesTest {
23+
24+
@Test
25+
public void imports_referenced_class_object_in_LocalVariable() {
26+
ArchConfiguration.get().setAnalyzeLocalVariableInstantiations(true);
27+
28+
JavaClasses classes = new ClassFileImporter().importClasses(ReferencingClassObjectsFromLocalVariable.class);
29+
Set<ReferencedClassObject> referencedClassObjects = classes.get(ReferencingClassObjectsFromLocalVariable.class).getReferencedClassObjects();
30+
31+
assertThatReferencedClassObjects(referencedClassObjects).containReferencedClassObjects(
32+
referencedClassObject(FilterInputStream.class, 14),
33+
referencedClassObject(List.class, 15),
34+
referencedClassObject(Double.class, 15),
35+
referencedClassObject(FilterInputStream.class, 21),
36+
referencedClassObject(InputStream.class, 22),
37+
referencedClassObject(FilterInputStream.class, 26),
38+
referencedClassObject(InputStream.class, 27),
39+
referencedClassObject(FilterInputStream.class, 32),
40+
referencedClassObject(InputStream.class, 33),
41+
referencedClassObject(FilterInputStream.class, 40),
42+
referencedClassObject(InputStream.class, 41),
43+
referencedClassObject(Map.class, 43),
44+
referencedClassObject(FilterInputStream.class, 43),
45+
referencedClassObject(InputStream.class, 43),
46+
referencedClassObject(Map.class, 45),
47+
referencedClassObject(Set.class, 45),
48+
referencedClassObject(FilterInputStream.class, 45),
49+
referencedClassObject(InputStream.class, 45),
50+
referencedClassObject(Number.class, 53)
51+
);
52+
}
53+
54+
}

‎archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/referencedclassobjects/ReferencingClassObjectsFromLocalVariable.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package com.tngtech.archunit.core.importer.testexamples.referencedclassobjects;
2+
3+
import java.io.FilterInputStream;
4+
import java.io.InputStream;
5+
import java.util.ArrayList;
6+
import java.util.List;
7+
import java.util.Map;
8+
import java.util.Set;
9+
10+
@SuppressWarnings("unused")
11+
public class ReferencingClassObjectsFromLocalVariable<T extends Number> {
12+
13+
static {
14+
FilterInputStream streamStatic = null;
15+
@SuppressWarnings("MismatchedQueryAndUpdateOfCollection") List<Double> listStatic = new ArrayList<>();
16+
//noinspection ResultOfMethodCallIgnored
17+
listStatic.size(); // if listStatic is not used it is optimized away. Surprisingly this is not the case for streamStatic above
18+
}
19+
20+
void reference() {
21+
FilterInputStream stream = null;
22+
InputStream stream2 = null;
23+
System.out.println(stream);
24+
System.out.println(stream2);
25+
// after statement and comment
26+
FilterInputStream stream3 = null;
27+
InputStream stream4 = null;
28+
System.out.println(stream3);
29+
System.out.println(stream4);
30+
{
31+
// in block
32+
FilterInputStream stream5 = null;
33+
InputStream stream6 = null;
34+
System.out.println(stream5);
35+
System.out.println(stream6);
36+
}
37+
}
38+
39+
void referenceByGeneric() {
40+
List<FilterInputStream> list = null;
41+
List<InputStream> list2 = null;
42+
// multiple generic parameters
43+
Map<FilterInputStream, InputStream> map = null;
44+
// nested generic parameters
45+
Map<Set<FilterInputStream>, InputStream> map2 = null;
46+
System.out.println(list);
47+
System.out.println(list2);
48+
System.out.println(map);
49+
System.out.println(map2);
50+
}
51+
52+
void referenceToOwnGenericType() {
53+
T myType = null;
54+
System.out.println(myType);
55+
}
56+
}

‎docs/userguide/010_Advanced_Configuration.adoc‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,3 +223,40 @@ in particular by custom predicates and conditions,
223223
there is at the moment no more sophisticated way than plain text parsing.
224224
Users can tailor this to their specific environments where they know
225225
which sorts of failure formats can appear in practice.
226+
227+
=== Analysis of Local Variable Instantiations
228+
229+
ArchUnit does not analyze local variables by default. There is limited support for it by adding class dependencies
230+
for every local variable instantiation: from the surrounding method to the type of the variable and, if that type has
231+
generic type parameters, also from the method to any concrete generic type. Since this has
232+
a performance impact, it is disabled by default, but it can be activated the following way:
233+
234+
[source,options="nowrap"]
235+
.archunit.properties
236+
----
237+
analyzeLocalVariableInstantiations=true
238+
----
239+
240+
If this feature is enabled, the following code would add class dependencies
241+
242+
* A.m() -> B
243+
* A.m() -> C
244+
* A.m() -> D
245+
* A.m() -> E
246+
247+
which otherwise wouldn't be the case:
248+
249+
[source,java,options="nowrap"]
250+
----
251+
public class A<T extends E> {
252+
253+
void m() {
254+
B var1 = null;
255+
D<C> var2 = null;
256+
T var3 = null;
257+
}
258+
259+
}
260+
----
261+
262+
Note that there are no other uses (method calls, field accesses) of types B, C and D in the code.

0 commit comments

Comments
 (0)