raise error for empty violation store

warn on empty rule violation file delete empty rule violation file Signed-off-by: Masoud Kiaeeha <[email protected]> Resolves TNG#1264
maxxkia · Jan 19, 2025 · a833065 · a833065
1 parent f11ffa5
commit a833065
Show file tree

Hide file tree

Showing 12 changed files with 389 additions and 65 deletions.
diff --git a/...unit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java b/...unit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java
@@ -43,6 +43,7 @@
 import static com.tngtech.archunit.junit.internal.ArchRuleDeclaration.elementShouldBeIgnored;
 import static com.tngtech.archunit.junit.internal.ArchRuleDeclaration.toDeclarations;
 import static com.tngtech.archunit.junit.internal.ArchTestExecution.getValue;
+import static com.tngtech.archunit.junit.internal.ReflectionUtils.findAnnotation;
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
 import static java.util.stream.Collectors.toList;
@@ -53,7 +54,7 @@ final class ArchUnitRunnerInternal extends ParentRunner<ArchTestExecution> imple
 
     ArchUnitRunnerInternal(Class<?> testClass) throws InitializationError {
         super(testClass);
-        checkAnnotation(testClass);
+        checkTestRunnable(testClass);
 
         try {
             ArchUnitSystemPropertyTestFilterJunit4.filter(this);
@@ -62,12 +63,8 @@ final class ArchUnitRunnerInternal extends ParentRunner<ArchTestExecution> imple
         }
     }
 
