-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unused local variable declarations seem not to be considered dependencies #1137
Comments
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 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 log.info("Exception from ex", ex); is just an |
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 |
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 |
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 importsimport 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 |
We need this functionality for reliable extraction of dependencies from bytecode. Please, implement this. Preferably we would like the types of caught exceptions to be present in It also appears that types used in
It would be great if these were reported as well. |
Another thing that seems to be missing is array creation: |
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
The text was updated successfully, but these errors were encountered: