Review: The methods that return "direct dependencies" only return dep…

…endencies of first order. I.e. we do not need to recursively find annotations on annotation parameter types or meta-annotations. This is consistent to all other dependencies, e.g. we do not depend on the super type of a return type of a method, since it is no "direct" dependency, but a transitive one. This also solves the problem of a recursive cycle, because we simply do not need to follow those paths. Issue: #136 Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
TNG · Jan 9, 2020 · fea4642 · fea4642
1 parent 3fe77a3
commit fea4642
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 18 deletions.
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java
@@ -1039,7 +1039,6 @@ private <T extends HasDescription & HasAnnotations<?>> Set<Dependency> annotatio
         for (JavaAnnotation<?> annotation : annotated.getAnnotations()) {
             result.add(Dependency.fromAnnotation(annotation));
             result.addAll(annotationParametersDependencies(annotation));
-            result.addAll(annotationDependencies(annotation.getRawType()));
         }
         return result.build();
     }
@@ -1067,11 +1066,9 @@ private Set<Dependency> annotationParameterDependencies(JavaAnnotation<?> origin
         if (value instanceof JavaClass) {
             JavaClass annotationMember = (JavaClass) value;
             result.add(Dependency.fromAnnotationMember(origin, annotationMember));
-            result.addAll(annotationDependencies(annotationMember));
         } else if (value instanceof JavaAnnotation<?>) {
             JavaAnnotation<?> nestedAnnotation = (JavaAnnotation<?>) value;
             result.add(Dependency.fromAnnotationMember(origin, nestedAnnotation.getRawType()));
-            result.addAll(annotationDependencies(nestedAnnotation.getRawType()));
             result.addAll(annotationParametersDependencies(nestedAnnotation));
         }
         return result.build();

diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java
@@ -9,6 +9,7 @@
 import java.util.List;
 import java.util.Set;
 
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.tngtech.archunit.base.ArchUnitException.InvalidSyntaxUsageException;
@@ -39,6 +40,9 @@
 import org.junit.runner.RunWith;
 
 import static com.google.common.collect.Iterables.getOnlyElement;
+import static com.tngtech.archunit.base.Guava.toGuava;
+import static com.tngtech.archunit.core.domain.Dependency.Functions.GET_ORIGIN_CLASS;
+import static com.tngtech.archunit.core.domain.Dependency.Functions.GET_TARGET_CLASS;
 import static com.tngtech.archunit.core.domain.JavaClass.Functions.GET_CODE_UNITS;
 import static com.tngtech.archunit.core.domain.JavaClass.Functions.GET_CONSTRUCTORS;
 import static com.tngtech.archunit.core.domain.JavaClass.Functions.GET_FIELDS;
@@ -66,6 +70,7 @@
 import static com.tngtech.archunit.core.domain.TestUtils.simulateCall;
 import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name;
 import static com.tngtech.archunit.testutil.Assertions.assertThat;
+import static com.tngtech.archunit.testutil.Assertions.assertThatClasses;
 import static com.tngtech.archunit.testutil.Conditions.codeUnitWithSignature;
 import static com.tngtech.archunit.testutil.Conditions.containing;
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.getHierarchy;
@@ -447,7 +452,20 @@ public void direct_dependencies_from_self_by_annotation() {
                         .to(B.class)
                         .inLineNumber(0))
         ;
-        // TODO test that annotation dependencies do not lead to a infinite loop
+    }
+
+    @Test
+    public void direct_dependencies_from_self_finds_correct_set_of_target_types() {
+        JavaClass javaClass = importPackagesOf(getClass()).get(ClassWithAnnotationDependencies.class);
+
+        Set<JavaClass> targets = FluentIterable.from(javaClass.getDirectDependenciesFromSelf())
+                .transform(toGuava(GET_TARGET_CLASS)).toSet();
+
+        assertThatClasses(targets).matchInAnyOrder(
+                B.class, AhavingMembersOfTypeB.class, Object.class, String.class,
+                List.class, Serializable.class, SomeSuperClass.class,
+                WithType.class, WithNestedAnnotations.class, OnClass.class,
+                OnMethod.class, OnConstructor.class, OnField.class, MetaAnnotated.class);
     }
 
     @Test
@@ -559,6 +577,25 @@ public void direct_dependencies_to_self_by_annotation() {
                         .inLineNumber(0));
     }
 
+    @Test
+    public void direct_dependencies_to_self_finds_correct_set_of_origin_types() {
+        JavaClasses classes = importPackagesOf(getClass());
+
+        Set<JavaClass> origins = getOriginsOfDependenciesTo(classes.get(WithType.class));
+
+        assertThatClasses(origins).matchInAnyOrder(ClassWithAnnotationDependencies.class, OnMethodParam.class);
+
+        origins = getOriginsOfDependenciesTo(classes.get(B.class));
+
+        assertThatClasses(origins).matchInAnyOrder(
+                ClassWithAnnotationDependencies.class, OnMethodParam.class, AAccessingB.class, AhavingMembersOfTypeB.class);
+    }
+
+    private Set<JavaClass> getOriginsOfDependenciesTo(JavaClass withType) {
+        return FluentIterable.from(withType.getDirectDependenciesToSelf())
+                .transform(toGuava(GET_ORIGIN_CLASS)).toSet();
+    }
+
     @Test
     public void function_getSimpleName() {
         assertThat(JavaClass.Functions.GET_SIMPLE_NAME.apply(importClassWithContext(List.class)))
@@ -1260,42 +1297,58 @@ private class NestedNamedInnerClass {
         }
     }
 
