Make every rule able to fail, and prove it (#4)

* add a red/green fixture test for every shipped rule

Each of the 13 rules gets a fixture built to violate it and one built to satisfy
it, asserted on the violation count and message rather than on the rule having
run. The library had no such test, which is why rules that match nothing have
been reporting success.

This commit is deliberately red: nine tests fail. Eight fixtures that must
produce a violation do not.

  - test classes with no production counterpart (rule is fully dead)
  - a blacklisted annotation on a constructor parameter
  - a blacklisted method call from a constructor
  - a blacklisted method call from an instance field initializer
  - System.out from a constructor
  - a @nested test class whose production class is missing entirely
  - a controller method covered only by a longer-named sibling's @nested class
  - a non-static SortMappings field

The ninth is not a fixture: the blacklist names three AssertJ methods that do not
exist, so those entries can never match.

Fixtures live outside it.aboutbits.archunit.toolbox so the project's own
ArchitectureTest does not analyse them, and are excluded from surefire because
some are named *Test.

Third-party types the blacklists name are stubbed rather than depended on:
spring-boot-toolbox depends on archunit-toolbox, so SortMappings, @Store and
@ArchAllowDirectAccess cannot come from there. The stub contracts were verified
against the real 2.5.2 artifact.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* match test class names and code unit bodies in one shared place

Two root causes, each shared by several rules.

The suffix regex was hand-built in five places, in two shapes, against two
different inputs. The counterpart rule's condition rebuilt it without the
leading ".+" and matched it against getSimpleName() with String.matches, which
anchors both ends - so only a class named exactly "Test" got past the guard and
every real test class returned before the counterpart lookup ever ran. The rule
has therefore never reported anything.

TestClassNames now owns the pattern, the stripping and the selection predicate.
The redundant guard in the condition is gone rather than corrected: the selection
already guarantees the suffix, and re-deriving it in the condition is what let
the two drift apart. Selection also matches the simple name and requires a
non-empty production name to be left over, so it can no longer accept a name the
stripping turns into nothing ("CacheTest" matched with "Cache" as the ".+").

Three rules walked getMethods(), which excludes constructors - and with them
every instance field initializer, since that is compiled into the constructor.
So a blacklisted annotation on a constructor parameter, the canonical Lombok
position, was invisible. All three now walk getCodeUnits(), which also subsumes
the separate static-initializer branches two of them carried.

Also drops three blacklist entries naming AssertJ methods that do not exist
(assertThrows, assertThrowsExactly, assertDoesNotThrow are JUnit's). An entry
that can never match reads as coverage without providing any.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* report what a rule cannot verify instead of passing

Removes the remaining ways a rule stayed quiet.

allowEmptyShould(true) is gone everywhere. A rule that selects nothing reported
success, indistinguishable from a rule that is satisfied, which is what let a
dead rule survive. EmptySelectionTest pins this for all eight rules that narrow
their input. Consequence to be aware of: CommonArchRuleCollection now fails in a
project with no controllers or no @Store classes, so implement it only where
those exist.

The @nested name rule skipped a nested class silently whenever the production
class was missing and the nested class was not inside a group - so the case with
nothing to compare against was the one case never reported. It now reports, and
in exchange honours @ArchIgnoreNoProductionCounterpart: a test class that
declares it has no production counterpart has no production methods either.

The security-test rule accepted any @nested class *starting with* the method
name, so getAll() counted as covered by GetAllArchived. It now requires the name
to match exactly or to continue with "$", which keeps grouped @nested classes
working.

The SortMappings rule swallowed every failure to read a field into a log.warn -
and the build has no SLF4J provider, so the warning was discarded outright. A
non-static field, an unreadable field, an unresolvable enum type and a value
that is not a Map are all violations now. Non-static in particular is reported
as such, since it can never be validated.

Finally, @ArchIgnoreNoProductionCounterpart and @ArchIgnoreGroupName no longer
meta-annotate ArchUnit's @ArchIgnore. The ArchUnit JUnit engine resolves
meta-annotations, so annotating a test class skipped *every* @archtest on it and
still reported BUILD SUCCESS - verified: putting it on this project's own
ArchitectureTest turned 11 rules into 11 skips. Both annotations are read by
their own type, so the meta-annotation bought nothing. ArchitectureTest now
carries it in place of the name that was hardcoded into the counterpart rule,
which also keeps the annotation exercised.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* document the current API and the opt-out annotations

The usage section still described the 1.1.0 API (extends ArchitectureTestBase,
a BLACKLISTED_CLASSES static block), which no longer exists. Replaces it with the
rule collections, records why no rule allows an empty selection, and documents
both opt-out annotations - neither of which any consumer appears to use yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* allow rules a project has no code for, and guard the import instead

Failing on an empty selection was the wrong call. Seven of the eight rules that
narrow their input have a legitimate empty case - no records, no controllers, no
@Store classes, nobody using @nested - and one of them described this repository
before this branch: a project whose only test is the ArchitectureTest has no
@test methods at all. Whether that code exists is the project's business.

allowEmptyShould(true) was never what protected against a dead rule either. It
fires on what a consumer's code happens to contain and says nothing about whether
a rule's logic works: the counterpart rule that started this had a perfectly
non-empty selection and a broken condition. The guarantee is the red test per
rule in this project, so all 13 rules now tolerate an empty selection.

That leaves one case worth failing on. A mistyped or moved package in
@AnalyzeClasses imports nothing, and every rule then passes without looking at a
single class. AnalyzedPackagesMustContainClassesArchRule turns that into one
failure naming the cause, instead of five rules passing silently and eight
reporting that they checked nothing. It is in both collections, so it applies
wherever the toolbox is used.

EmptySelectionTest is inverted to match: it now pins that every rule accepts a
project with nothing for it to check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* use markdown javadoc

Converts every documentation comment to the /// form of JEP 467: 54 javadoc
comments plus two block comments that already sat in documentation positions.
Markdown idiom throughout - `code` for {@code}, [Type#member] for {@link}, blank
/// lines for <p>, and backticked annotation names for the &#64; entities.

The five comments inside method bodies stay /* */, since /// is a documentation
comment form and those document statements rather than declarations.

No behaviour change. Verified with javadoc -Xdoclint:all: no warnings, the
[#member] references resolve, and the generated HTML shows code spans, paragraphs
and links rather than literal markdown.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* make the opt-outs usable once, and close the blacklist hole

Addresses the review on #4, all three points verified against reverts.

The opt-outs are read with areNotMetaAnnotatedWith / isMetaAnnotatedWith instead
of the direct-only variants, so a project carries @ArchIgnoreNoProductionCounterpart
on one stereotype of its own rather than repeating it on 74 classes. ArchUnit counts
a direct annotation as meta-annotated, so annotating a single class still works.
@ArchIgnoreGroupName gets the same treatment in all three places it is read: it is
the sibling opt-out with the identical ergonomics problem, and leaving it direct-only
would be half a fix.

Architecture tests are exempted from the production-counterpart rule by package,
via .._architecture.. alongside .._support.. / .._config.., so dropping the
hardcoded "ArchitectureTest" name does not push boilerplate onto every consumer.
Deliberately not added to TestClassVisibilityArchRule: being package private is as
achievable for an architecture test as for any other test, so the two exclusion
lists encode different facts and are meant to differ.

org.junit.jupiter.api.Assertions.assertThrowsExactly is blacklisted again. It only
ever existed under the AssertJ namespace that the previous commit removed, so the
cleanup dropped the house rule along with the bogus entry. Now covered by a fixture
that actually calls it, not only by a list assertion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* use a multiline string for the empty-import message

Review feedback on #4: text block instead of concatenation. The message now reads
over two lines, which also matches how the other rules format multi-line output.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* address review: lookup cost, null mappings, and an upgrade note

Four review comments on #4.

The two counterpart lookups scanned every imported class per test class. JavaClasses
is map-backed by fully qualified name, so both are now contain()/get(). New fixture
nestedclassname.goodgroup pins that a production class nested inside another is still
found, since its name contains a '$' and nothing covered that before - without it a
keying difference would have passed silently, the badgroup fixture expecting a
violation either way.

SortMappings gets the null-value fixture: a static field that reads back as null
yields nothing to compare, so the rule reports rather than calling it exhaustive.
The undeterminable-enum and non-enum-key paths remain untested.

Readme gains an upgrade note naming what breaks when a consumer moves off 1.2.0 -
revived rules surfacing real violations, and the new empty-import failure - and the
opt-out section now says that @disabled and @ArchIgnore are matched through
meta-annotations too, so a stereotype carrying either exempts its classes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: SirCotare

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);
     }
 }