Skip to content

Commit

Permalink
Improve automatic resolution of class import dependencies #785
Browse files Browse the repository at this point in the history
When importing classes there will always be some dependencies that are missing. E.g. we import package `com.myapp`, but naturally those classes will at least depend on classes in `java.lang`. ArchUnit has an automatic import dependency resolution process to pick up such missing classes from the classpath (by default).
However, this resolution process was somewhat inconsistent. It would in a first run resolve all missing member types, then in a second run resolve all missing access targets, then supertypes and then annotations. Thus, e.g. classes that were discovered when resolving supertypes would not have their member types resolved anymore, which could lead to surprising behavior. Also, some dependencies were not automatically resolved, in particular:

* generic type signatures
* enclosing classes of nested classes
* component types of array types
* types of declared thrown exceptions
* target types of instanceof checks
* referenced class objects

This PR consolidates the resolution process and fixes these shortcomings. It does so by registering all import dependencies already withing the ASM visitors. This way, once all the classes picked originally are imported we now have a set of class names that are needed as dependencies. When we run the importer again to resolve these missing types we will again transitively register the types these types were missing. By distinguishing 6 types of dependencies (member types, accesses, supertypes, enclosing types, annotations and generic type signatures) we can control how deeply we want to resolve each of these sort of dependencies. How many iterations we make for each such type is now configurable via `archunit.properties`. The defaults are configured in a way that the behavior is similar to before, except that the mentioned shortcomings are now fixed. I.e. we resolve one level for member types and accesses, so all the "used types" of the imported classes are present. The other types of dependencies we resolve until we have registered all needed types. That way (meta-)annotations, generic signatures, etc., are all fully resolved. This should provide a reasonable default behavior and also fix most issues that ever occurred because of inconsistent resolution behavior.
  • Loading branch information
codecholeric authored Feb 18, 2022
2 parents e678711 + efdc603 commit 5564a77
Show file tree
Hide file tree
Showing 47 changed files with 2,248 additions and 1,227 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import.dependencyResolutionProcess.maxIterationsForEnclosingTypes=0
import.dependencyResolutionProcess.maxIterationsForGenericSignatureTypes=0
extension.archunit-example-extension.enabled=false
extension.archunit-example-extension.example-prop=exampleValue
freeze.store.default.path=../archunit-example/example-plain/src/test/resources/frozen
2 changes: 2 additions & 0 deletions archunit-junit/junit4/src/test/resources/archunit.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import.dependencyResolutionProcess.maxIterationsForEnclosingTypes=0
import.dependencyResolutionProcess.maxIterationsForGenericSignatureTypes=0
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import.dependencyResolutionProcess.maxIterationsForEnclosingTypes=0
import.dependencyResolutionProcess.maxIterationsForGenericSignatureTypes=0

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.tngtech.archunit.core.importer.testexamples.annotations;