+    private static class SomeSuperClass {
+    }
+
     @OnClass
     @WithNestedAnnotations(
+            outerType = AhavingMembersOfTypeB.class,
             nested = {
                     @WithType(type = B.class)
             }
     )
     @MetaAnnotated
-    public class ClassWithAnnotationDependencies {
+    public static class ClassWithAnnotationDependencies extends SomeSuperClass {
         @OnField
         Object field;
 
         @OnConstructor
-        ClassWithAnnotationDependencies() {}
+        ClassWithAnnotationDependencies(Serializable param) {
+        }
 
         @OnMethod
-        void method() {
+        List<?> method() {
+            return null;
         }
 
-        void method(@OnMethodParam Object obj) {
+        void method(@OnMethodParam String param) {
         }
     }
 
     @interface OnClass {}
     @interface OnField {}
     @interface OnConstructor {}
     @interface OnMethod {}
+
+    @WithType(type = B.class)
     @interface OnMethodParam {}
+
+    @Retention(RUNTIME)
     @interface WithType { Class<?> type(); }
+
+    @Retention(RUNTIME)
     @interface WithNestedAnnotations {
+        Class<?> outerType();
+
         WithType[] nested();
     }
 
+    @Retention(RUNTIME)
     @MetaAnnotation
     @interface MetaAnnotation {
     }
 
+    @Retention(RUNTIME)
     @MetaAnnotation
     @interface MetaAnnotated {
     }

diff --git a/...nit/src/test/java/com/tngtech/archunit/core/domain/testobjects/AhavingMembersOfTypeB.java b/...nit/src/test/java/com/tngtech/archunit/core/domain/testobjects/AhavingMembersOfTypeB.java
@@ -1,5 +1,7 @@
 package com.tngtech.archunit.core.domain.testobjects;
 
+@DomainAnnotation
+@SuppressWarnings({"RedundantThrows", "unused"})
 public class AhavingMembersOfTypeB {
     private B b;
 

diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/testobjects/B.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/testobjects/B.java
@@ -1,5 +1,6 @@
 package com.tngtech.archunit.core.domain.testobjects;
 
+@DomainAnnotation
 public class B {
     String field;
 

diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/testobjects/DomainAnnotation.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/testobjects/DomainAnnotation.java
@@ -0,0 +1,9 @@
+package com.tngtech.archunit.core.domain.testobjects;
+
+import java.lang.annotation.Retention;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+@Retention(RUNTIME)
+public @interface DomainAnnotation {
+}
diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java
@@ -226,6 +226,7 @@
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.constructor;
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.field;
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.method;
+import static com.tngtech.archunit.testutil.TestUtils.namesOf;
 import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assume.assumeTrue;
@@ -2253,14 +2254,6 @@ private Set<Integer> lineNumbersOf(Set<JavaFieldAccess> fieldAccesses) {
         return result;
     }
 
-    private Set<String> namesOf(Iterable<? extends HasName> thingsWithNames) {
-        Set<String> result = new HashSet<>();
-        for (HasName hasName : thingsWithNames) {
-            result.add(hasName.getName());
-        }
-        return result;
-    }
-
     private <T extends HasName> Set<T> getByName(Iterable<T> thingsWithName, String name) {
         Set<T> result = new HashSet<>();
         for (T hasName : thingsWithName) {

diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/Assertions.java b/archunit/src/test/java/com/tngtech/archunit/testutil/Assertions.java
@@ -279,7 +279,7 @@ public void matchInAnyOrder(Class<?>... classes) {
         }
 
         public void matchExactly(Class<?>... classes) {
-            assertThat((Object[]) actual).as("classes").hasSize(classes.length);
+            assertThat(TestUtils.namesOf(actual)).as("classes").containsExactlyElementsOf(namesOf(classes));
             for (int i = 0; i < actual.length; i++) {
                 assertThat(actual[i]).as("Element %d", i).matches(classes[i]);
             }

diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java b/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java
@@ -2,9 +2,13 @@
 
 import java.io.File;
 import java.lang.reflect.Method;
+import java.util.LinkedHashSet;
 import java.util.Properties;
 import java.util.Random;
+import java.util.Set;
 
+import com.google.common.collect.ImmutableSet;
+import com.tngtech.archunit.core.domain.properties.HasName;
 import org.assertj.core.util.Files;
 
 import static com.google.common.base.Preconditions.checkArgument;
@@ -51,4 +55,16 @@ public static Properties properties(String... keyValues) {
         }
         return result;
     }
+
+    public static Set<String> namesOf(HasName[] thingsWithNames) {
+        return namesOf(ImmutableSet.copyOf(thingsWithNames));
+    }
+
+    public static Set<String> namesOf(Iterable<? extends HasName> thingsWithNames) {
+        Set<String> result = new LinkedHashSet<>();
+        for (HasName hasName : thingsWithNames) {
+            result.add(hasName.getName());
+        }
+        return result;
+    }
 }
diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/JavaAnnotationAssertion.java b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/JavaAnnotationAssertion.java
@@ -20,7 +20,7 @@ public class JavaAnnotationAssertion {
     public static Set<Map<String, Object>> propertiesOf(Set<? extends JavaAnnotation<?>> annotations) {
         List<Annotation> converted = new ArrayList<>();
         for (JavaAnnotation<?> annotation : annotations) {
-            converted.add(annotation.as((Class) annotation.getType().reflect()));
+            converted.add(annotation.as((Class) annotation.getRawType().reflect()));
         }
         return propertiesOf(converted.toArray(new Annotation[0]));
     }