-    private static AnalyzeClasses checkAnnotation(Class<?> testClass) {
-        AnalyzeClasses analyzeClasses = testClass.getAnnotation(AnalyzeClasses.class);
-        ArchTestInitializationException.check(analyzeClasses != null,
-                "Class %s must be annotated with @%s",
-                testClass.getSimpleName(), AnalyzeClasses.class.getSimpleName());
-        return analyzeClasses;
+    private static void checkTestRunnable(Class<?> testClass) {
+        findAnnotation(testClass, AnalyzeClasses.class);
     }
 
     @Override
@@ -180,7 +177,7 @@ private static class JUnit4ClassAnalysisRequest implements ClassAnalysisRequest
         private final AnalyzeClasses analyzeClasses;
 
         JUnit4ClassAnalysisRequest(Class<?> testClass) {
-            analyzeClasses = checkAnnotation(testClass);
+            analyzeClasses = findAnnotation(testClass, AnalyzeClasses.class);
         }
 
         @Override

diff --git a/...it-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerTest.java b/...it-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerTest.java
@@ -1,5 +1,6 @@
 package com.tngtech.archunit.junit.internal;
 
+import java.lang.annotation.Retention;
 import java.util.Set;
 
 import com.tngtech.archunit.core.domain.JavaClass;
@@ -28,6 +29,7 @@
 
 import static com.tngtech.archunit.core.domain.TestUtils.importClasses;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
@@ -49,7 +51,9 @@ public class ArchUnitRunnerTest {
     @InjectMocks
     private ArchUnitRunnerInternal runner = newRunner(SomeArchTest.class);
     @InjectMocks
-    private ArchUnitRunnerInternal runnerOfMaxTest = newRunner(MaxAnnotatedTest.class);
+    private ArchUnitRunnerInternal runnerOfMaxAnnotatedTest = newRunner(MaxAnnotatedTest.class);
+    @InjectMocks
+    private ArchUnitRunnerInternal runnerOfMetaAnnotatedTest = newRunner(MetaAnnotatedTest.class);
 
     @Before
     public void setUp() {
@@ -58,7 +62,7 @@ public void setUp() {
 
     @Test
     public void runner_creates_correct_analysis_request() {
-        runnerOfMaxTest.run(new RunNotifier());
+        runnerOfMaxAnnotatedTest.run(new RunNotifier());
 
         verify(cache).getClassesToAnalyzeFor(eq(MaxAnnotatedTest.class), analysisRequestCaptor.capture());
 
@@ -92,10 +96,26 @@ public void rejects_missing_analyze_annotation() {
         )
                 .isInstanceOf(ArchTestInitializationException.class)
                 .hasMessageContaining(Object.class.getSimpleName())
-                .hasMessageContaining("must be annotated")
+                .hasMessageContaining("is not (meta-)annotated")
                 .hasMessageContaining(AnalyzeClasses.class.getSimpleName());
     }
 
+    @Test
+    public void runner_creates_correct_analysis_request_for_meta_annotated_class() {
+        runnerOfMetaAnnotatedTest.run(new RunNotifier());
+
+        verify(cache).getClassesToAnalyzeFor(eq(MetaAnnotatedTest.class), analysisRequestCaptor.capture());
+
+        AnalyzeClasses analyzeClasses = MetaAnnotatedTest.class.getAnnotation(MetaAnnotatedTest.MetaAnalyzeClasses.class)
+                .annotationType().getAnnotation(AnalyzeClasses.class);
+        ClassAnalysisRequest analysisRequest = analysisRequestCaptor.getValue();
+        assertThat(analysisRequest.getPackageNames()).isEqualTo(analyzeClasses.packages());
+        assertThat(analysisRequest.getPackageRoots()).isEqualTo(analyzeClasses.packagesOf());
+        assertThat(analysisRequest.getLocationProviders()).isEqualTo(analyzeClasses.locations());
+        assertThat(analysisRequest.scanWholeClasspath()).as("scan whole classpath").isTrue();
+        assertThat(analysisRequest.getImportOptions()).isEqualTo(analyzeClasses.importOptions());
+    }
+
     private ArchUnitRunnerInternal newRunner(Class<?> testClass) {
         try {
             return new ArchUnitRunnerInternal(testClass);
@@ -160,4 +180,19 @@ public static class MaxAnnotatedTest {
         public static void someTest(JavaClasses classes) {
         }
     }
+
+    @MetaAnnotatedTest.MetaAnalyzeClasses
+    public static class MetaAnnotatedTest {
+        @ArchTest
+        public static void someTest(JavaClasses classes) {
+        }
+
+        @Retention(RUNTIME)
+        @AnalyzeClasses(
+                packages = {"com.foo", "com.bar"},
+                wholeClasspath = true
+        )
+        public @interface MetaAnalyzeClasses {
+        }
+    }
 }
diff --git a/...nit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java b/...nit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java
@@ -39,12 +39,13 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.tngtech.archunit.junit.internal.DisplayNameResolver.determineDisplayName;
+import static com.tngtech.archunit.junit.internal.ReflectionUtils.findAnnotation;
 import static com.tngtech.archunit.junit.internal.ReflectionUtils.getAllFields;
 import static com.tngtech.archunit.junit.internal.ReflectionUtils.getAllMethods;
 import static com.tngtech.archunit.junit.internal.ReflectionUtils.getValueOrThrowException;
 import static com.tngtech.archunit.junit.internal.ReflectionUtils.invokeMethod;
+import static com.tngtech.archunit.junit.internal.ReflectionUtils.tryFindAnnotation;
 import static com.tngtech.archunit.junit.internal.ReflectionUtils.withAnnotation;
 
 class ArchUnitTestDescriptor extends AbstractArchUnitTestDescriptor implements CreatesChildren {
@@ -71,8 +72,8 @@ static void resolve(TestDescriptor parent, ElementResolver resolver, ClassCache
     }
 
     private static void createTestDescriptor(TestDescriptor parent, ClassCache classCache, Class<?> clazz, ElementResolver childResolver) {
-        if (clazz.getAnnotation(AnalyzeClasses.class) == null) {
-            LOG.warn("Class {} is not annotated with @{} and thus cannot run as a top level test. "
+        if (!tryFindAnnotation(clazz, AnalyzeClasses.class).isPresent()) {
+            LOG.warn("Class {} is not (meta-)annotated with @{} and thus cannot run as a top level test. "
                             + "This warning can be ignored if {} is only used as part of a rules library included via {}.in({}.class).",
                     clazz.getName(), AnalyzeClasses.class.getSimpleName(),
                     clazz.getSimpleName(), ArchTests.class.getSimpleName(), clazz.getSimpleName());
@@ -291,15 +292,7 @@ private static class JUnit5ClassAnalysisRequest implements ClassAnalysisRequest
         private final AnalyzeClasses analyzeClasses;
 
         JUnit5ClassAnalysisRequest(Class<?> testClass) {
-            analyzeClasses = checkAnnotation(testClass);
-        }
-
-        private static AnalyzeClasses checkAnnotation(Class<?> testClass) {
-            AnalyzeClasses analyzeClasses = testClass.getAnnotation(AnalyzeClasses.class);
-            checkArgument(analyzeClasses != null,
-                    "Class %s must be annotated with @%s",
-                    testClass.getSimpleName(), AnalyzeClasses.class.getSimpleName());
-            return analyzeClasses;
+            analyzeClasses = findAnnotation(testClass, AnalyzeClasses.class);
         }
 
         @Override

diff --git a/...nit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java b/...nit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java
@@ -27,6 +27,7 @@
 import com.tngtech.archunit.junit.internal.testexamples.FullAnalyzeClassesSpec;
 import com.tngtech.archunit.junit.internal.testexamples.LibraryWithPrivateTests;
 import com.tngtech.archunit.junit.internal.testexamples.SimpleRuleLibrary;
+import com.tngtech.archunit.junit.internal.testexamples.TestClassWithMetaAnnotationForAnalyzeClasses;
 import com.tngtech.archunit.junit.internal.testexamples.TestClassWithMetaTag;
 import com.tngtech.archunit.junit.internal.testexamples.TestClassWithMetaTags;
 import com.tngtech.archunit.junit.internal.testexamples.TestClassWithTags;
@@ -169,6 +170,20 @@ void a_single_test_class() {
             assertThat(child.getParent().get()).isEqualTo(descriptor);
         }
 
+        @Test
+        void a_test_class_with_meta_annotated_analyze_classes() {
+            EngineDiscoveryTestRequest discoveryRequest = new EngineDiscoveryTestRequest().withClass(TestClassWithMetaAnnotationForAnalyzeClasses.class);
+
+            TestDescriptor descriptor = testEngine.discover(discoveryRequest, engineId);
+
+            TestDescriptor child = getOnlyElement(descriptor.getChildren());
+            assertThat(child).isInstanceOf(ArchUnitTestDescriptor.class);
+            assertThat(child.getUniqueId()).isEqualTo(engineId.append(CLASS_SEGMENT_TYPE, TestClassWithMetaAnnotationForAnalyzeClasses.class.getName()));
+            assertThat(child.getDisplayName()).isEqualTo(TestClassWithMetaAnnotationForAnalyzeClasses.class.getSimpleName());
+            assertThat(child.getType()).isEqualTo(CONTAINER);
+            assertThat(child.getParent()).get().isEqualTo(descriptor);
+        }
+
         @Test
         void source_of_a_single_test_class() {
             EngineDiscoveryTestRequest discoveryRequest = new EngineDiscoveryTestRequest().withClass(SimpleRuleField.class);
@@ -505,10 +520,10 @@ void mixed_class_methods_and_fields() {
             expectedLeafIds.add(simpleRuleFieldTestId(engineId));
             expectedLeafIds.add(simpleRuleMethodTestId(engineId));
             Stream.concat(
-                    SimpleRules.RULE_FIELD_NAMES.stream().map(fieldName ->
-                            simpleRulesId(engineId).append(FIELD_SEGMENT_TYPE, fieldName)),
-                    SimpleRules.RULE_METHOD_NAMES.stream().map(methodName ->
-                            simpleRulesId(engineId).append(METHOD_SEGMENT_TYPE, methodName)))
+                            SimpleRules.RULE_FIELD_NAMES.stream().map(fieldName ->
+                                    simpleRulesId(engineId).append(FIELD_SEGMENT_TYPE, fieldName)),
+                            SimpleRules.RULE_METHOD_NAMES.stream().map(methodName ->
+                                    simpleRulesId(engineId).append(METHOD_SEGMENT_TYPE, methodName)))
                     .forEach(expectedLeafIds::add);
 
             assertThat(getAllLeafUniqueIds(rootDescriptor))
@@ -1074,6 +1089,21 @@ void cache_is_cleared_afterwards() {
             verify(classCache, atLeastOnce()).getClassesToAnalyzeFor(any(Class.class), any(ClassAnalysisRequest.class));
             verifyNoMoreInteractions(classCache);
         }
+
+        @Test
+        void a_class_with_analyze_classes_as_meta_annotation() {
+            execute(createEngineId(), TestClassWithMetaAnnotationForAnalyzeClasses.class);
+
+            verify(classCache).getClassesToAnalyzeFor(eq(TestClassWithMetaAnnotationForAnalyzeClasses.class), classAnalysisRequestCaptor.capture());
+            ClassAnalysisRequest request = classAnalysisRequestCaptor.getValue();
+            AnalyzeClasses expected = TestClassWithMetaAnnotationForAnalyzeClasses.class.getAnnotation(TestClassWithMetaAnnotationForAnalyzeClasses.MetaAnalyzeClasses.class)
+                    .annotationType().getAnnotation(AnalyzeClasses.class);
+            assertThat(request.getPackageNames()).isEqualTo(expected.packages());
+            assertThat(request.getPackageRoots()).isEqualTo(expected.packagesOf());
+            assertThat(request.getLocationProviders()).isEqualTo(expected.locations());
+            assertThat(request.scanWholeClasspath()).as("scan whole classpath").isTrue();
+            assertThat(request.getImportOptions()).isEqualTo(expected.importOptions());
+        }
     }
 
     @Nested

diff --git a/...ch/archunit/junit/internal/testexamples/TestClassWithMetaAnnotationForAnalyzeClasses.java b/...ch/archunit/junit/internal/testexamples/TestClassWithMetaAnnotationForAnalyzeClasses.java
@@ -0,0 +1,21 @@
+package com.tngtech.archunit.junit.internal.testexamples;
+
+import java.lang.annotation.Retention;
+
+import com.tngtech.archunit.junit.AnalyzeClasses;
+import com.tngtech.archunit.junit.ArchTest;
+import com.tngtech.archunit.lang.ArchRule;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+@TestClassWithMetaAnnotationForAnalyzeClasses.MetaAnalyzeClasses
+public class TestClassWithMetaAnnotationForAnalyzeClasses {
+
+  @ArchTest
+  public static final ArchRule rule_in_class_with_meta_analyze_class_annotation = RuleThatFails.on(UnwantedClass.class);
+
+  @Retention(RUNTIME)
+  @AnalyzeClasses(wholeClasspath = true)
+  public @interface MetaAnalyzeClasses {
+  }
+}
diff --git a/...it/src/main/java/com/tngtech/archunit/junit/internal/ArchTestInitializationException.java b/...it/src/main/java/com/tngtech/archunit/junit/internal/ArchTestInitializationException.java
@@ -1,7 +1,7 @@
 package com.tngtech.archunit.junit.internal;
 
 class ArchTestInitializationException extends RuntimeException {
-    private ArchTestInitializationException(String message, Object... args) {
+    ArchTestInitializationException(String message, Object... args) {
         super(String.format(message, args));
     }
 

diff --git a/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ReflectionUtils.java b/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ReflectionUtils.java
@@ -23,6 +23,8 @@
 import java.lang.reflect.Modifier;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -38,20 +40,6 @@ class ReflectionUtils {
     private ReflectionUtils() {
     }
 
-    static Set<Class<?>> getAllSupertypes(Class<?> type) {
-        if (type == null) {
-            return Collections.emptySet();
-        }
-
-        ImmutableSet.Builder<Class<?>> result = ImmutableSet.<Class<?>>builder()
-                .add(type)
-                .addAll(getAllSupertypes(type.getSuperclass()));
-        for (Class<?> c : type.getInterfaces()) {
-            result.addAll(getAllSupertypes(c));
-        }
-        return result.build();
-    }
-
     static Collection<Field> getAllFields(Class<?> owner, Predicate<? super Field> predicate) {
         return getAll(owner, Class::getDeclaredFields).filter(predicate).collect(toList());
     }
@@ -68,18 +56,22 @@ private static <T> Stream<T> getAll(Class<?> type, Function<Class<?>, T[]> colle
         return result.build();
     }
 
-    static <T> T newInstanceOf(Class<T> type) {
-        return com.tngtech.archunit.base.ReflectionUtils.newInstanceOf(type);
-    }
+    private static Set<Class<?>> getAllSupertypes(Class<?> type) {
+        if (type == null) {
+            return Collections.emptySet();
+        }
 
-    @SuppressWarnings("unchecked") // callers must know, what they do here, we can't make this compile safe anyway
-    private static <T> T getValue(Field field, Object owner) {
-        try {
-            field.setAccessible(true);
-            return (T) field.get(owner);
-        } catch (IllegalAccessException e) {
-            throw new ReflectionException(e);
+        ImmutableSet.Builder<Class<?>> result = ImmutableSet.<Class<?>>builder()
+                .add(type)
+                .addAll(getAllSupertypes(type.getSuperclass()));
+        for (Class<?> c : type.getInterfaces()) {
+            result.addAll(getAllSupertypes(c));
         }
+        return result.build();
+    }
+
+    static <T> T newInstanceOf(Class<T> type) {
+        return com.tngtech.archunit.base.ReflectionUtils.newInstanceOf(type);
     }
 
     static <T> T getValueOrThrowException(Field field, Class<?> fieldOwner, Function<Throwable, ? extends RuntimeException> exceptionConverter) {
@@ -94,6 +86,16 @@ static <T> T getValueOrThrowException(Field field, Class<?> fieldOwner, Function
         }
     }
 
+    @SuppressWarnings("unchecked") // callers must know what they do here, we can't make this compile safe anyway
+    private static <T> T getValue(Field field, Object owner) {
+        try {
+            field.setAccessible(true);
+            return (T) field.get(owner);
+        } catch (IllegalAccessException e) {
+            throw new ReflectionException(e);
+        }
+    }
+
     static <T> T invokeMethod(Method method, Class<?> methodOwner, Object... args) {
         if (Modifier.isStatic(method.getModifiers())) {
             return invoke(null, method, args);
@@ -102,7 +104,7 @@ static <T> T invokeMethod(Method method, Class<?> methodOwner, Object... args) {
         }
     }
 
-    @SuppressWarnings("unchecked") // callers must know, what they do here, we can't make this compile safe anyway
+    @SuppressWarnings("unchecked") // callers must know what they do here, we can't make this compile safe anyway
     private static <T> T invoke(Object owner, Method method, Object... args) {
         method.setAccessible(true);
         try {
@@ -124,4 +126,41 @@ private static <T extends Throwable> void rethrowUnchecked(Throwable throwable)
     static Predicate<AnnotatedElement> withAnnotation(Class<? extends Annotation> annotationType) {
         return input -> input.isAnnotationPresent(annotationType);
     }
+
+    /**
+     * Same as {@link #tryFindAnnotation(Class, Class)}, but throws an exception if no annotation can be found.
+     */
+    static <T extends Annotation> T findAnnotation(final Class<?> clazz, Class<T> annotationType) {
+        return tryFindAnnotation(clazz, annotationType).orElseThrow(() ->
+                new ArchTestInitializationException("Class %s is not (meta-)annotated with @%s", clazz.getName(), annotationType.getSimpleName()));
+    }
+
+    /**
+     * Recursively searches for an annotation of type {@link T} on the given {@code clazz}.
+     * Returns the first matching annotation that is found.
+     * Any further matching annotation possibly present within the meta-annotation hierarchy will be ignored.
+     * If no matching annotation can be found {@link Optional#empty()} will be returned.
+     *
+     * @param clazz The {@link Class} from which to retrieve the annotation
+     * @return The first found annotation instance reachable in the meta-annotation hierarchy or {@link Optional#empty()} if none can be found
+     */
+    static <T extends Annotation> Optional<T> tryFindAnnotation(final Class<?> clazz, Class<T> annotationType) {
+        return tryFindAnnotation(clazz.getAnnotations(), annotationType, new HashSet<>());
+    }
+
+    private static <T extends Annotation> Optional<T> tryFindAnnotation(final Annotation[] annotations, Class<T> annotationType, Set<Annotation> visited) {
+        for (Annotation annotation : annotations) {
+            if (!visited.add(annotation)) {
+                continue;
+            }
+
+            Optional<T> result = annotationType.isInstance(annotation)
+                    ? Optional.of(annotationType.cast(annotation))
+                    : tryFindAnnotation(annotation.annotationType().getAnnotations(), annotationType, visited);
+            if (result.isPresent()) {
+                return result;
+            }
+        }
+        return Optional.empty();
+    }
 }