public @interface AnotherAnnotationWithAnnotationParameter {
SomeAnnotationWithAnnotationParameter value();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.tngtech.archunit.core.importer.testexamples.annotations;

public @interface SomeAnnotationWithAnnotationParameter {
SomeAnnotationWithClassParameter value();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.tngtech.archunit.core.importer.testexamples.annotations;

public @interface SomeAnnotationWithClassParameter {
Class<?> value();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.tngtech.archunit.core.importer.testexamples.classhierarchy;

public class Child extends Parent implements ParentInterfaceDirect {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.tngtech.archunit.core.importer.testexamples.classhierarchy;

public class GrandParent implements ParentInterfaceIndirect {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.tngtech.archunit.core.importer.testexamples.classhierarchy;

public interface GrandParentInterfaceDirect {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.tngtech.archunit.core.importer.testexamples.classhierarchy;

public interface GrandParentInterfaceIndirect {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.tngtech.archunit.core.importer.testexamples.classhierarchy;

public class Parent extends GrandParent {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.tngtech.archunit.core.importer.testexamples.classhierarchy;

public interface ParentInterfaceDirect extends GrandParentInterfaceDirect {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.tngtech.archunit.core.importer.testexamples.classhierarchy;

public interface ParentInterfaceIndirect extends GrandParentInterfaceIndirect {
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public Set<JavaClass> get() {
private Map<String, JavaAnnotation<JavaClass>> annotations = emptyMap();
private JavaClassDependencies javaClassDependencies = new JavaClassDependencies(this); // just for stubs; will be overwritten for imported classes
private ReverseDependencies reverseDependencies = ReverseDependencies.EMPTY; // just for stubs; will be overwritten for imported classes
private final CompletionProcess completionProcess = CompletionProcess.start();
private final CompletionProcess completionProcess;

JavaClass(JavaClassBuilder builder) {
source = checkNotNull(builder.getSource());
Expand All @@ -151,6 +151,7 @@ public Set<JavaClass> get() {
reflectSupplier = Suppliers.memoize(new ReflectClassSupplier());
sourceCodeLocation = SourceCodeLocation.of(this);
javaPackage = JavaPackage.simple(this);
completionProcess = builder.isStub() ? CompletionProcess.stub() : CompletionProcess.start();
}

/**
Expand Down Expand Up @@ -648,6 +649,11 @@ public List<JavaTypeVariable<JavaClass>> getTypeParameters() {
return typeParameters;
}

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

@PublicAPI(usage = ACCESS)
public Set<ReferencedClassObject> getReferencedClassObjects() {
return members.getReferencedClassObjects();
Expand Down Expand Up @@ -1545,6 +1551,19 @@ public static List<String> namesOf(Iterable<Class<?>> paramTypes) {
return formatNamesOf(paramTypes);
}

/**
* @return all throws declarations on any method or constructor of this class
* (e.g. {@code void method() throws SomeException})
*/
@PublicAPI(usage = ACCESS)
public Set<ThrowsDeclaration<? extends JavaCodeUnit>> getThrowsDeclarations() {
ImmutableSet.Builder<ThrowsDeclaration<? extends JavaCodeUnit>> result = ImmutableSet.builder();
for (JavaCodeUnit codeUnit : getCodeUnits()) {
result.addAll(codeUnit.getThrowsClause());
}
return result.build();
}

private static class Superclass {
private static final Superclass ABSENT = new Superclass(Optional.<JavaType>empty());

Expand Down Expand Up @@ -1646,7 +1665,74 @@ static EnclosingDeclaration ofClass(Optional<JavaClass> clazz) {
}
}

private static class CompletionProcess {
private abstract static class CompletionProcess {
private static final CompletionProcess stubCompletionProcess = new CompletionProcess() {
@Override
boolean hasFinished() {
return false;
}

@Override
public void markClassHierarchyComplete() {
}

@Override
public void markEnclosingDeclarationComplete() {
}

@Override
public void markTypeParametersComplete() {
}

@Override
public void markGenericSuperclassComplete() {
}

@Override
public void markGenericInterfacesComplete() {
}

@Override
public void markMembersComplete() {
}

@Override
public void markAnnotationsComplete() {
}

@Override
public void markDependenciesComplete() {
}
};

abstract boolean hasFinished();

public abstract void markClassHierarchyComplete();

public abstract void markEnclosingDeclarationComplete();

public abstract void markTypeParametersComplete();

public abstract void markGenericSuperclassComplete();

public abstract void markGenericInterfacesComplete();

public abstract void markMembersComplete();

public abstract void markAnnotationsComplete();

public abstract void markDependenciesComplete();

static CompletionProcess start() {
return new FullCompletionProcess();
}

static CompletionProcess stub() {
return stubCompletionProcess;
}
}

private static class FullCompletionProcess extends CompletionProcess {
private boolean classHierarchyComplete = false;
private boolean enclosingDeclarationComplete = false;
private boolean typeParametersComplete = false;
Expand All @@ -1656,9 +1742,7 @@ private static class CompletionProcess {
private boolean annotationsComplete = false;
private boolean dependenciesComplete = false;

private CompletionProcess() {
}

@Override
boolean hasFinished() {
return classHierarchyComplete
&& enclosingDeclarationComplete
Expand All @@ -1670,41 +1754,45 @@ boolean hasFinished() {
&& dependenciesComplete;
}

@Override
public void markClassHierarchyComplete() {
this.classHierarchyComplete = true;
}

@Override
public void markEnclosingDeclarationComplete() {
this.enclosingDeclarationComplete = true;
}

@Override
public void markTypeParametersComplete() {
this.typeParametersComplete = true;
}

@Override
public void markGenericSuperclassComplete() {
this.genericSuperclassComplete = true;
}

@Override
public void markGenericInterfacesComplete() {
this.genericInterfacesComplete = true;
}

@Override
public void markMembersComplete() {
this.membersComplete = true;
}

@Override
public void markAnnotationsComplete() {
this.annotationsComplete = true;
}

@Override
public void markDependenciesComplete() {
this.dependenciesComplete = true;
}

static CompletionProcess start() {
return new CompletionProcess();
}
}

public static final class Functions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,8 @@ private Set<Dependency> genericParameterTypeArgumentDependencies(JavaCodeUnit co

private Set<Dependency> throwsDeclarationDependenciesFromSelf() {
ImmutableSet.Builder<Dependency> result = ImmutableSet.builder();
for (JavaCodeUnit codeUnit : javaClass.getCodeUnits()) {
for (ThrowsDeclaration<? extends JavaCodeUnit> throwsDeclaration : codeUnit.getThrowsClause()) {
result.addAll(Dependency.tryCreateFromThrowsDeclaration(throwsDeclaration));
}
for (ThrowsDeclaration<? extends JavaCodeUnit> throwsDeclaration : javaClass.getThrowsDeclarations()) {
result.addAll(Dependency.tryCreateFromThrowsDeclaration(throwsDeclaration));
}
return result.build();
}
Expand All @@ -206,10 +204,8 @@ private Set<Dependency> annotationDependenciesFromSelf() {

private Set<Dependency> instanceofCheckDependenciesFromSelf() {
ImmutableSet.Builder<Dependency> result = ImmutableSet.builder();
for (JavaCodeUnit codeUnit : javaClass.getCodeUnits()) {
for (InstanceofCheck instanceofCheck : codeUnit.getInstanceofChecks()) {
result.addAll(Dependency.tryCreateFromInstanceofCheck(instanceofCheck));
}
for (InstanceofCheck instanceofCheck : javaClass.getInstanceofChecks()) {
result.addAll(Dependency.tryCreateFromInstanceofCheck(instanceofCheck));
}
return result.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,14 @@ Set<JavaEnumConstant> getEnumConstants() {
return result.build();
}

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

Set<ReferencedClassObject> getReferencedClassObjects() {
ImmutableSet.Builder<ReferencedClassObject> result = ImmutableSet.builder();
for (JavaCodeUnit codeUnit : codeUnits) {
Expand Down
Loading

0 comments on commit 5564a77

Please sign in to comment.