diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/conditions/AnyTransitiveDependencyCondition.java b/archunit/src/main/java/com/tngtech/archunit/lang/conditions/AnyTransitiveDependencyCondition.java new file mode 100644 index 0000000000..aea644bcdc --- /dev/null +++ b/archunit/src/main/java/com/tngtech/archunit/lang/conditions/AnyTransitiveDependencyCondition.java @@ -0,0 +1,112 @@ +package com.tngtech.archunit.lang.conditions; + +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import com.google.common.collect.ImmutableList; +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.Dependency; +import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.lang.ArchCondition; +import com.tngtech.archunit.lang.ConditionEvent; +import com.tngtech.archunit.lang.ConditionEvents; +import com.tngtech.archunit.lang.SimpleConditionEvent; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.Iterables.getLast; +import static java.util.stream.Collectors.joining; + +public class AnyTransitiveDependencyCondition extends ArchCondition { + + private final DescribedPredicate conditionPredicate; + private final TransitiveDependencyPath transitiveDependencyPath = new TransitiveDependencyPath(); + private Collection allClasses; + + public AnyTransitiveDependencyCondition(DescribedPredicate conditionPredicate) { + super("transitively depend on any classes that " + conditionPredicate.getDescription()); + + this.conditionPredicate = checkNotNull(conditionPredicate); + } + + @Override + public void init(Collection allObjectsToTest) { + this.allClasses = allObjectsToTest; + } + + @Override + public void check(JavaClass javaClass, ConditionEvents events) { + boolean hasTransitiveDependency = false; + for (JavaClass dependency : getDirectDependencies(javaClass)) { + if (!allClasses.contains(dependency)) { + List dependencyPath = transitiveDependencyPath.findFirstPathToTransitiveDependency(dependency); + if (!dependencyPath.isEmpty()) { + events.add(newTransitivePathFoundEvent(javaClass, dependencyPath)); + hasTransitiveDependency = true; + } + } + } + if (!hasTransitiveDependency) { + events.add(noTransitivePathFoundEvent(javaClass)); + } + } + + private static ConditionEvent newTransitivePathFoundEvent(JavaClass selected, List transitivePath) { + String message = + String.format("Class <%s> transitively depends on <%s>", selected.getFullName(), getLast(transitivePath).getFullName()); + + if (transitivePath.size() > 1) { + message += " by [" + transitivePath.stream().map(JavaClass::getName).collect(joining("->")) + "]"; + } + message += " in " + selected.getSourceCodeLocation(); + return SimpleConditionEvent.satisfied(selected, message); + } + + private static ConditionEvent noTransitivePathFoundEvent(JavaClass selected) { + return SimpleConditionEvent.violated(selected, + String.format("Class <%s> not transitively depends on any matching class", selected.getFullName())); + } + + private static Set getDirectDependencies(JavaClass item) { + Set directDependencies = new HashSet<>(); + for (Dependency dependency : item.getDirectDependenciesFromSelf()) { + directDependencies.add(dependency.getTargetClass().getBaseComponentType()); + } + return directDependencies; + } + + private class TransitiveDependencyPath { + /** + * @return the first dependency path to a matching class or empty if there is none + */ + List findFirstPathToTransitiveDependency(JavaClass clazz) { + ImmutableList.Builder transitivePath = ImmutableList.builder(); + addDependenciesToPathFrom(clazz, transitivePath, new HashSet<>(allClasses)); + return transitivePath.build().reverse(); + } + + private boolean addDependenciesToPathFrom( + JavaClass clazz, + ImmutableList.Builder dependencyPath, + Set analyzedClasses + ) { + if (conditionPredicate.test(clazz)) { + dependencyPath.add(clazz); + return true; + } + + analyzedClasses.add(clazz); + + for (JavaClass directDependency : getDirectDependencies(clazz)) { + if (!analyzedClasses.contains(directDependency) + && addDependenciesToPathFrom(directDependency, dependencyPath, analyzedClasses)) { + dependencyPath.add(clazz); + return true; + } + } + + return false; + } + } +} diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java b/archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java index 1bebc7e9d0..9c7a12925e 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/conditions/ArchConditions.java @@ -309,6 +309,11 @@ public static ArchCondition transitivelyDependOnClassesThat(final Des GET_TRANSITIVE_DEPENDENCIES_FROM_SELF); } + @PublicAPI(usage = ACCESS) + public static ArchCondition transitivelyDependOnAnyClassesThat(final DescribedPredicate predicate) { + return new AnyTransitiveDependencyCondition(predicate); + } + @PublicAPI(usage = ACCESS) public static ArchCondition onlyDependOnClassesThat(final DescribedPredicate predicate) { return new AllDependenciesCondition( diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/ClassesShouldInternal.java b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/ClassesShouldInternal.java index d0b87f5ad3..848176ee99 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/ClassesShouldInternal.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/ClassesShouldInternal.java @@ -524,6 +524,16 @@ public ClassesShouldConjunction onlyDependOnClassesThat(DescribedPredicate transitivelyDependOnAnyClassesThat() { + return new ClassesThatInternal<>(predicate -> addCondition(ArchConditions.transitivelyDependOnAnyClassesThat(predicate))); + } + + @Override + public ClassesShouldConjunction transitivelyDependOnAnyClassesThat(DescribedPredicate predicate) { + return addCondition(ArchConditions.transitivelyDependOnAnyClassesThat(predicate)); + } + @Override public ClassesThat transitivelyDependOnClassesThat() { return new ClassesThatInternal<>(predicate -> addCondition(ArchConditions.transitivelyDependOnClassesThat(predicate))); diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/ClassesShould.java b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/ClassesShould.java index 853699cd6c..330c864e78 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/ClassesShould.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/elements/ClassesShould.java @@ -1015,6 +1015,48 @@ public interface ClassesShould { @PublicAPI(usage = ACCESS) ClassesShouldConjunction onlyDependOnClassesThat(DescribedPredicate predicate); + /** + * Asserts that all classes selected by this rule transitively depend on any matching classes.
+ * This is a much more efficient variant of {@link #transitivelyDependOnClassesThat()} that can be used to detect transitive dependencies in a + * huge codebase or to classes in large 3rd-party libraries like the Android SDK. + * It focuses on detecting all direct dependencies of the selected classes that are themselves matched or have any + * transitive dependencies on matched classes. Thus, it doesn't discover all possible dependency paths but stops at the first match to be fast and + * resource-friendly.
+ * NOTE: This usually makes more sense the negated way, e.g. + *

+ *


+     * {@link ArchRuleDefinition#noClasses() noClasses()}.{@link GivenClasses#should() should()}.{@link #transitivelyDependOnAnyClassesThat()}.{@link ClassesThat#haveFullyQualifiedName(String) haveFullyQualifiedName(String)}
+     * 
+ * + * NOTE: 'dependOn' catches wide variety of violations, e.g. having fields of type, having method parameters of type, extending type etc... + * + * @return A syntax element that allows choosing to which classes a transitive dependency should exist + */ + @PublicAPI(usage = ACCESS) + ClassesThat transitivelyDependOnAnyClassesThat(); + + + /** + * Asserts that all classes selected by this rule transitively depend on any matching classes.
+ * This is a much more efficient variant of {@link #transitivelyDependOnClassesThat()} that can be used to detect transitive dependencies in a + * huge codebase or to classes in large 3rd-party libraries like the Android SDK. + * It focuses on detecting all direct dependencies of the selected classes that are themselves matched or have any + * transitive dependencies on matched classes. Thus, it doesn't discover all possible dependency paths but stops at the first match to be fast and + * resource-friendly.
+ * NOTE: This usually makes more sense the negated way, e.g. + *

+ *


+     * {@link ArchRuleDefinition#noClasses() noClasses()}.{@link GivenClasses#should() should()}.{@link #transitivelyDependOnAnyClassesThat(DescribedPredicate) transitivelyDependOnAnyClassesThat(myPredicate)}
+     * 
+ * + * NOTE: 'dependOn' catches wide variety of violations, e.g. having fields of type, having method parameters of type, extending type etc... + * + * @param predicate Determines which {@link JavaClass JavaClasses} match the dependency target + * @return A syntax element that can either be used as working rule, or to continue specifying a more complex rule + */ + @PublicAPI(usage = ACCESS) + ClassesShouldConjunction transitivelyDependOnAnyClassesThat(DescribedPredicate predicate); + /** * Asserts that all classes selected by this rule transitively depend on certain classes.
* NOTE: This usually makes more sense the negated way, e.g. diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/ShouldClassesThatTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/ShouldClassesThatTest.java index de43a0c458..bfca89989b 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/ShouldClassesThatTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/ShouldClassesThatTest.java @@ -13,6 +13,7 @@ import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaClass.Predicates; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.domain.properties.HasName; import com.tngtech.archunit.core.domain.properties.HasType; @@ -36,7 +37,6 @@ import static com.tngtech.archunit.core.domain.properties.HasName.Functions.GET_NAME; import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name; import static com.tngtech.archunit.core.domain.properties.HasType.Functions.GET_RAW_TYPE; -import static com.tngtech.archunit.lang.conditions.ArchConditions.fullyQualifiedName; import static com.tngtech.archunit.lang.conditions.ArchPredicates.are; import static com.tngtech.archunit.lang.conditions.ArchPredicates.have; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; @@ -393,7 +393,8 @@ public void containAnyMembersThat(ClassesThat noClasse .on(Data_of_containAnyMembersThat.OkayOrigin.class, Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.OkayTarget.class, Data_of_containAnyMembersThat.ViolatingTarget.class); - assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.ViolatingTarget.class); + assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, + Data_of_containAnyMembersThat.ViolatingTarget.class); } @Test @@ -403,7 +404,8 @@ public void containAnyFieldsThat(ClassesThat noClasses .on(Data_of_containAnyMembersThat.OkayOrigin.class, Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.OkayTarget.class, Data_of_containAnyMembersThat.ViolatingTarget.class); - assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.ViolatingTarget.class); + assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, + Data_of_containAnyMembersThat.ViolatingTarget.class); } @Test @@ -413,7 +415,8 @@ public void containAnyCodeUnitsThat(ClassesThat noClas .on(Data_of_containAnyMembersThat.OkayOrigin.class, Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.OkayTarget.class, Data_of_containAnyMembersThat.ViolatingTarget.class); - assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.ViolatingTarget.class); + assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, + Data_of_containAnyMembersThat.ViolatingTarget.class); } @Test @@ -423,7 +426,8 @@ public void containAnyMethodsThat(ClassesThat noClasse .on(Data_of_containAnyMembersThat.OkayOrigin.class, Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.OkayTarget.class, Data_of_containAnyMembersThat.ViolatingTarget.class); - assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.ViolatingTarget.class); + assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, + Data_of_containAnyMembersThat.ViolatingTarget.class); } @Test @@ -434,7 +438,8 @@ public void containAnyConstructorsThat(ClassesThat noC .on(Data_of_containAnyMembersThat.OkayOrigin.class, Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.OkayTarget.class, Data_of_containAnyMembersThat.ViolatingTarget.class); - assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, Data_of_containAnyMembersThat.ViolatingTarget.class); + assertThatTypes(classes).matchInAnyOrder(Data_of_containAnyMembersThat.ViolatingOrigin.class, + Data_of_containAnyMembersThat.ViolatingTarget.class); } @Test @@ -446,7 +451,8 @@ public void containAnyStaticInitializersThat(ClassesThat testClass1 = TransitivelyDependOnClassesThatTestCases.TestClass1.class; + Class testClass2 = TransitivelyDependOnClassesThatTestCases.TestClass2.class; + Class directlyDependentClass1 = TransitivelyDependOnClassesThatTestCases.DirectlyDependentClass1.class; + Class directlyDependentClass2 = TransitivelyDependOnClassesThatTestCases.DirectlyDependentClass2.class; + Class directlyDependentClass3 = TransitivelyDependOnClassesThatTestCases.DirectlyDependentClass3.class; + Class level1TransitivelyDependentClass1 = TransitivelyDependOnClassesThatTestCases.Level1TransitivelyDependentClass1.class; + Class level2TransitivelyDependentClass1 = TransitivelyDependOnClassesThatTestCases.Level2TransitivelyDependentClass1.class; + Class level2TransitivelyDependentClass2 = TransitivelyDependOnClassesThatTestCases.Level2TransitivelyDependentClass2.class; + Class[] matchingTransitivelyDependentClasses = + new Class[]{level2TransitivelyDependentClass1, level2TransitivelyDependentClass2, directlyDependentClass3}; + + JavaClasses classes = new ClassFileImporter().importClasses( + testClass1, + testClass2, + directlyDependentClass1, + directlyDependentClass2, + directlyDependentClass3, + level1TransitivelyDependentClass1, + level2TransitivelyDependentClass1, + level2TransitivelyDependentClass2 + ); + + ClassesShould noClassesShould = noClasses().that().haveSimpleNameStartingWith("TestClass").should(); + ArchRule rule = viaPredicate + ? noClassesShould.transitivelyDependOnAnyClassesThat(Predicates.belongToAnyOf(matchingTransitivelyDependentClasses)) + : noClassesShould.transitivelyDependOnAnyClassesThat().belongToAnyOf(matchingTransitivelyDependentClasses); + + assertThatRule(rule).checking(classes) + .hasViolations(3) + .hasViolationMatching(String.format(".*<%s> transitively depends on <(?:%s|%s)> by \\[%s->.*\\] in .*", + quote(testClass1.getName()), + quote(level2TransitivelyDependentClass1.getName()), + quote(level2TransitivelyDependentClass2.getName()), + quote(directlyDependentClass2.getName()) + )) + .hasViolationMatching(String.format(".*<%s> transitively depends on <%s> by \\[%s->%s->%s\\] in .*", + quote(testClass1.getName()), + quote(level2TransitivelyDependentClass1.getName()), + quote(directlyDependentClass1.getName()), + quote(level1TransitivelyDependentClass1.getName()), + quote(level2TransitivelyDependentClass1.getName()) + )) + .hasViolationMatching(String.format(".*<%s> transitively depends on <%s> in .*", + quote(testClass1.getName()), + quote(directlyDependentClass3.getName()) + )); } @Test @DataProvider(value = {"true", "false"}) public void transitivelyDependOnClassesThat_reports_all_transitive_dependencies(boolean viaPredicate) { - Class testClass = TransitivelyDependOnClassesThatTestCases.TestClass.class; + Class testClass1 = TransitivelyDependOnClassesThatTestCases.TestClass1.class; + Class testClass2 = TransitivelyDependOnClassesThatTestCases.TestClass2.class; Class directlyDependentClass1 = TransitivelyDependOnClassesThatTestCases.DirectlyDependentClass1.class; Class directlyDependentClass2 = TransitivelyDependOnClassesThatTestCases.DirectlyDependentClass2.class; - Class transitivelyDependentClass = TransitivelyDependOnClassesThatTestCases.TransitivelyDependentClass.class; + Class directlyDependentClass3 = TransitivelyDependOnClassesThatTestCases.DirectlyDependentClass3.class; + Class level1TransitivelyDependentClass1 = TransitivelyDependOnClassesThatTestCases.Level1TransitivelyDependentClass1.class; + Class level2TransitivelyDependentClass1 = TransitivelyDependOnClassesThatTestCases.Level2TransitivelyDependentClass1.class; + Class level2TransitivelyDependentClass2 = TransitivelyDependOnClassesThatTestCases.Level2TransitivelyDependentClass2.class; + Class[] matchingTransitivelyDependentClasses = + new Class[]{level2TransitivelyDependentClass1, level2TransitivelyDependentClass2, directlyDependentClass3}; + JavaClasses classes = new ClassFileImporter().importClasses( - testClass, directlyDependentClass1, directlyDependentClass2, transitivelyDependentClass + testClass1, + testClass2, + directlyDependentClass1, + directlyDependentClass2, + directlyDependentClass3, + level1TransitivelyDependentClass1, + level2TransitivelyDependentClass1, + level2TransitivelyDependentClass2 ); - ClassesShould noClassesShould = noClasses().that().haveFullyQualifiedName(testClass.getName()).should(); + ClassesShould noClassesShould = noClasses().that().haveSimpleNameStartingWith("TestClass").should(); ArchRule rule = viaPredicate - ? noClassesShould.transitivelyDependOnClassesThat(have(fullyQualifiedName(transitivelyDependentClass.getName()))) - : noClassesShould.transitivelyDependOnClassesThat().haveFullyQualifiedName(transitivelyDependentClass.getName()); + ? noClassesShould.transitivelyDependOnClassesThat(Predicates.belongToAnyOf(matchingTransitivelyDependentClasses)) + : noClassesShould.transitivelyDependOnClassesThat().belongToAnyOf(matchingTransitivelyDependentClasses); assertThatRule(rule).checking(classes) - .hasViolations(2) + .hasViolations(8) + .hasViolationMatching(String.format(".*%s\\.%s.* has type .*%s.*", + quote(level1TransitivelyDependentClass1.getName()), "transitiveDependency1", quote(level2TransitivelyDependentClass1.getName()) + )) + .hasViolationMatching(String.format(".*%s\\.%s.* has type .*%s.*", + quote(directlyDependentClass2.getName()), "transitiveDependency2", quote(level2TransitivelyDependentClass2.getName()) + )) .hasViolationMatching(String.format(".*%s\\.%s.* has type .*%s.*", - quote(directlyDependentClass1.getName()), "transitiveDependency1", quote(transitivelyDependentClass.getName()) + quote(level2TransitivelyDependentClass2.getName()), "transitiveDependency1", quote(level2TransitivelyDependentClass1.getName()) )) .hasViolationMatching(String.format(".*%s\\.%s.* has type .*%s.*", - quote(directlyDependentClass2.getName()), "transitiveDependency2", quote(transitivelyDependentClass.getName()) + quote(testClass1.getName()), "directDependency3", quote(directlyDependentClass3.getName()) )); }