Make every rule able to fail, and prove it

pom.xml

@@ -50,6 +50,32 @@
             <version>1.4.2</version>
             <scope>compile</scope>
         </dependency>
+
+        <!-- Test only: needed to run this library's own rule tests -->
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <!--
+            Fixtures for the two rules in CommonArchRuleCollection need real Spring MVC annotations.
+            Test scope only, so consumers do not inherit them.
+        -->
+        <dependency>
+            <groupId>org.springframework</groupId>
+            <artifactId>spring-web</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.springframework</groupId>
+            <artifactId>spring-context</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>
@@ -98,6 +124,20 @@
                     <fork>true</fork>
                 </configuration>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <!--
+                        it.aboutbits.archunit.fixture contains deliberately non-conforming classes that
+                        the rule tests import as input. Some are named *Test and would otherwise be
+                        collected and reported as tests of this project.
+                    -->
+                    <excludes>
+                        <exclude>it/aboutbits/archunit/fixture/**</exclude>
+                    </excludes>
+                </configuration>
+            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-checkstyle-plugin</artifactId>

readme.md

@@ -16,34 +16,109 @@ Add this library to the classpath by adding the following maven dependency. Vers
 </dependency>
 ```
 
+## Upgrading to 1.3.0
+
+**This release will fail builds that passed on 1.2.0, on purpose.** Nine rules were silently
+reporting success because they matched nothing; fixing them turns real violations into build
+failures for the first time. Expect two kinds:
+
+- **Revived rules surface real violations.** Chiefly the two that were fully dead:
+  `test_classes_should_be_in_the_same_package_as_their_production_code` and
+  `nested_test_classes_have_matching_production_method_name`. Triage them with the opt-out
+  stereotype described under [Opting out](#opting-out) before annotating classes one at a time.
+  The stricter paths (`getCodeUnits()` reaching constructors and field initializers, exact
+  security-test matching, `SortMappings` reporting what it cannot read) surfaced nothing in a
+  large codebase, so noise from those is unlikely.
+- **`analyzed_packages_must_contain_classes` is new and fails on an empty import.** If the
+  packages given to `@AnalyzeClasses` are mistyped or have moved, that is now a failure instead
+  of 13 rules quietly passing.
+
+Nothing else needs a migration: no rule fails over code your project does not have.
+
 ## Usage
 
-To use this package, simply extend one of the provided ArchUnit classes.
-For example `ArchitectureTestBase`:
+Implement one of the provided rule collections in your own architecture test.
 
 ```java
 
 @AnalyzeClasses(
         packages = ArchitectureTest.PACKAGE
 )
 @NullMarked
-class ArchitectureTest extends ArchitectureTestBase {
+@ArchIgnoreNoProductionCounterpart
+class ArchitectureTest implements BaseArchRuleCollection {
     static final String PACKAGE = "the.base.package.of.your.project";
+}
+```
 
-    static {
-        // Configuration
-    }
+`BaseArchRuleCollection` holds the rules that apply to any Java project. `CommonArchRuleCollection`
+adds rules for Spring MVC controllers and for `SortMappings`, so implement it only in a project that
+has them.
+
+The blacklists are mutable, so a project can drop an entry it disagrees with:
+
+```java
+
+static {
+    BlacklistClassesArchRule.BLACKLISTED_CLASSES.remove("net.datafaker.Faker");
 }
 ```
 
-In the static block you can configure some blacklists provided by the base class.
+The same applies to `ArchRuleConfig.TEST_CLASS_SUFFIXES` when a project introduces a new test type.
+
+### Rules your project has no code for
+
+Every rule tolerates a selection that comes up empty, so a rule simply passes on a project it does
+not apply to. Whether a project has records, controllers, `@Store` classes or `@Nested` test classes
+is the project's business, not something this library requires.
+
+That each rule can actually fail is guaranteed by a red test per rule in this repository, rather than
+by making your build fail over code you do not have. An empty selection says nothing about whether a
+rule's logic works.
+
+One case is a real problem though, and `analyzed_packages_must_contain_classes` covers it: if the
+packages given to `@AnalyzeClasses` are mistyped or have moved, nothing is imported and every other
+rule would pass without looking at a single class. That fails, once, with a message naming the cause.
+
+### Opting out
+
+Two annotations exempt a class from a specific rule. Neither is meta-annotated with ArchUnit's
+`@ArchIgnore`: the ArchUnit JUnit engine resolves meta-annotations, so that would skip *every*
+`@ArchTest` on the annotated class and report success rather than exempting it from one rule.
+
+| annotation | put it on | exempts from |
+|---|---|---|
+| `@ArchIgnoreNoProductionCounterpart` | a test class | needing a production class of the same name in the same package, and having its `@Nested` classes matched against production methods |
+| `@ArchIgnoreGroupName` | a `@Nested` test class | needing a production method of the same name, for a class that only groups tests |
+
+Use `@ArchIgnoreNoProductionCounterpart` for a test named after the behaviour it describes rather than
+after a production class.
+
+Both are read as meta-annotations, so a project declares its intent once on its own stereotype instead
+of repeating the annotation on every class:
 
 ```java
-    static {
-    ArchitectureTestBase.BLACKLISTED_CLASSES.remove("net.datafaker.Faker");
+
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.RUNTIME)
+@ArchIgnoreNoProductionCounterpart
+public @interface BusinessTest {
 }
 ```
 
+Annotating a single class directly still works — ArchUnit counts a direct annotation as
+meta-annotated.
+
+The same applies to `@Disabled` and ArchUnit's `@ArchIgnore`, which these rules also honour: a
+stereotype that carries either of them exempts every class using it. That matches how JUnit and the
+ArchUnit engine themselves read those two annotations — a class whose tests do not run is not held to
+naming rules — but it does mean a stereotype can exempt more than it appears to, so keep an eye on
+what your own test annotations carry.
+
+Architecture tests need neither: any class in a package named `_architecture` is exempt from the
+production-counterpart rule, alongside the existing `_support` and `_config` exclusions. Use the
+annotation for the one-off that lives elsewhere.
+
 ## Local Development
 
 To use this library as a local development dependency, you can simply refer to the version `BUILD-SNAPSHOT`.

src/main/java/it/aboutbits/archunit/toolbox/BaseArchRuleCollection.java

@@ -1,5 +1,6 @@
 package it.aboutbits.archunit.toolbox;
 
+import it.aboutbits.archunit.toolbox.rule.base.AnalyzedPackagesMustContainClassesArchRule;
 import it.aboutbits.archunit.toolbox.rule.base.BlacklistAnnotationsArchRule;
 import it.aboutbits.archunit.toolbox.rule.base.BlacklistClassesArchRule;
 import it.aboutbits.archunit.toolbox.rule.base.BlacklistMethodsArchRule;
@@ -15,6 +16,7 @@
 
 @NullMarked
 public interface BaseArchRuleCollection extends
+        AnalyzedPackagesMustContainClassesArchRule,
         BlacklistAnnotationsArchRule,
         BlacklistClassesArchRule,
         BlacklistMethodsArchRule,

src/main/java/it/aboutbits/archunit/toolbox/CommonArchRuleCollection.java

@@ -1,11 +1,13 @@
 package it.aboutbits.archunit.toolbox;
 
+import it.aboutbits.archunit.toolbox.rule.base.AnalyzedPackagesMustContainClassesArchRule;
 import it.aboutbits.archunit.toolbox.rule.common.ControllerRequestMappingsMustBeSecurityTested;
 import it.aboutbits.archunit.toolbox.rule.common.SortMappingsExhaustiveArchRule;
 import org.jspecify.annotations.NullMarked;
 
 @NullMarked
 public interface CommonArchRuleCollection extends
+        AnalyzedPackagesMustContainClassesArchRule,
         ControllerRequestMappingsMustBeSecurityTested,
         SortMappingsExhaustiveArchRule {
 }

src/main/java/it/aboutbits/archunit/toolbox/config/ArchRuleConfig.java

@@ -10,12 +10,10 @@ public final class ArchRuleConfig {
     private ArchRuleConfig() {
     }
 
-    /**
-     * List of supported test class name suffixes.
-     * <p>
-     * When introducing a new test type (e.g. IntegrationTest), add its suffix here
-     * instead of directly modifying the regex pattern.
-     **/
+    /// List of supported test class name suffixes.
+    ///
+    /// When introducing a new test type (e.g. IntegrationTest), add its suffix here
+    /// instead of directly modifying the regex pattern.
     public static final Set<String> TEST_CLASS_SUFFIXES = new HashSet<>(
             Set.of(
                     "Test",

src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java

@@ -0,0 +1,26 @@
+package it.aboutbits.archunit.toolbox.rule.base;
+
+import com.tngtech.archunit.core.domain.JavaClasses;
+import com.tngtech.archunit.junit.ArchTest;
+import org.jspecify.annotations.NullMarked;
+
+/// Checks that the analyzed packages contain any classes at all.
+///
+/// Every other rule tolerates an empty selection, because whether a project has records, controllers
+/// or `@Nested` test classes is the project's business and not something this library gets to require.
+/// That leaves exactly one dangerous case: a mistyped or moved package in `@AnalyzeClasses` imports
+/// nothing, and every rule then passes without looking at a single class. This rule is what turns that
+/// into a failure, once, with a message that names the actual problem.
+@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
+@NullMarked
+public interface AnalyzedPackagesMustContainClassesArchRule {
+    @SuppressWarnings({"unused", "checkstyle:MethodName", "java:S100"})
+    @ArchTest
+    default void analyzed_packages_must_contain_classes(JavaClasses classes) {
+        if (classes.isEmpty()) {
+            throw new AssertionError("""
+                    No classes were imported, so none of the architecture rules checked anything.
+                    Verify the packages passed to @AnalyzeClasses.""");
+        }
+    }
+}

src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRule.java

@@ -12,6 +12,7 @@
 import java.util.Set;
 
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
+import static it.aboutbits.archunit.toolbox.util.CodeUnitUtil.describeKind;
 import static it.aboutbits.archunit.toolbox.util.LineNumberUtil.getLineNumber;
 
 @SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
@@ -63,6 +64,7 @@ public interface BlacklistAnnotationsArchRule {
     default void no_blacklisted_annotations_are_used(JavaClasses classes) {
         classes()
                 .should(new NotUseBlacklistedAnnotations())
+                .allowEmptyShould(true)
                 .check(classes);
     }
 
@@ -87,33 +89,36 @@ public void check(JavaClass javaClass, ConditionEvents events) {
                 }
             }
 
-            // Check annotations on methods and their parameters
-            for (var method : javaClass.getMethods()) {
-                // Check method annotations
-                for (var annotation : method.getAnnotations()) {
+            // getCodeUnits() covers methods, constructors and the static initializer. getMethods()
+            // would miss constructors, and with them the most common position of all: a blacklisted
+            // annotation on a constructor parameter.
+            for (var codeUnit : javaClass.getCodeUnits()) {
+                for (var annotation : codeUnit.getAnnotations()) {
                     if (BLACKLISTED_ANNOTATIONS.contains(annotation.getRawType().getFullName())) {
                         var message = String.format(
-                                "Method %s is annotated with blacklisted annotation @%s (%s.java:%d)",
-                                method.getFullName(),
+                                "%s %s is annotated with blacklisted annotation @%s (%s.java:%d)",
+                                describeKind(codeUnit),
+                                codeUnit.getFullName(),
                                 annotation.getRawType().getFullName(),
                                 javaClass.getSimpleName(),
-                                getLineNumber(method)
+                                getLineNumber(codeUnit)
                         );
-                        events.add(SimpleConditionEvent.violated(method, message));
+                        events.add(SimpleConditionEvent.violated(codeUnit, message));
                     }
                 }
-                // Check method parameter annotations
-                for (var parameter : method.getParameters()) {
+
+                for (var parameter : codeUnit.getParameters()) {
                     for (var annotation : parameter.getAnnotations()) {
                         if (BLACKLISTED_ANNOTATIONS.contains(annotation.getRawType().getFullName())) {
                             var message = String.format(
-                                    "Parameter %s of method %s is annotated with blacklisted annotation @%s (%s.java:%d)",
+                                    "Parameter %s of %s %s is annotated with blacklisted annotation @%s (%s.java:%d)",
                                     parameter.getIndex(),
-                                    method.getFullName(),
+                                    describeKind(codeUnit).toLowerCase(java.util.Locale.ROOT),
+                                    codeUnit.getFullName(),
                                     annotation.getRawType().getFullName(),
                                     javaClass.getSimpleName(),
-                                    getLineNumber(method)
-                            ); // Parameter doesn't have its own SLOC, use method's
+                                    getLineNumber(codeUnit)
+                            ); // Parameter doesn't have its own SLOC, use the code unit's
                             events.add(SimpleConditionEvent.violated(parameter, message));
                         }
                     }

src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRule.java

@@ -35,6 +35,7 @@ public boolean test(JavaClass javaClass) {
                             }
                         }
                 )
+                .allowEmptyShould(true)
                 .check(classes);
     }
 }