ArchUnit
ArchUnit copied to clipboard
Unused local variable declarations seem not to be considered dependencies
Unused local variable declarations or unused objects seem not to be considered dependencies, neither are their corresponding import statements. Is this a limitation of ArchUnit, a bug, or am I missing something? Only in case of an exception, there is a functional dependency. For me this became apparent when trying to modularize an application whose boundaries were set by ArchUnit. I needed to refactor the code that was catching a dependency which it ought not to have depended on, which for me was rather unexpected.
Please see: https://github.com/AukeHaanstra/ArchUnitIssue/blob/main/src/test/java/org/example/DependencyAssertionsTest.java
Thanks for raising the issue and providing the example project; that's very helpful! 💚
Some of the presumably missing violations are due to the fact that local variables are currently not analyzed (#768), and that import "statements" do not even exist at byte code level. Therefore your classes F
and H
don't have dependencies to A
nor Ex
:
class F {
void doF() {
A a = null; // dependency NOT detected
}
}
class H {
public void doH() {
new B().giveEx(); // dependency NOT detected
Ex ex = new B().giveEx(); // dependency NOT detected
}
}
ArchUnit does not detect a dependency if there is no specific reference in the byte code. Similarly to #1012, this in your class D
:
log.info("Exception from ex", ex);
is just an invokeinterface
of InterfaceMethod org/slf4j/Logger.info:(Ljava/lang/String;Ljava/lang/Throwable;)V
, but no specific dependency to Ex
.
But I'm actually unsure whether
public void doD() {
B b = new B();
try {
b.doB();
} catch (Ex ex) { // dependency NOT detected
log.info("Exception from ex", ex);
}
}
shouldn't actually have the dependency to Ex
that you expected, since classes.get(D.class).getMethod("doD").getTryCatchBlocks().iterator().next().getCaughtThrowables()
indeed contains Ex
. 🤔
So the situation is that ArchUnit could be able to detect that type of dependency, though at this moment, this has not been implemented yet?
Yes, exactly: The domain object for the TryCatchBlock
is already available, but this information was not added to the JavaClassDependencies
.
I think I might try to implement this then. It seems like a quite small and straightforward issue. Though I don't yet know about possibly complicating factors like nested or anonymous classes and nested try-catch statements. I think the first feature to implement would be for the simplest case as in my tests. Besides that I have some difficulty recognizing the ArchUnit API in the ArchUnit test suite, they seem to be white-box type and you seem to have constructed a DSL there. Is there some information on this for new developers, or is it simply in the code?
As a workaround, you can also use a user-defined condition like this:
static imports
import static com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAPackage;
import static com.tngtech.archunit.lang.SimpleConditionEvent.satisfied;
import static com.tngtech.archunit.lang.conditions.ArchConditions.dependOnClassesThat;
ArchCondition<JavaClass> dependOnClassesInPackage(String packageIdentifier) {
return dependOnClassesThat(resideInAPackage(packageIdentifier))
.or(new ArchCondition<JavaClass>("<overridden>") {
@Override
public void check(JavaClass javaClass, ConditionEvents events) {
for (JavaCodeUnit codeUnit : javaClass.getCodeUnits()) {
for (TryCatchBlock tryCatchBlock : codeUnit.getTryCatchBlocks()) {
tryCatchBlock.getCaughtThrowables().stream()
.filter(resideInAPackage(packageIdentifier))
.map(throwable -> satisfied(
codeUnit,
codeUnit.getDescription() + " catches " + throwable.getName() + " in " + tryCatchBlock.getSourceCodeLocation()
))
.forEach(events::add);
}
}
}
})
.as("depend on classes in package '%s'", packageIdentifier);
}
Ah, that's a neat workaround, thanks a lot! I updated my example tests accordingly.
I guess it would make sense to add the types found within this custom condition to be just part of JavaClassDependencies
, right?