From 052069a4efcc62572095405fb1a868d6e7b539e4 Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Wed, 19 Aug 2026 12:31:04 +0200 Subject: [PATCH 1/9] 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 --- pom.xml | 40 +++++++++++ .../archunit/fixture/barren/PlainClass.java | 14 ++++ .../badclass/AnnotatedClass.java | 5 ++ .../AnnotatedConstructorParameter.java | 14 ++++ .../badfield/AnnotatedField.java | 10 +++ .../badmethod/AnnotatedMethod.java | 7 ++ .../badparam/AnnotatedParameter.java | 7 ++ .../blacklistannotations/good/CleanClass.java | 13 ++++ .../blacklistclasses/bad/UsesFaker.java | 9 +++ .../good/UsesNothingBlacklisted.java | 7 ++ .../badconstructor/CallsFromConstructor.java | 11 +++ .../CallsFromFieldInitializer.java | 16 +++++ .../badmethod/CallsFromMethod.java | 11 +++ .../badstatic/CallsFromStaticInitializer.java | 19 +++++ .../good/CallsAllowedAssertion.java | 9 +++ .../jspecify/bad/UnannotatedClass.java | 4 ++ .../fixture/jspecify/good/AnnotatedClass.java | 7 ++ .../nestedclassname/badgroup/Widget.java | 6 ++ .../nestedclassname/badgroup/WidgetTest.java | 13 ++++ .../nestedclassname/badmethod/Widget.java | 6 ++ .../nestedclassname/badmethod/WidgetTest.java | 10 +++ .../badnoproduction/WidgetTest.java | 10 +++ .../fixture/nestedclassname/good/Widget.java | 6 ++ .../nestedclassname/good/WidgetTest.java | 9 +++ .../goodoptout/ScenarioTest.java | 15 ++++ .../nestedclassname/goodoptout/Widget.java | 6 ++ .../goodoptout/WidgetTest.java | 10 +++ .../nestedclassvisibility/bad/WidgetTest.java | 9 +++ .../good/WidgetTest.java | 9 +++ .../EnclosingReadsNestedRecordField.java | 11 +++ .../goodaccessor/EnclosingUsesAccessor.java | 10 +++ .../EnclosingReadsOptedOutRecord.java | 13 ++++ .../badmissing/WidgetController.java | 12 ++++ .../badnonested/WidgetController.java | 12 ++++ .../WidgetControllerSecurityTest.java | 5 ++ .../badprefix/WidgetController.java | 17 +++++ .../WidgetControllerSecurityTest.java | 13 ++++ .../securitytested/good/WidgetController.java | 12 ++++ .../good/WidgetControllerSecurityTest.java | 9 +++ .../goodnestedgroup/WidgetController.java | 12 ++++ .../WidgetControllerSecurityTest.java | 18 +++++ .../fixture/sortmappings/bad/WidgetSort.java | 6 ++ .../fixture/sortmappings/bad/WidgetStore.java | 14 ++++ .../sortmappings/badnonstatic/WidgetSort.java | 6 ++ .../badnonstatic/WidgetStore.java | 14 ++++ .../fixture/sortmappings/good/WidgetSort.java | 6 ++ .../sortmappings/good/WidgetStore.java | 16 +++++ .../badconstructor/PrintsFromConstructor.java | 7 ++ .../systemout/badlambda/PrintsFromLambda.java | 9 +++ .../systemout/badmethod/PrintsFromMethod.java | 7 ++ .../PrintsFromStaticInitializer.java | 7 ++ .../fixture/systemout/good/UsesNoConsole.java | 7 ++ .../testclasspackage/bad/WidgetTest.java | 5 ++ .../fixture/testclasspackage/good/Widget.java | 4 ++ .../testclasspackage/good/WidgetTest.java | 4 ++ .../goodoptout/ScenarioTest.java | 8 +++ .../testclasspackage/goodoptout/Widget.java | 4 ++ .../goodoptout/WidgetTest.java | 5 ++ .../testclassvisibility/bad/WidgetTest.java | 4 ++ .../testclassvisibility/good/WidgetTest.java | 4 ++ .../testmethodvisibility/bad/WidgetTest.java | 9 +++ .../testmethodvisibility/good/WidgetTest.java | 9 +++ .../archunit/toolbox/RuleEvaluation.java | 63 ++++++++++++++++ .../BlacklistAnnotationsArchRuleTest.java | 64 +++++++++++++++++ .../base/BlacklistClassesArchRuleTest.java | 27 +++++++ .../base/BlacklistMethodsArchRuleTest.java | 72 +++++++++++++++++++ .../base/EnforceJspecifyArchRuleTest.java | 29 ++++++++ .../base/NoSystemOutOrErrArchRuleTest.java | 49 +++++++++++++ ...MustBeAccessedViaAccessorArchRuleTest.java | 39 ++++++++++ ...TestClassInCorrectPackageArchRuleTest.java | 35 +++++++++ .../base/TestClassVisibilityArchRuleTest.java | 29 ++++++++ .../TestMethodVisibilityArchRuleTest.java | 29 ++++++++ .../TestNestedClassMatchNameArchRuleTest.java | 58 +++++++++++++++ ...TestNestedClassVisibilityArchRuleTest.java | 29 ++++++++ ...questMappingsMustBeSecurityTestedTest.java | 63 ++++++++++++++++ .../SortMappingsExhaustiveArchRuleTest.java | 43 +++++++++++ .../toolbox/util/LineNumberUtilTest.java | 54 ++++++++++++++ .../archunit/ArchAllowDirectAccess.java | 16 +++++ .../toolbox/persistence/SortMappings.java | 18 +++++ .../springboot/toolbox/stereotype/Store.java | 16 +++++ src/test/java/net/datafaker/Faker.java | 8 +++ src/test/java/org/junit/Ignore.java | 15 ++++ 82 files changed, 1367 insertions(+) create mode 100644 src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badclass/AnnotatedClass.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badctorparam/AnnotatedConstructorParameter.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badfield/AnnotatedField.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badmethod/AnnotatedMethod.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badparam/AnnotatedParameter.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/good/CleanClass.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistclasses/bad/UsesFaker.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistclasses/good/UsesNothingBlacklisted.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badconstructor/CallsFromConstructor.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badfieldinit/CallsFromFieldInitializer.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badmethod/CallsFromMethod.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badstatic/CallsFromStaticInitializer.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/good/CallsAllowedAssertion.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/jspecify/bad/UnannotatedClass.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/jspecify/good/AnnotatedClass.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badnoproduction/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/good/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/good/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassvisibility/bad/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassvisibility/good/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/recordaccessor/badnested/EnclosingReadsNestedRecordField.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/recordaccessor/goodaccessor/EnclosingUsesAccessor.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/recordaccessor/goodoptout/EnclosingReadsOptedOutRecord.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/badmissing/WidgetController.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetController.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetControllerSecurityTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetController.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/good/WidgetController.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/good/WidgetControllerSecurityTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetController.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetSort.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetStore.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetSort.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetStore.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/sortmappings/good/WidgetSort.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/sortmappings/good/WidgetStore.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/systemout/badconstructor/PrintsFromConstructor.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/systemout/badlambda/PrintsFromLambda.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/systemout/badmethod/PrintsFromMethod.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/systemout/badstatic/PrintsFromStaticInitializer.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/systemout/good/UsesNoConsole.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/bad/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/good/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/good/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclassvisibility/bad/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclassvisibility/good/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testmethodvisibility/bad/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testmethodvisibility/good/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/EnforceJspecifyArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/util/LineNumberUtilTest.java create mode 100644 src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java create mode 100644 src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java create mode 100644 src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java create mode 100644 src/test/java/net/datafaker/Faker.java create mode 100644 src/test/java/org/junit/Ignore.java diff --git a/pom.xml b/pom.xml index 3fdae42..b17c305 100644 --- a/pom.xml +++ b/pom.xml @@ -50,6 +50,32 @@ 1.4.2 compile + + + + org.junit.jupiter + junit-jupiter-engine + test + + + org.assertj + assertj-core + test + + + + org.springframework + spring-web + test + + + org.springframework + spring-context + test + @@ -98,6 +124,20 @@ true + + org.apache.maven.plugins + maven-surefire-plugin + + + + it/aboutbits/archunit/fixture/** + + + org.apache.maven.plugins maven-checkstyle-plugin diff --git a/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java b/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java new file mode 100644 index 0000000..93a9c6d --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java @@ -0,0 +1,14 @@ +package it.aboutbits.archunit.fixture.barren; + +import org.jspecify.annotations.NullMarked; + +/** + * A codebase with no test classes, no records, no controllers and no stores. Used to pin that a rule + * whose selection comes up empty fails instead of reporting success. + */ +@NullMarked +public class PlainClass { + public String value() { + return "value"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badclass/AnnotatedClass.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badclass/AnnotatedClass.java new file mode 100644 index 0000000..98d48c2 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badclass/AnnotatedClass.java @@ -0,0 +1,5 @@ +package it.aboutbits.archunit.fixture.blacklistannotations.badclass; + +@org.junit.Ignore +public class AnnotatedClass { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badctorparam/AnnotatedConstructorParameter.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badctorparam/AnnotatedConstructorParameter.java new file mode 100644 index 0000000..e7dea28 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badctorparam/AnnotatedConstructorParameter.java @@ -0,0 +1,14 @@ +package it.aboutbits.archunit.fixture.blacklistannotations.badctorparam; + +/** The canonical Lombok position, and the one a rule iterating only getMethods() cannot see. */ +public class AnnotatedConstructorParameter { + private final String value; + + public AnnotatedConstructorParameter(@lombok.NonNull String value) { + this.value = value; + } + + public String value() { + return value; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badfield/AnnotatedField.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badfield/AnnotatedField.java new file mode 100644 index 0000000..25523eb --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badfield/AnnotatedField.java @@ -0,0 +1,10 @@ +package it.aboutbits.archunit.fixture.blacklistannotations.badfield; + +public class AnnotatedField { + @lombok.NonNull + private String value = "x"; + + public String value() { + return value; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badmethod/AnnotatedMethod.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badmethod/AnnotatedMethod.java new file mode 100644 index 0000000..37898c2 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badmethod/AnnotatedMethod.java @@ -0,0 +1,7 @@ +package it.aboutbits.archunit.fixture.blacklistannotations.badmethod; + +public class AnnotatedMethod { + @org.junit.Ignore + public void doWork() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badparam/AnnotatedParameter.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badparam/AnnotatedParameter.java new file mode 100644 index 0000000..0c3fa24 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badparam/AnnotatedParameter.java @@ -0,0 +1,7 @@ +package it.aboutbits.archunit.fixture.blacklistannotations.badparam; + +public class AnnotatedParameter { + public void doWork(@lombok.NonNull String value) { + System.identityHashCode(value); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/good/CleanClass.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/good/CleanClass.java new file mode 100644 index 0000000..7f1136e --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/good/CleanClass.java @@ -0,0 +1,13 @@ +package it.aboutbits.archunit.fixture.blacklistannotations.good; + +public class CleanClass { + private String value = "x"; + + public void doWork(String input) { + this.value = input; + } + + public String value() { + return value; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistclasses/bad/UsesFaker.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistclasses/bad/UsesFaker.java new file mode 100644 index 0000000..67f78ce --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistclasses/bad/UsesFaker.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.blacklistclasses.bad; + +import net.datafaker.Faker; + +public class UsesFaker { + public String randomName() { + return new Faker().name(); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistclasses/good/UsesNothingBlacklisted.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistclasses/good/UsesNothingBlacklisted.java new file mode 100644 index 0000000..2628a05 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistclasses/good/UsesNothingBlacklisted.java @@ -0,0 +1,7 @@ +package it.aboutbits.archunit.fixture.blacklistclasses.good; + +public class UsesNothingBlacklisted { + public String randomName() { + return "fixed"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badconstructor/CallsFromConstructor.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badconstructor/CallsFromConstructor.java new file mode 100644 index 0000000..86b3631 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badconstructor/CallsFromConstructor.java @@ -0,0 +1,11 @@ +package it.aboutbits.archunit.fixture.blacklistmethods.badconstructor; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class CallsFromConstructor { + public CallsFromConstructor() { + assertThatThrownBy(() -> { + throw new IllegalStateException(); + }); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badfieldinit/CallsFromFieldInitializer.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badfieldinit/CallsFromFieldInitializer.java new file mode 100644 index 0000000..53cfa2b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badfieldinit/CallsFromFieldInitializer.java @@ -0,0 +1,16 @@ +package it.aboutbits.archunit.fixture.blacklistmethods.badfieldinit; + +import org.assertj.core.api.AbstractThrowableAssert; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** An instance field initializer is compiled into the constructor. */ +public class CallsFromFieldInitializer { + private final AbstractThrowableAssert assertion = assertThatThrownBy(() -> { + throw new IllegalStateException(); + }); + + public Object assertion() { + return assertion; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badmethod/CallsFromMethod.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badmethod/CallsFromMethod.java new file mode 100644 index 0000000..1a0ad43 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badmethod/CallsFromMethod.java @@ -0,0 +1,11 @@ +package it.aboutbits.archunit.fixture.blacklistmethods.badmethod; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class CallsFromMethod { + public void check() { + assertThatThrownBy(() -> { + throw new IllegalStateException(); + }); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badstatic/CallsFromStaticInitializer.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badstatic/CallsFromStaticInitializer.java new file mode 100644 index 0000000..9b92eea --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badstatic/CallsFromStaticInitializer.java @@ -0,0 +1,19 @@ +package it.aboutbits.archunit.fixture.blacklistmethods.badstatic; + +import org.assertj.core.api.AbstractThrowableAssert; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class CallsFromStaticInitializer { + static final AbstractThrowableAssert ASSERTION; + + static { + ASSERTION = assertThatThrownBy(() -> { + throw new IllegalStateException(); + }); + } + + public Object assertion() { + return ASSERTION; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/good/CallsAllowedAssertion.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/good/CallsAllowedAssertion.java new file mode 100644 index 0000000..c81173d --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/good/CallsAllowedAssertion.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.blacklistmethods.good; + +import static org.assertj.core.api.Assertions.assertThat; + +public class CallsAllowedAssertion { + public void check() { + assertThat("a").isEqualTo("a"); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/jspecify/bad/UnannotatedClass.java b/src/test/java/it/aboutbits/archunit/fixture/jspecify/bad/UnannotatedClass.java new file mode 100644 index 0000000..778cc40 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/jspecify/bad/UnannotatedClass.java @@ -0,0 +1,4 @@ +package it.aboutbits.archunit.fixture.jspecify.bad; + +public class UnannotatedClass { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/jspecify/good/AnnotatedClass.java b/src/test/java/it/aboutbits/archunit/fixture/jspecify/good/AnnotatedClass.java new file mode 100644 index 0000000..a5d1a88 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/jspecify/good/AnnotatedClass.java @@ -0,0 +1,7 @@ +package it.aboutbits.archunit.fixture.jspecify.good; + +import org.jspecify.annotations.NullMarked; + +@NullMarked +public class AnnotatedClass { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/Widget.java new file mode 100644 index 0000000..3e88f3c --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/Widget.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.nestedclassname.badgroup; + +public class Widget { + public void deleteAll() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/WidgetTest.java new file mode 100644 index 0000000..cad94a1 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/WidgetTest.java @@ -0,0 +1,13 @@ +package it.aboutbits.archunit.fixture.nestedclassname.badgroup; + +import org.junit.jupiter.api.Nested; + +/** The group implies a production class Widget$DeleteAction, which does not exist. */ +class WidgetTest { + @Nested + class DeleteAction { + @Nested + class DeleteAll { + } + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/Widget.java new file mode 100644 index 0000000..372f0ba --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/Widget.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.nestedclassname.badmethod; + +public class Widget { + public void doWork() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/WidgetTest.java new file mode 100644 index 0000000..8009d03 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/WidgetTest.java @@ -0,0 +1,10 @@ +package it.aboutbits.archunit.fixture.nestedclassname.badmethod; + +import org.junit.jupiter.api.Nested; + +/** Widget has no doSomethingElse() method. */ +class WidgetTest { + @Nested + class DoSomethingElse { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badnoproduction/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badnoproduction/WidgetTest.java new file mode 100644 index 0000000..27483ab --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badnoproduction/WidgetTest.java @@ -0,0 +1,10 @@ +package it.aboutbits.archunit.fixture.nestedclassname.badnoproduction; + +import org.junit.jupiter.api.Nested; + +/** No Widget class at all, so there is no method name to match DoWork against. */ +class WidgetTest { + @Nested + class DoWork { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/good/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/good/Widget.java new file mode 100644 index 0000000..dfa3418 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/good/Widget.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.nestedclassname.good; + +public class Widget { + public void doWork() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/good/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/good/WidgetTest.java new file mode 100644 index 0000000..0b917e5 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/good/WidgetTest.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.nestedclassname.good; + +import org.junit.jupiter.api.Nested; + +class WidgetTest { + @Nested + class DoWork { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java new file mode 100644 index 0000000..0401854 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java @@ -0,0 +1,15 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodoptout; + +import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; +import org.junit.jupiter.api.Nested; + +/** + * Declares that it has no production counterpart, so its @Nested classes have no production methods + * to be matched against either. + */ +@ArchIgnoreNoProductionCounterpart +class ScenarioTest { + @Nested + class SomeBehaviour { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/Widget.java new file mode 100644 index 0000000..99a5a2e --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/Widget.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodoptout; + +public class Widget { + public void doWork() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/WidgetTest.java new file mode 100644 index 0000000..f47da21 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/WidgetTest.java @@ -0,0 +1,10 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodoptout; + +import org.junit.jupiter.api.Nested; + +/** A conforming test class, so the rule has something to select. */ +class WidgetTest { + @Nested + class DoWork { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassvisibility/bad/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassvisibility/bad/WidgetTest.java new file mode 100644 index 0000000..026cf06 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassvisibility/bad/WidgetTest.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.nestedclassvisibility.bad; + +import org.junit.jupiter.api.Nested; + +class WidgetTest { + @Nested + public class DoWork { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassvisibility/good/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassvisibility/good/WidgetTest.java new file mode 100644 index 0000000..33a7fae --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassvisibility/good/WidgetTest.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.nestedclassvisibility.good; + +import org.junit.jupiter.api.Nested; + +class WidgetTest { + @Nested + class DoWork { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/badnested/EnclosingReadsNestedRecordField.java b/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/badnested/EnclosingReadsNestedRecordField.java new file mode 100644 index 0000000..64ae7cc --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/badnested/EnclosingReadsNestedRecordField.java @@ -0,0 +1,11 @@ +package it.aboutbits.archunit.fixture.recordaccessor.badnested; + +public class EnclosingReadsNestedRecordField { + record Money(long amount) { + } + + /** Nestmates share access to private members, so this compiles to a direct field read. */ + public long readDirectly(Money money) { + return money.amount; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/goodaccessor/EnclosingUsesAccessor.java b/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/goodaccessor/EnclosingUsesAccessor.java new file mode 100644 index 0000000..1cd95ed --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/goodaccessor/EnclosingUsesAccessor.java @@ -0,0 +1,10 @@ +package it.aboutbits.archunit.fixture.recordaccessor.goodaccessor; + +public class EnclosingUsesAccessor { + record Money(long amount) { + } + + public long readViaAccessor(Money money) { + return money.amount(); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/goodoptout/EnclosingReadsOptedOutRecord.java b/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/goodoptout/EnclosingReadsOptedOutRecord.java new file mode 100644 index 0000000..9b4cfc8 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/goodoptout/EnclosingReadsOptedOutRecord.java @@ -0,0 +1,13 @@ +package it.aboutbits.archunit.fixture.recordaccessor.goodoptout; + +import it.aboutbits.springboot.toolbox.archunit.ArchAllowDirectAccess; + +public class EnclosingReadsOptedOutRecord { + @ArchAllowDirectAccess(reason = "fixture") + record Money(long amount) { + } + + public long readDirectly(Money money) { + return money.amount; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmissing/WidgetController.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmissing/WidgetController.java new file mode 100644 index 0000000..76e288b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmissing/WidgetController.java @@ -0,0 +1,12 @@ +package it.aboutbits.archunit.fixture.securitytested.badmissing; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class WidgetController { + @GetMapping("/widgets") + public String getAll() { + return "[]"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetController.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetController.java new file mode 100644 index 0000000..500b47e --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetController.java @@ -0,0 +1,12 @@ +package it.aboutbits.archunit.fixture.securitytested.badnonested; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class WidgetController { + @GetMapping("/widgets") + public String getAll() { + return "[]"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetControllerSecurityTest.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetControllerSecurityTest.java new file mode 100644 index 0000000..7000f8c --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetControllerSecurityTest.java @@ -0,0 +1,5 @@ +package it.aboutbits.archunit.fixture.securitytested.badnonested; + +/** The security test class exists but covers nothing. */ +class WidgetControllerSecurityTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetController.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetController.java new file mode 100644 index 0000000..0eb18d4 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetController.java @@ -0,0 +1,17 @@ +package it.aboutbits.archunit.fixture.securitytested.badprefix; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class WidgetController { + @GetMapping("/widgets") + public String getAll() { + return "[]"; + } + + @GetMapping("/widgets/archived") + public String getAllArchived() { + return "[]"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java new file mode 100644 index 0000000..729ebeb --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java @@ -0,0 +1,13 @@ +package it.aboutbits.archunit.fixture.securitytested.badprefix; + +import org.junit.jupiter.api.Nested; + +/** + * Only getAllArchived() is security tested. A prefix match would let this @Nested class stand in + * for getAll() as well. + */ +class WidgetControllerSecurityTest { + @Nested + class GetAllArchived { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/good/WidgetController.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/good/WidgetController.java new file mode 100644 index 0000000..17b76ae --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/good/WidgetController.java @@ -0,0 +1,12 @@ +package it.aboutbits.archunit.fixture.securitytested.good; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class WidgetController { + @GetMapping("/widgets") + public String getAll() { + return "[]"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/good/WidgetControllerSecurityTest.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/good/WidgetControllerSecurityTest.java new file mode 100644 index 0000000..598d1e6 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/good/WidgetControllerSecurityTest.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.securitytested.good; + +import org.junit.jupiter.api.Nested; + +class WidgetControllerSecurityTest { + @Nested + class GetAll { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetController.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetController.java new file mode 100644 index 0000000..7bf56a2 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetController.java @@ -0,0 +1,12 @@ +package it.aboutbits.archunit.fixture.securitytested.goodnestedgroup; + +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; + +@Controller +public class WidgetController { + @GetMapping("/widgets") + public String getAll() { + return "[]"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java new file mode 100644 index 0000000..283b479 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java @@ -0,0 +1,18 @@ +package it.aboutbits.archunit.fixture.securitytested.goodnestedgroup; + +import it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName; +import org.junit.jupiter.api.Nested; + +/** + * getAll() is covered by a @Nested class grouped inside the method-named class. Also exercises + * @Controller rather than @RestController. + */ +class WidgetControllerSecurityTest { + @Nested + @ArchIgnoreGroupName + class GetAll { + @Nested + class WhenAdmin { + } + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetSort.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetSort.java new file mode 100644 index 0000000..8f8deac --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetSort.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.sortmappings.bad; + +public enum WidgetSort { + NAME, + CREATED_AT +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetStore.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetStore.java new file mode 100644 index 0000000..0d92f80 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetStore.java @@ -0,0 +1,14 @@ +package it.aboutbits.archunit.fixture.sortmappings.bad; + +import it.aboutbits.springboot.toolbox.persistence.SortMappings; +import it.aboutbits.springboot.toolbox.stereotype.Store; + +/** CREATED_AT has no mapping. */ +@Store("widget") +public class WidgetStore { + static final SortMappings SORT_MAPPINGS = SortMappings.of(WidgetSort.NAME); + + public Object mappings() { + return SORT_MAPPINGS; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetSort.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetSort.java new file mode 100644 index 0000000..b4960b6 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetSort.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.sortmappings.badnonstatic; + +public enum WidgetSort { + NAME, + CREATED_AT +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetStore.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetStore.java new file mode 100644 index 0000000..7811f27 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetStore.java @@ -0,0 +1,14 @@ +package it.aboutbits.archunit.fixture.sortmappings.badnonstatic; + +import it.aboutbits.springboot.toolbox.persistence.SortMappings; +import it.aboutbits.springboot.toolbox.stereotype.Store; + +/** A non-static field cannot be read reflectively, so its mappings cannot be validated. */ +@Store("widget") +public class WidgetStore { + private final SortMappings sortMappings = SortMappings.of(WidgetSort.NAME); + + public Object mappings() { + return sortMappings; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/good/WidgetSort.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/good/WidgetSort.java new file mode 100644 index 0000000..a15b5ad --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/good/WidgetSort.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.sortmappings.good; + +public enum WidgetSort { + NAME, + CREATED_AT +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/good/WidgetStore.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/good/WidgetStore.java new file mode 100644 index 0000000..af29033 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/good/WidgetStore.java @@ -0,0 +1,16 @@ +package it.aboutbits.archunit.fixture.sortmappings.good; + +import it.aboutbits.springboot.toolbox.persistence.SortMappings; +import it.aboutbits.springboot.toolbox.stereotype.Store; + +@Store("widget") +public class WidgetStore { + static final SortMappings SORT_MAPPINGS = SortMappings.of( + WidgetSort.NAME, + WidgetSort.CREATED_AT + ); + + public Object mappings() { + return SORT_MAPPINGS; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/systemout/badconstructor/PrintsFromConstructor.java b/src/test/java/it/aboutbits/archunit/fixture/systemout/badconstructor/PrintsFromConstructor.java new file mode 100644 index 0000000..5ce2379 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/systemout/badconstructor/PrintsFromConstructor.java @@ -0,0 +1,7 @@ +package it.aboutbits.archunit.fixture.systemout.badconstructor; + +public class PrintsFromConstructor { + public PrintsFromConstructor() { + System.err.println("noise"); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/systemout/badlambda/PrintsFromLambda.java b/src/test/java/it/aboutbits/archunit/fixture/systemout/badlambda/PrintsFromLambda.java new file mode 100644 index 0000000..5154969 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/systemout/badlambda/PrintsFromLambda.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.systemout.badlambda; + +import java.util.List; + +public class PrintsFromLambda { + public void shout() { + List.of("a").forEach(value -> System.out.println(value)); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/systemout/badmethod/PrintsFromMethod.java b/src/test/java/it/aboutbits/archunit/fixture/systemout/badmethod/PrintsFromMethod.java new file mode 100644 index 0000000..6f787bc --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/systemout/badmethod/PrintsFromMethod.java @@ -0,0 +1,7 @@ +package it.aboutbits.archunit.fixture.systemout.badmethod; + +public class PrintsFromMethod { + public void shout() { + System.out.println("noise"); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/systemout/badstatic/PrintsFromStaticInitializer.java b/src/test/java/it/aboutbits/archunit/fixture/systemout/badstatic/PrintsFromStaticInitializer.java new file mode 100644 index 0000000..f7e846e --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/systemout/badstatic/PrintsFromStaticInitializer.java @@ -0,0 +1,7 @@ +package it.aboutbits.archunit.fixture.systemout.badstatic; + +public class PrintsFromStaticInitializer { + static { + System.out.println("noise"); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/systemout/good/UsesNoConsole.java b/src/test/java/it/aboutbits/archunit/fixture/systemout/good/UsesNoConsole.java new file mode 100644 index 0000000..18c0519 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/systemout/good/UsesNoConsole.java @@ -0,0 +1,7 @@ +package it.aboutbits.archunit.fixture.systemout.good; + +public class UsesNoConsole { + public String quiet() { + return "quiet"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/bad/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/bad/WidgetTest.java new file mode 100644 index 0000000..1146717 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/bad/WidgetTest.java @@ -0,0 +1,5 @@ +package it.aboutbits.archunit.fixture.testclasspackage.bad; + +/** No Widget class in this package. */ +class WidgetTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/good/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/good/Widget.java new file mode 100644 index 0000000..ad98dde --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/good/Widget.java @@ -0,0 +1,4 @@ +package it.aboutbits.archunit.fixture.testclasspackage.good; + +public class Widget { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/good/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/good/WidgetTest.java new file mode 100644 index 0000000..c44df5f --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/good/WidgetTest.java @@ -0,0 +1,4 @@ +package it.aboutbits.archunit.fixture.testclasspackage.good; + +class WidgetTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java new file mode 100644 index 0000000..696227a --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java @@ -0,0 +1,8 @@ +package it.aboutbits.archunit.fixture.testclasspackage.goodoptout; + +import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; + +/** Named after the behaviour it describes, with no production class of its own. */ +@ArchIgnoreNoProductionCounterpart +class ScenarioTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/Widget.java new file mode 100644 index 0000000..81ade2b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/Widget.java @@ -0,0 +1,4 @@ +package it.aboutbits.archunit.fixture.testclasspackage.goodoptout; + +public class Widget { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/WidgetTest.java new file mode 100644 index 0000000..45a3184 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/WidgetTest.java @@ -0,0 +1,5 @@ +package it.aboutbits.archunit.fixture.testclasspackage.goodoptout; + +/** A conforming test class, so the rule has something to select. */ +class WidgetTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclassvisibility/bad/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclassvisibility/bad/WidgetTest.java new file mode 100644 index 0000000..a9ba1e6 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclassvisibility/bad/WidgetTest.java @@ -0,0 +1,4 @@ +package it.aboutbits.archunit.fixture.testclassvisibility.bad; + +public class WidgetTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclassvisibility/good/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclassvisibility/good/WidgetTest.java new file mode 100644 index 0000000..4c0f4fa --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclassvisibility/good/WidgetTest.java @@ -0,0 +1,4 @@ +package it.aboutbits.archunit.fixture.testclassvisibility.good; + +class WidgetTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testmethodvisibility/bad/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testmethodvisibility/bad/WidgetTest.java new file mode 100644 index 0000000..a1b063a --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testmethodvisibility/bad/WidgetTest.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.testmethodvisibility.bad; + +import org.junit.jupiter.api.Test; + +class WidgetTest { + @Test + public void it_works() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testmethodvisibility/good/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testmethodvisibility/good/WidgetTest.java new file mode 100644 index 0000000..5610f40 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testmethodvisibility/good/WidgetTest.java @@ -0,0 +1,9 @@ +package it.aboutbits.archunit.fixture.testmethodvisibility.good; + +import org.junit.jupiter.api.Test; + +class WidgetTest { + @Test + void it_works() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java b/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java new file mode 100644 index 0000000..99f2239 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java @@ -0,0 +1,63 @@ +package it.aboutbits.archunit.toolbox; + +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import org.jspecify.annotations.NullMarked; + +import java.util.regex.Pattern; + +/** + * Imports a rule fixture and inspects what a rule reports about it. + *

+ * Rules are invoked through their own {@code default} method, so a test exercises exactly what a + * consumer gets, including the message text. + *

+ */ +@NullMarked +public final class RuleEvaluation { + private static final String FIXTURE_ROOT = "it.aboutbits.archunit.fixture."; + private static final Pattern VIOLATION_COUNT = Pattern.compile("was violated \\((\\d+) times?\\)"); + + private RuleEvaluation() { + } + + /** + * Imports one fixture package, failing loudly if it is empty: a mistyped package would otherwise + * make every assertion about it pass for the wrong reason. + */ + public static JavaClasses fixture(String subPackage) { + var packageName = FIXTURE_ROOT + subPackage; + var classes = new ClassFileImporter().importPackages(packageName); + + if (classes.size() == 0) { + throw new IllegalStateException("Fixture package imported no classes: " + packageName); + } + + return classes; + } + + /** + * Runs a rule that is expected to report at least one violation and returns the failure. + * Failing here means the rule accepted a fixture that was built to violate it. + */ + public static AssertionError violationOf(Runnable ruleCheck) { + try { + ruleCheck.run(); + } catch (AssertionError failure) { + return failure; + } + + throw new AssertionError("Expected the rule to report a violation, but it reported success."); + } + + /** The number of violations ArchUnit reported, read back from its failure message. */ + public static int violationCount(AssertionError failure) { + var matcher = VIOLATION_COUNT.matcher(String.valueOf(failure.getMessage())); + + if (!matcher.find()) { + throw new IllegalStateException("Not an ArchUnit violation report: " + failure.getMessage()); + } + + return Integer.parseInt(matcher.group(1)); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java new file mode 100644 index 0000000..3106873 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java @@ -0,0 +1,64 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class BlacklistAnnotationsArchRuleTest implements BlacklistAnnotationsArchRule { + @Test + void a_blacklisted_annotation_on_a_class_is_reported() { + var failure = violationOf(() -> no_blacklisted_annotations_are_used(fixture("blacklistannotations.badclass"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("Class").hasMessageContaining("org.junit.Ignore"); + } + + @Test + void a_blacklisted_annotation_on_a_method_is_reported() { + var failure = violationOf(() -> no_blacklisted_annotations_are_used(fixture("blacklistannotations.badmethod"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("Method").hasMessageContaining("org.junit.Ignore"); + } + + @Test + void a_blacklisted_annotation_on_a_method_parameter_is_reported() { + var failure = violationOf(() -> no_blacklisted_annotations_are_used(fixture("blacklistannotations.badparam"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("Parameter 0 of method").hasMessageContaining("lombok.NonNull"); + } + + @Test + void a_blacklisted_annotation_on_a_field_is_reported() { + var failure = violationOf(() -> no_blacklisted_annotations_are_used(fixture("blacklistannotations.badfield"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("Field value").hasMessageContaining("lombok.NonNull"); + } + + /** + * The position that matters most in practice, and the one a rule looking only at getMethods() + * cannot see. + */ + @Test + void a_blacklisted_annotation_on_a_constructor_parameter_is_reported() { + var failure = violationOf( + () -> no_blacklisted_annotations_are_used(fixture("blacklistannotations.badctorparam"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("Parameter 0 of constructor") + .hasMessageContaining("lombok.NonNull"); + } + + @Test + void a_class_using_no_blacklisted_annotation_is_accepted() { + no_blacklisted_annotations_are_used(fixture("blacklistannotations.good")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRuleTest.java new file mode 100644 index 0000000..ea273fb --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRuleTest.java @@ -0,0 +1,27 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class BlacklistClassesArchRuleTest implements BlacklistClassesArchRule { + @Test + void depending_on_a_blacklisted_class_is_reported() { + var failure = violationOf(() -> no_blacklisted_classes_are_used(fixture("blacklistclasses.bad"))); + + assertThat(violationCount(failure)).isPositive(); + assertThat(failure) + .hasMessageContaining("UsesFaker") + .hasMessageContaining("net.datafaker.Faker"); + } + + @Test + void depending_on_no_blacklisted_class_is_accepted() { + no_blacklisted_classes_are_used(fixture("blacklistclasses.good")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java new file mode 100644 index 0000000..e1d7f51 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java @@ -0,0 +1,72 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class BlacklistMethodsArchRuleTest implements BlacklistMethodsArchRule { + @Test + void a_blacklisted_call_from_a_method_is_reported() { + var failure = violationOf(() -> no_blacklisted_methods_are_used(fixture("blacklistmethods.badmethod"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("Method") + .hasMessageContaining("assertThatThrownBy"); + } + + @Test + void a_blacklisted_call_from_a_constructor_is_reported() { + var failure = violationOf(() -> no_blacklisted_methods_are_used(fixture("blacklistmethods.badconstructor"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("Constructor") + .hasMessageContaining("assertThatThrownBy"); + } + + /** An instance field initializer is compiled into the constructor, so it needs the same reach. */ + @Test + void a_blacklisted_call_from_an_instance_field_initializer_is_reported() { + var failure = violationOf(() -> no_blacklisted_methods_are_used(fixture("blacklistmethods.badfieldinit"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("Constructor") + .hasMessageContaining("assertThatThrownBy"); + } + + @Test + void a_blacklisted_call_from_a_static_initializer_is_reported() { + var failure = violationOf(() -> no_blacklisted_methods_are_used(fixture("blacklistmethods.badstatic"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("Static initializer") + .hasMessageContaining("assertThatThrownBy"); + } + + @Test + void an_allowed_assertion_is_accepted() { + no_blacklisted_methods_are_used(fixture("blacklistmethods.good")); + } + + /** + * A blacklist entry naming a method that does not exist can never match, so it reads as coverage + * without providing any. + */ + @Test + void the_blacklist_does_not_name_assertj_methods_that_do_not_exist() { + assertThat(BLACKLISTED_METHODS) + .doesNotContain( + "org.assertj.core.api.Assertions.assertThrows", + "org.assertj.core.api.Assertions.assertThrowsExactly", + "org.assertj.core.api.Assertions.assertDoesNotThrow" + ); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/EnforceJspecifyArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/EnforceJspecifyArchRuleTest.java new file mode 100644 index 0000000..c6b14ee --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/EnforceJspecifyArchRuleTest.java @@ -0,0 +1,29 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class EnforceJspecifyArchRuleTest implements EnforceJspecifyArchRule { + @Test + void a_top_level_class_without_a_jspecify_annotation_is_reported() { + var classes = fixture("jspecify.bad"); + + var failure = violationOf(() -> top_level_classes_must_be_annotated_with_jspecify(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("UnannotatedClass") + .hasMessageContaining("NullMarked"); + } + + @Test + void an_annotated_top_level_class_is_accepted() { + top_level_classes_must_be_annotated_with_jspecify(fixture("jspecify.good")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRuleTest.java new file mode 100644 index 0000000..5c58d80 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRuleTest.java @@ -0,0 +1,49 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class NoSystemOutOrErrArchRuleTest implements NoSystemOutOrErrArchRule { + @Test + void a_console_write_from_a_method_is_reported() { + var failure = violationOf(() -> no_system_out_or_err_is_used(fixture("systemout.badmethod"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("Method").hasMessageContaining("java.lang.System.out"); + } + + @Test + void a_console_write_from_a_constructor_is_reported() { + var failure = violationOf(() -> no_system_out_or_err_is_used(fixture("systemout.badconstructor"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("Constructor").hasMessageContaining("java.lang.System.err"); + } + + @Test + void a_console_write_from_a_static_initializer_is_reported() { + var failure = violationOf(() -> no_system_out_or_err_is_used(fixture("systemout.badstatic"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("Static initializer"); + } + + @Test + void a_console_write_from_a_lambda_is_reported() { + var failure = violationOf(() -> no_system_out_or_err_is_used(fixture("systemout.badlambda"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("java.lang.System.out"); + } + + @Test + void a_class_writing_to_no_console_is_accepted() { + no_system_out_or_err_is_used(fixture("systemout.good")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java new file mode 100644 index 0000000..3f6e46b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java @@ -0,0 +1,39 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static it.aboutbits.archunit.toolbox.rule.base.RecordPropertiesMustBeAccessedViaAccessorArchRule.record_properties_must_be_accessed_via_accessor; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class RecordPropertiesMustBeAccessedViaAccessorArchRuleTest { + /** + * Only reachable for a nested record: nestmates share access to private members, so the read + * compiles to a direct field access rather than an accessor call. + */ + @Test + void a_direct_read_of_a_record_field_from_outside_the_record_is_reported() { + var classes = fixture("recordaccessor.badnested"); + + var failure = violationOf(() -> record_properties_must_be_accessed_via_accessor.check(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("Record property [amount]") + .hasMessageContaining("Use accessor method [amount()] instead"); + } + + @Test + void a_read_through_the_accessor_is_accepted() { + record_properties_must_be_accessed_via_accessor.check(fixture("recordaccessor.goodaccessor")); + } + + @Test + void a_record_opting_out_of_the_rule_is_accepted() { + record_properties_must_be_accessed_via_accessor.check(fixture("recordaccessor.goodoptout")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java new file mode 100644 index 0000000..7efacb3 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java @@ -0,0 +1,35 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class TestClassInCorrectPackageArchRuleTest implements TestClassInCorrectPackageArchRule { + @Test + void a_test_class_without_a_production_class_in_the_same_package_is_reported() { + var classes = fixture("testclasspackage.bad"); + + var failure = violationOf( + () -> test_classes_should_be_in_the_same_package_as_their_production_code(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("does not have a matching production class") + .hasMessageContaining("it.aboutbits.archunit.fixture.testclasspackage.bad.Widget"); + } + + @Test + void a_test_class_next_to_its_production_class_is_accepted() { + test_classes_should_be_in_the_same_package_as_their_production_code(fixture("testclasspackage.good")); + } + + @Test + void a_test_class_annotated_as_having_no_production_counterpart_is_accepted() { + test_classes_should_be_in_the_same_package_as_their_production_code(fixture("testclasspackage.goodoptout")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRuleTest.java new file mode 100644 index 0000000..d4fbe0b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRuleTest.java @@ -0,0 +1,29 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class TestClassVisibilityArchRuleTest implements TestClassVisibilityArchRule { + @Test + void a_public_test_class_is_reported() { + var classes = fixture("testclassvisibility.bad"); + + var failure = violationOf(() -> test_classes_must_be_package_private(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("WidgetTest") + .hasMessageContaining("package private"); + } + + @Test + void a_package_private_test_class_is_accepted() { + test_classes_must_be_package_private(fixture("testclassvisibility.good")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRuleTest.java new file mode 100644 index 0000000..5124975 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRuleTest.java @@ -0,0 +1,29 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class TestMethodVisibilityArchRuleTest implements TestMethodVisibilityArchRule { + @Test + void a_public_test_method_is_reported() { + var classes = fixture("testmethodvisibility.bad"); + + var failure = violationOf(() -> test_methods_must_be_package_private(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("it_works") + .hasMessageContaining("package private"); + } + + @Test + void a_package_private_test_method_is_accepted() { + test_methods_must_be_package_private(fixture("testmethodvisibility.good")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java new file mode 100644 index 0000000..c7c7368 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java @@ -0,0 +1,58 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class TestNestedClassMatchNameArchRuleTest implements TestNestedClassMatchNameArchRule { + @Test + void a_nested_test_class_naming_no_production_method_is_reported() { + var classes = fixture("nestedclassname.badmethod"); + + var failure = violationOf(() -> nested_test_classes_have_matching_production_method_name(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("WidgetTest$DoSomethingElse") + .hasMessageContaining("doSomethingElse"); + } + + @Test + void a_nested_group_without_a_matching_production_class_is_reported() { + var classes = fixture("nestedclassname.badgroup"); + + var failure = violationOf(() -> nested_test_classes_have_matching_production_method_name(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("WidgetTest$DeleteAction$DeleteAll") + .hasMessageContaining("does not have a matching production class"); + } + + @Test + void a_nested_test_class_whose_production_class_is_missing_entirely_is_reported() { + var classes = fixture("nestedclassname.badnoproduction"); + + var failure = violationOf(() -> nested_test_classes_have_matching_production_method_name(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("WidgetTest$DoWork") + .hasMessageContaining("does not have a matching production class"); + } + + @Test + void a_nested_test_class_matching_a_production_method_is_accepted() { + nested_test_classes_have_matching_production_method_name(fixture("nestedclassname.good")); + } + + @Test + void a_test_class_annotated_as_having_no_production_counterpart_is_accepted() { + nested_test_classes_have_matching_production_method_name(fixture("nestedclassname.goodoptout")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRuleTest.java new file mode 100644 index 0000000..280ebd8 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRuleTest.java @@ -0,0 +1,29 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class TestNestedClassVisibilityArchRuleTest implements TestNestedClassVisibilityArchRule { + @Test + void a_public_nested_test_class_is_reported() { + var classes = fixture("nestedclassvisibility.bad"); + + var failure = violationOf(() -> nested_test_classes_must_be_package_private(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("WidgetTest$DoWork") + .hasMessageContaining("package private"); + } + + @Test + void a_package_private_nested_test_class_is_accepted() { + nested_test_classes_must_be_package_private(fixture("nestedclassvisibility.good")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java new file mode 100644 index 0000000..efd95b9 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java @@ -0,0 +1,63 @@ +package it.aboutbits.archunit.toolbox.rule.common; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class ControllerRequestMappingsMustBeSecurityTestedTest implements ControllerRequestMappingsMustBeSecurityTested { + @Test + void a_mapped_method_without_a_security_test_class_is_reported() { + var classes = fixture("securitytested.badmissing"); + + var failure = violationOf( + () -> controller_methods_with_request_mapping_must_be_security_tested(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("getAll") + .hasMessageContaining("WidgetControllerSecurityTest is missing"); + } + + @Test + void a_security_test_class_without_the_nested_method_class_is_reported() { + var classes = fixture("securitytested.badnonested"); + + var failure = violationOf( + () -> controller_methods_with_request_mapping_must_be_security_tested(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("does not contain a @Nested test class named GetAll"); + } + + /** + * getAll() must not be considered covered by the @Nested class belonging to getAllArchived(). + */ + @Test + void a_mapped_method_covered_only_by_a_longer_named_sibling_is_reported() { + var classes = fixture("securitytested.badprefix"); + + var failure = violationOf( + () -> controller_methods_with_request_mapping_must_be_security_tested(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("getAll()") + .hasMessageContaining("does not contain a @Nested test class named GetAll"); + } + + @Test + void a_mapped_method_with_a_matching_nested_test_class_is_accepted() { + controller_methods_with_request_mapping_must_be_security_tested(fixture("securitytested.good")); + } + + /** A @Nested class grouped inside the method-named class still counts as coverage. */ + @Test + void a_mapped_method_covered_by_a_nested_group_is_accepted() { + controller_methods_with_request_mapping_must_be_security_tested(fixture("securitytested.goodnestedgroup")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java new file mode 100644 index 0000000..6eac599 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java @@ -0,0 +1,43 @@ +package it.aboutbits.archunit.toolbox.rule.common; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationCount; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class SortMappingsExhaustiveArchRuleTest implements SortMappingsExhaustiveArchRule { + @Test + void a_sort_enum_value_without_a_mapping_is_reported() { + var classes = fixture("sortmappings.bad"); + + var failure = violationOf(() -> sort_mappings_cover_all_sort_enum_values(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("is missing mappings for enum WidgetSort values") + .hasMessageContaining("CREATED_AT"); + } + + /** + * A non-static field cannot be read reflectively, so it used to be skipped with nothing but a + * discarded log warning. + */ + @Test + void a_non_static_sort_mappings_field_is_reported() { + var classes = fixture("sortmappings.badnonstatic"); + + var failure = violationOf(() -> sort_mappings_cover_all_sort_enum_values(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("must be static"); + } + + @Test + void exhaustive_sort_mappings_are_accepted() { + sort_mappings_cover_all_sort_enum_values(fixture("sortmappings.good")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/util/LineNumberUtilTest.java b/src/test/java/it/aboutbits/archunit/toolbox/util/LineNumberUtilTest.java new file mode 100644 index 0000000..2073d12 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/util/LineNumberUtilTest.java @@ -0,0 +1,54 @@ +package it.aboutbits.archunit.toolbox.util; + +import com.tngtech.archunit.core.importer.ClassFileImporter; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.util.LineNumberUtil.getLineNumber; +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class LineNumberUtilTest { + @Nested + class GetLineNumber { + @Test + void a_class_falls_back_to_the_line_of_its_constructor() { + var javaClass = new ClassFileImporter().importClass(WithConstructor.class); + + // Bytecode carries no line for the class itself, hence the constructor fallback + assertThat(getLineNumber(javaClass)).isPositive(); + } + + @Test + void a_type_without_any_constructor_reports_no_line() { + var javaInterface = new ClassFileImporter().importClass(WithoutConstructor.class); + + assertThat(getLineNumber(javaInterface)).isZero(); + } + + @Test + void a_method_reports_its_own_line() { + var javaClass = new ClassFileImporter().importClass(WithConstructor.class); + + assertThat(getLineNumber(javaClass.getMethod("value"))).isPositive(); + } + } + + private static final class WithConstructor { + private final String value; + + private WithConstructor(String value) { + this.value = value; + } + + @SuppressWarnings("unused") + String value() { + return value; + } + } + + private interface WithoutConstructor { + String value(); + } +} diff --git a/src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java b/src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java new file mode 100644 index 0000000..2a21048 --- /dev/null +++ b/src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java @@ -0,0 +1,16 @@ +package it.aboutbits.springboot.toolbox.archunit; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Stub of the spring-boot-toolbox opt-out annotation. Verified against the real artifact: RUNTIME + * retention, targets TYPE, FIELD and RECORD_COMPONENT. + */ +@Target({ElementType.TYPE, ElementType.FIELD, ElementType.RECORD_COMPONENT}) +@Retention(RetentionPolicy.RUNTIME) +public @interface ArchAllowDirectAccess { + String reason(); +} diff --git a/src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java b/src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java new file mode 100644 index 0000000..bcf308e --- /dev/null +++ b/src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java @@ -0,0 +1,18 @@ +package it.aboutbits.springboot.toolbox.persistence; + +import java.util.HashMap; + +/** + * Stub of the spring-boot-toolbox type. Verified against the real artifact: it extends HashMap keyed + * by the Sort enum, which is what the rule relies on when it reads the mappings reflectively. + */ +public class SortMappings> extends HashMap { + @SafeVarargs + public static > SortMappings of(T... keys) { + var mappings = new SortMappings(); + for (var key : keys) { + mappings.put(key, key.name()); + } + return mappings; + } +} diff --git a/src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java b/src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java new file mode 100644 index 0000000..a8d25fb --- /dev/null +++ b/src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java @@ -0,0 +1,16 @@ +package it.aboutbits.springboot.toolbox.stereotype; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Stub of the spring-boot-toolbox stereotype. spring-boot-toolbox depends on archunit-toolbox, so it + * cannot be a dependency here. Verified against the real artifact: RUNTIME retention, TYPE target. + */ +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +public @interface Store { + String value(); +} diff --git a/src/test/java/net/datafaker/Faker.java b/src/test/java/net/datafaker/Faker.java new file mode 100644 index 0000000..ada3e8d --- /dev/null +++ b/src/test/java/net/datafaker/Faker.java @@ -0,0 +1,8 @@ +package net.datafaker; + +/** Stub of the blacklisted net.datafaker.Faker. */ +public class Faker { + public String name() { + return "stub"; + } +} diff --git a/src/test/java/org/junit/Ignore.java b/src/test/java/org/junit/Ignore.java new file mode 100644 index 0000000..a9fcb71 --- /dev/null +++ b/src/test/java/org/junit/Ignore.java @@ -0,0 +1,15 @@ +package org.junit; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Stub of the blacklisted JUnit 4 annotation, so a rule test can pin a real entry of + * BLACKLISTED_ANNOTATIONS without putting JUnit 4 on this library's classpath. + */ +@Target({ElementType.TYPE, ElementType.METHOD}) +@Retention(RetentionPolicy.RUNTIME) +public @interface Ignore { +} From 0aef3db41c2c2fc0180db116534c654e3c1b9f73 Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Wed, 19 Aug 2026 12:31:19 +0200 Subject: [PATCH 2/9] 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 --- .../base/BlacklistAnnotationsArchRule.java | 32 +++--- .../rule/base/BlacklistMethodsArchRule.java | 51 +++------- .../rule/base/NoSystemOutOrErrArchRule.java | 50 ++++------ .../TestClassInCorrectPackageArchRule.java | 25 ++--- .../base/TestClassVisibilityArchRule.java | 5 +- .../archunit/toolbox/util/CodeUnitUtil.java | 30 ++++++ .../archunit/toolbox/util/TestClassNames.java | 98 +++++++++++++++++++ .../toolbox/util/CodeUnitUtilTest.java | 38 +++++++ .../toolbox/util/TestClassNamesTest.java | 65 ++++++++++++ 9 files changed, 293 insertions(+), 101 deletions(-) create mode 100644 src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java create mode 100644 src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtilTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRule.java index f44afb7..f3e5cb1 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRule.java +++ b/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"}) @@ -87,33 +88,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)); } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java index 863e875..90d5492 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.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"}) @@ -22,9 +23,6 @@ public interface BlacklistMethodsArchRule { Set.of( // We should use `assertThatExceptionOfType(...).isThrownBy(...)` instead of `assertThatThrownBy(...)` "org.assertj.core.api.Assertions.assertThatThrownBy", - "org.assertj.core.api.Assertions.assertThrows", - "org.assertj.core.api.Assertions.assertThrowsExactly", - "org.assertj.core.api.Assertions.assertDoesNotThrow", "org.junit.jupiter.api.Assertions.assertThrows", "org.junit.jupiter.api.Assertions.assertDoesNotThrow", // assertThat (allowed is only org.assertj.core.api.Assertions.assertThat) @@ -80,47 +78,30 @@ public NotUseBlacklistedMethods() { @Override public void check(JavaClass javaClass, ConditionEvents events) { - // Check all method calls from this class - for (var method : javaClass.getMethods()) { - for (var methodCall : method.getMethodCallsFromSelf()) { + // getCodeUnits() covers methods, constructors and the static initializer. getMethods() + // would miss constructors, and with them every instance field initializer. + for (var codeUnit : javaClass.getCodeUnits()) { + for (var methodCall : codeUnit.getMethodCallsFromSelf()) { var fullMethodName = "%s.%s".formatted( methodCall.getTargetOwner().getFullName(), methodCall.getTarget().getName() ); - if (BLACKLISTED_METHODS.contains(fullMethodName)) { - var message = String.format( - "Method %s calls blacklisted method %s (%s.java:%d)", - method.getFullName(), - fullMethodName, - javaClass.getSimpleName(), - getLineNumber(methodCall) - ); - events.add(SimpleConditionEvent.violated(method, message)); + if (!BLACKLISTED_METHODS.contains(fullMethodName)) { + continue; } - } - } - // Check static initializers for method calls - javaClass.getStaticInitializer().ifPresent(staticInitializer -> { - for (var methodCall : staticInitializer.getMethodCallsFromSelf()) { - var fullMethodName = "%s.%s".formatted( - methodCall.getTargetOwner().getFullName(), - methodCall.getTarget().getName() + var message = String.format( + "%s %s calls blacklisted method %s (%s.java:%d)", + describeKind(codeUnit), + codeUnit.getFullName(), + fullMethodName, + javaClass.getSimpleName(), + getLineNumber(methodCall) ); - - if (BLACKLISTED_METHODS.contains(fullMethodName)) { - var message = String.format( - "Static initializer in %s calls blacklisted method %s (%s.java:%d)", - javaClass.getFullName(), - fullMethodName, - javaClass.getSimpleName(), - getLineNumber(methodCall) - ); - events.add(SimpleConditionEvent.violated(staticInitializer, message)); - } + events.add(SimpleConditionEvent.violated(codeUnit, message)); } - }); + } } } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRule.java index 79b99f4..f290997 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRule.java @@ -9,6 +9,7 @@ import org.jspecify.annotations.NullMarked; 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"}) @@ -33,44 +34,27 @@ public NotUseSystemOutOrErr() { @Override public void check(JavaClass javaClass, ConditionEvents events) { - checkCodeUnits(javaClass, events); - javaClass.getStaticInitializer().ifPresent(staticInitializer -> { - for (var fieldAccess : staticInitializer.getFieldAccesses()) { - if (isSystemOutOrErr( + // getCodeUnits() covers methods, constructors and the static initializer. getMethods() + // would miss constructors, and with them every instance field initializer. + for (var codeUnit : javaClass.getCodeUnits()) { + for (var fieldAccess : codeUnit.getFieldAccesses()) { + if (!isSystemOutOrErr( fieldAccess.getTargetOwner().getFullName(), fieldAccess.getTarget().getName() )) { - var message = String.format( - "Static initializer in %s accesses %s.%s (%s.java:%d)", - javaClass.getFullName(), - SYSTEM_CLASS, - fieldAccess.getTarget().getName(), - javaClass.getSimpleName(), - getLineNumber(fieldAccess) - ); - events.add(SimpleConditionEvent.violated(staticInitializer, message)); + continue; } - } - }); - } - private void checkCodeUnits(JavaClass javaClass, ConditionEvents events) { - for (var method : javaClass.getMethods()) { - for (var fieldAccess : method.getFieldAccesses()) { - if (isSystemOutOrErr( - fieldAccess.getTargetOwner().getFullName(), - fieldAccess.getTarget().getName() - )) { - var message = String.format( - "Method %s accesses %s.%s (%s.java:%d)", - method.getFullName(), - SYSTEM_CLASS, - fieldAccess.getTarget().getName(), - javaClass.getSimpleName(), - getLineNumber(fieldAccess) - ); - events.add(SimpleConditionEvent.violated(method, message)); - } + var message = String.format( + "%s %s accesses %s.%s (%s.java:%d)", + describeKind(codeUnit), + codeUnit.getFullName(), + SYSTEM_CLASS, + fieldAccess.getTarget().getName(), + javaClass.getSimpleName(), + getLineNumber(fieldAccess) + ); + events.add(SimpleConditionEvent.violated(codeUnit, message)); } } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java index 6481dd7..dba6c1d 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java @@ -6,10 +6,10 @@ import com.tngtech.archunit.lang.ArchCondition; import com.tngtech.archunit.lang.ConditionEvents; import com.tngtech.archunit.lang.SimpleConditionEvent; +import it.aboutbits.archunit.toolbox.util.TestClassNames; import org.jspecify.annotations.NullMarked; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; -import static it.aboutbits.archunit.toolbox.config.ArchRuleConfig.TEST_CLASS_SUFFIXES; @SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"}) @NullMarked @@ -17,10 +17,7 @@ public interface TestClassInCorrectPackageArchRule { @SuppressWarnings({"unused", "checkstyle:MethodName", "java:S100"}) @ArchTest default void test_classes_should_be_in_the_same_package_as_their_production_code(JavaClasses classes) { - classes().that() - .haveNameMatching(".+(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$") - .and() - .doNotHaveSimpleName("ArchitectureTest") + classes().that(TestClassNames.testClasses()) .and() .areNotAnnotatedWith(org.junit.jupiter.api.Disabled.class) .and() @@ -30,7 +27,6 @@ default void test_classes_should_be_in_the_same_package_as_their_production_code .and() .resideOutsideOfPackages(".._support..", ".._config..") .should(new BeInTheSamePackageAsTheProductionClass(classes)) - .allowEmptyShould(true) .check(classes); } @@ -44,18 +40,15 @@ public BeInTheSamePackageAsTheProductionClass(JavaClasses allClasses) { @Override public void check(JavaClass testClass, ConditionEvents events) { - var testClassSuffixRegex = "(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$"; - - var testClassName = testClass.getSimpleName(); - if (!testClassName.matches(testClassSuffixRegex)) { - return; - } - - // Derive the production class name - var productionClassSimpleName = testClassName.replaceAll(testClassSuffixRegex, ""); + /* + * No suffix guard here on purpose. The selection above already guarantees the suffix, + * and re-deriving it in the condition is what previously disabled this rule outright: + * the guard rebuilt the regex without the leading ".+", and String.matches anchors both + * ends, so every test class returned before ever looking for its production class. + */ + var productionClassSimpleName = TestClassNames.productionClassSimpleName(testClass.getSimpleName()); var productionClassFullName = testClass.getPackageName() + "." + productionClassSimpleName; - // Check if the production class exists in the same package var productionClass = allClasses.stream() .filter(clazz -> clazz.getFullName().equals(productionClassFullName)) .findFirst(); diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java index d601051..88434d0 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java @@ -2,10 +2,10 @@ import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.junit.ArchTest; +import it.aboutbits.archunit.toolbox.util.TestClassNames; import org.jspecify.annotations.NullMarked; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; -import static it.aboutbits.archunit.toolbox.config.ArchRuleConfig.TEST_CLASS_SUFFIXES; @SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"}) @NullMarked @@ -14,8 +14,7 @@ public interface TestClassVisibilityArchRule { @ArchTest default void test_classes_must_be_package_private(JavaClasses classes) { classes() - .that() - .haveNameMatching(".+(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$") + .that(TestClassNames.testClasses()) .and() .resideOutsideOfPackages(".._support..", ".._config..") .should() diff --git a/src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java b/src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java new file mode 100644 index 0000000..4302256 --- /dev/null +++ b/src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java @@ -0,0 +1,30 @@ +package it.aboutbits.archunit.toolbox.util; + +import com.tngtech.archunit.core.domain.JavaCodeUnit; +import com.tngtech.archunit.core.domain.JavaConstructor; +import com.tngtech.archunit.core.domain.JavaMethod; +import com.tngtech.archunit.core.domain.JavaStaticInitializer; +import org.jspecify.annotations.NullMarked; + +@NullMarked +public final class CodeUnitUtil { + private CodeUnitUtil() { + } + + /** + * Human readable kind of a code unit, for violation messages. + *

+ * Rules that inspect bodies must iterate {@code getCodeUnits()} rather than {@code getMethods()}: + * the latter excludes constructors, and an instance field initializer is compiled into the + * constructor, so both are invisible to a rule that only looks at methods. + *

+ */ + public static String describeKind(JavaCodeUnit codeUnit) { + return switch (codeUnit) { + case JavaMethod _ -> "Method"; + case JavaConstructor _ -> "Constructor"; + case JavaStaticInitializer _ -> "Static initializer"; + default -> "Code unit"; + }; + } +} diff --git a/src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java b/src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java new file mode 100644 index 0000000..8cfa4f9 --- /dev/null +++ b/src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java @@ -0,0 +1,98 @@ +package it.aboutbits.archunit.toolbox.util; + +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaClass; +import org.jspecify.annotations.NullMarked; + +import java.util.Comparator; +import java.util.stream.Collectors; + +import static it.aboutbits.archunit.toolbox.config.ArchRuleConfig.TEST_CLASS_SUFFIXES; + +/** + * Single source of truth for recognising a test class by its name suffix. + *

+ * Every rule that selects test classes must go through {@link #testClasses()}, and every rule that + * derives a production class name must go through {@link #productionClassSimpleName(String)}. + * Hand-building the suffix regex per rule is what allowed a condition to be anchored differently + * from the selection that fed it, silently disabling the rule. + *

+ */ +@NullMarked +public final class TestClassNames { + private TestClassNames() { + } + + /** + * Matches the name of a test class: at least one character, then one of the configured suffixes. + *

+ * The leading {@code .+} is load-bearing. {@link String#matches(String)} and ArchUnit's + * {@code haveNameMatching} both anchor at each end, so without it the pattern only matches a + * class named exactly {@code Test}. It is also what keeps such a class out of the selection, + * since it has no name left once the suffix is stripped. + *

+ */ + public static String testClassNameRegex() { + return ".+(" + suffixAlternation() + ")$"; + } + + /** + * Matches only the trailing suffix, for stripping it off a test class name. Deliberately not + * anchored at the start - this is used with {@link String#replaceAll(String, String)}, never + * with {@link String#matches(String)}. + */ + public static String suffixRegex() { + return "(" + suffixAlternation() + ")$"; + } + + /** + * Whether a simple name is the name of a test class. + *

+ * Requires a production class name to be left over once the suffix is stripped, so that the + * selection cannot disagree with {@link #productionClassSimpleName(String)}. "CacheTest" matches + * the pattern with "Cache" as the leading {@code .+}, yet stripping removes "CacheTest" whole and + * leaves nothing to look for. + *

+ */ + public static boolean isTestClassName(String simpleName) { + return simpleName.matches(testClassNameRegex()) + && !productionClassSimpleName(simpleName).isEmpty(); + } + + /** + * The simple name of the production class a test class belongs to, e.g. {@code WidgetCacheTest} + * to {@code Widget}. + */ + public static String productionClassSimpleName(String testClassSimpleName) { + return testClassSimpleName.replaceAll(suffixRegex(), ""); + } + + /** + * Selects test classes by their simple name. Matching the simple name rather than the + * fully qualified name keeps the selection and the conditions that follow it in agreement: + * {@code some.pkg.Test} matches the fully qualified name but is not a test class. + */ + public static DescribedPredicate testClasses() { + return new DescribedPredicate<>("have a simple name matching '%s'".formatted(testClassNameRegex())) { + @Override + public boolean test(JavaClass javaClass) { + return isTestClassName(javaClass.getSimpleName()); + } + }; + } + + /* + * Longest suffix first, then alphabetically. TEST_CLASS_SUFFIXES is a mutable HashSet, so + * without an explicit order the generated regex - and every rule description built from it - + * varies between JVM runs. + */ + private static String suffixAlternation() { + var longestFirst = Comparator.comparingInt(String::length) + .reversed() + .thenComparing(Comparator.naturalOrder()); + + return TEST_CLASS_SUFFIXES.stream() + .sorted(longestFirst) + .collect(Collectors.joining("|")); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtilTest.java b/src/test/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtilTest.java new file mode 100644 index 0000000..2523c17 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtilTest.java @@ -0,0 +1,38 @@ +package it.aboutbits.archunit.toolbox.util; + +import com.tngtech.archunit.core.importer.ClassFileImporter; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class CodeUnitUtilTest { + @Test + void every_kind_of_code_unit_is_described() { + var javaClass = new ClassFileImporter().importClass(Described.class); + + var kinds = javaClass.getCodeUnits() + .stream() + .map(CodeUnitUtil::describeKind) + .distinct() + .toList(); + + assertThat(kinds).containsExactlyInAnyOrder("Method", "Constructor", "Static initializer"); + } + + @SuppressWarnings("unused") + private static final class Described { + private static final String CONSTANT; + + static { + CONSTANT = "constant"; + } + + private Described() { + } + + private void method() { + } + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java b/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java new file mode 100644 index 0000000..797ee8a --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java @@ -0,0 +1,65 @@ +package it.aboutbits.archunit.toolbox.util; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class TestClassNamesTest { + @Test + void a_name_ending_in_a_configured_suffix_is_a_test_class_name() { + assertThat(TestClassNames.isTestClassName("WidgetTest")).isTrue(); + assertThat(TestClassNames.isTestClassName("WidgetCacheTest")).isTrue(); + assertThat(TestClassNames.isTestClassName("WidgetEventTest")).isTrue(); + assertThat(TestClassNames.isTestClassName("WidgetSecurityTest")).isTrue(); + } + + /** + * The regression guard for the defect that disabled the counterpart rule outright: the condition + * rebuilt the pattern without the leading ".+", and String.matches anchors both ends, so only a + * class named exactly "Test" got through. + */ + @Test + void the_pattern_is_not_satisfied_by_the_bare_suffix_alone() { + assertThat(TestClassNames.isTestClassName("Test")).isFalse(); + assertThat("WidgetTest".matches(TestClassNames.testClassNameRegex())).isTrue(); + } + + /** + * "CacheTest" matches the pattern with "Cache" as the leading ".+", but stripping removes + * "CacheTest" whole, so there would be no production class name left to look for. + */ + @Test + void a_name_that_strips_down_to_nothing_is_not_a_test_class_name() { + assertThat(TestClassNames.isTestClassName("CacheTest")).isFalse(); + assertThat(TestClassNames.isTestClassName("SecurityTest")).isFalse(); + assertThat(TestClassNames.isTestClassName("WidgetCacheTest")).isTrue(); + } + + @Test + void a_name_not_ending_in_a_configured_suffix_is_not_a_test_class_name() { + assertThat(TestClassNames.isTestClassName("Widget")).isFalse(); + assertThat(TestClassNames.isTestClassName("WidgetTester")).isFalse(); + assertThat(TestClassNames.isTestClassName("TestWidget")).isFalse(); + } + + @Test + void the_production_class_name_is_the_name_without_its_suffix() { + assertThat(TestClassNames.productionClassSimpleName("WidgetTest")).isEqualTo("Widget"); + assertThat(TestClassNames.productionClassSimpleName("WidgetCacheTest")).isEqualTo("Widget"); + assertThat(TestClassNames.productionClassSimpleName("WidgetEventTest")).isEqualTo("Widget"); + assertThat(TestClassNames.productionClassSimpleName("WidgetSecurityTest")).isEqualTo("Widget"); + } + + /** + * TEST_CLASS_SUFFIXES is a mutable HashSet, so without an explicit ordering the generated regex + * and every rule description built from it would vary between JVM runs. + */ + @Test + void the_generated_pattern_has_a_stable_order() { + assertThat(TestClassNames.testClassNameRegex()) + .isEqualTo(TestClassNames.testClassNameRegex()) + .isEqualTo(".+(SecurityTest|CacheTest|EventTest|Test)$"); + } +} From 630fab9d24f1a31eb63f5141751399621b7104ac Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Wed, 19 Aug 2026 12:31:40 +0200 Subject: [PATCH 3/9] 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 --- .../base/TestMethodVisibilityArchRule.java | 1 - .../TestNestedClassMatchNameArchRule.java | 26 +- .../TestNestedClassVisibilityArchRule.java | 1 - ...erRequestMappingsMustBeSecurityTested.java | 21 +- .../SortMappingsExhaustiveArchRule.java | 275 +++++++++--------- .../toolbox/support/ArchIgnoreGroupName.java | 10 +- .../ArchIgnoreNoProductionCounterpart.java | 10 +- .../archunit/toolbox/ArchitectureTest.java | 10 + .../archunit/toolbox/EmptySelectionTest.java | 75 +++++ .../support/ArchIgnoreGroupNameTest.java | 18 ++ ...ArchIgnoreNoProductionCounterpartTest.java | 23 ++ 11 files changed, 303 insertions(+), 167 deletions(-) create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupNameTest.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRule.java index 5b16356..81d76dd 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRule.java @@ -32,7 +32,6 @@ public boolean test(JavaClass javaClass) { }) .should() .bePackagePrivate() - .allowEmptyShould(true) .check(classes); } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java index 16cbb1c..0d31c94 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java @@ -7,12 +7,12 @@ import com.tngtech.archunit.lang.ArchCondition; import com.tngtech.archunit.lang.ConditionEvents; import com.tngtech.archunit.lang.SimpleConditionEvent; +import it.aboutbits.archunit.toolbox.util.TestClassNames; import org.jspecify.annotations.NullMarked; import java.util.stream.Collectors; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; -import static it.aboutbits.archunit.toolbox.config.ArchRuleConfig.TEST_CLASS_SUFFIXES; import static it.aboutbits.archunit.toolbox.util.LineNumberUtil.getLineNumber; @SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"}) @@ -21,14 +21,18 @@ public interface TestNestedClassMatchNameArchRule { @SuppressWarnings({"unused", "checkstyle:MethodName", "java:S100"}) @ArchTest default void nested_test_classes_have_matching_production_method_name(JavaClasses classes) { - classes().that() - .haveNameMatching(".+(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$") + classes().that(TestClassNames.testClasses()) .and() .areNotAnnotatedWith(org.junit.jupiter.api.Disabled.class) .and() .areNotAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) + .and() + /* + * A test class that declares it has no production counterpart has no production + * methods to match its @Nested classes against either. + */ + .areNotAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart.class) .should(new HaveNestedClassesThatHaveAMatchingProductionMethodName(classes)) - .allowEmptyShould(true) .check(classes); } @@ -113,8 +117,7 @@ public void check(JavaClass testClass, ConditionEvents events) { var productionClassName = "%s.%s%s".formatted( testClass.getPackageName(), - testClass.getSimpleName() - .replaceAll("(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$", ""), + TestClassNames.productionClassSimpleName(testClass.getSimpleName()), enclosingClassSuffix.orElse("") ); @@ -122,7 +125,12 @@ public void check(JavaClass testClass, ConditionEvents events) { .filter(clazz -> clazz.getFullName().equals(productionClassName)) .findFirst(); - if (productionClassOptional.isEmpty() && enclosingClassSuffix.isPresent()) { + /* + * Reported whether or not the @Nested class is inside a @Nested group. Without a + * production class there is nothing to match the name against, so staying silent + * here means the @Nested class is never checked at all. + */ + if (productionClassOptional.isEmpty()) { var message = "The @Nested test class <%s> (%s.java:%s)%ndoes not have a matching production class <%s>".formatted( nestedClass.getName(), nestedClassBaseClassSimpleName, @@ -130,9 +138,7 @@ public void check(JavaClass testClass, ConditionEvents events) { productionClassName ); events.add(SimpleConditionEvent.violated(nestedClass, message)); - } - - if (productionClassOptional.isPresent()) { + } else { var productionClass = productionClassOptional.get(); var methodExists = productionClass.getMethods() diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRule.java index dbac531..3558a85 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRule.java @@ -17,7 +17,6 @@ default void nested_test_classes_must_be_package_private(JavaClasses classes) { .areAnnotatedWith(org.junit.jupiter.api.Nested.class) .should() .bePackagePrivate() - .allowEmptyShould(true) .check(classes); } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java index abbaf9f..ad1a429 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java @@ -80,14 +80,15 @@ public void check(JavaMethod method, ConditionEvents events) { return; } + var expectedNestedClassName = "%s$%s".formatted( + securityTestClass.getName(), + expectedNestedMethodClassName + ); + var nestedMethodTestClassFound = securityTestClass.getPackage() .getClasses() .stream() - .anyMatch(clazz -> clazz.getName() - .startsWith("%s$%s".formatted( - securityTestClass.getName(), - expectedNestedMethodClassName - )) + .anyMatch(clazz -> isExpectedNestedClass(clazz.getName(), expectedNestedClassName) && clazz.isAnnotatedWith(org.junit.jupiter.api.Nested.class) && !clazz.isAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) && !clazz.isAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class) @@ -106,5 +107,15 @@ public void check(JavaMethod method, ConditionEvents events) { )); } } + + /* + * The @Nested class named after the controller method, or a @Nested class grouped inside it + * (GetAll$WhenAdmin). Matching on a bare prefix would also accept an unrelated longer + * sibling, so getAll() would count as covered by a @Nested class named GetAllArchived. + */ + private static boolean isExpectedNestedClass(String candidateName, String expectedName) { + return candidateName.equals(expectedName) + || candidateName.startsWith(expectedName + "$"); + } } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java index 1349b2e..bb5452b 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java @@ -2,17 +2,17 @@ import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.domain.JavaField; +import com.tngtech.archunit.core.domain.JavaModifier; import com.tngtech.archunit.core.domain.JavaParameterizedType; import com.tngtech.archunit.junit.ArchTest; import com.tngtech.archunit.lang.ArchCondition; import com.tngtech.archunit.lang.ConditionEvents; import com.tngtech.archunit.lang.SimpleConditionEvent; -import it.aboutbits.archunit.toolbox.util.LineNumberUtil; -import lombok.extern.slf4j.Slf4j; import org.jspecify.annotations.NullMarked; -import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -22,6 +22,8 @@ @NullMarked public interface SortMappingsExhaustiveArchRule { + String SORT_MAPPINGS_CLASS = "it.aboutbits.springboot.toolbox.persistence.SortMappings"; + @SuppressWarnings({"unused", "checkstyle:MethodName", "java:S100"}) @ArchTest default void sort_mappings_cover_all_sort_enum_values(JavaClasses classes) { @@ -29,11 +31,17 @@ default void sort_mappings_cover_all_sort_enum_values(JavaClasses classes) { .that() .areAnnotatedWith("it.aboutbits.springboot.toolbox.stereotype.Store") .should(new HaveExhaustiveSortMappingsIfPresent()) - .allowEmptyShould(true) .check(classes); } - @Slf4j + /** + * Checks that every value of a Sort enum has a mapping. + *

+ * Every case this cannot verify is reported as a violation rather than skipped. Reading a + * mapping requires reflection, and a mapping that cannot be read is indistinguishable from one + * that is exhaustive - so silence here means the rule quietly stops covering that field. + *

+ */ class HaveExhaustiveSortMappingsIfPresent extends ArchCondition { public HaveExhaustiveSortMappingsIfPresent() { super("have SortMappings that map all values of the associated Sort enum"); @@ -41,157 +49,144 @@ public HaveExhaustiveSortMappingsIfPresent() { @Override public void check(JavaClass javaClass, ConditionEvents events) { - var mappings = detectSortMappings(javaClass); + var sortMappingsFields = javaClass.getFields() + .stream() + .filter(field -> field.getRawType().isAssignableTo(SORT_MAPPINGS_CLASS)) + .toList(); - if (mappings.fieldToKeyNames().isEmpty()) { + // A @Store that does not sort is not a violation, there is simply nothing to check. + if (sortMappingsFields.isEmpty()) { return; } - validateSortMappings(javaClass, events, mappings); - } - - private static DetectedSortField detectSortMappings(JavaClass javaClass) { - // Read SortMappings static fields, resolve their enum type, and collect key names - var fieldToKeyNames = new HashMap>(); - var fieldToEnumClassName = new HashMap(); + Class runtimeClass; try { - var runtimeClass = Class.forName(javaClass.getFullName()); - for (var field : javaClass.getFields()) { - if (field.getRawType().isAssignableTo( - "it.aboutbits.springboot.toolbox.persistence.SortMappings")) { - String enumClassName = null; - - // Try to resolve enum type from the field's generic type parameter - var fieldType = field.getType(); - if (fieldType instanceof JavaParameterizedType pt && !pt.getActualTypeArguments() - .isEmpty()) { - enumClassName = pt.getActualTypeArguments() - .getFirst() - .toErasure() - .getFullName(); - } - - try { - var reflectField = runtimeClass.getDeclaredField(field.getName()); - reflectField.setAccessible(true); - var value = reflectField.get(null); - if (value instanceof Map m) { - var keys = m.keySet(); - var keyNames = keys.stream() - .filter(k -> k instanceof Enum) - .map(k -> ((Enum) k).name()) - .collect(Collectors.toSet()); - fieldToKeyNames.put(field.getName(), keyNames); - - // If generic info is missing, infer enum type from the first key - if (enumClassName == null) { - var anyKey = keys.stream() - .filter(k -> k instanceof Enum) - .findFirst(); - if (anyKey.isPresent()) { - enumClassName = ((Enum) anyKey.get()).getDeclaringClass() - .getName(); - } - } - } - } catch (Exception _) { - // ignore fields we cannot read (non-static or other issues) - log.warn( - "Failed to read SortMappings field {} in {} ({}.java:{})", - field.getName(), - javaClass.getFullName(), - javaClass.getSimpleName(), - getLineNumber(field) - ); - } - - if (enumClassName != null) { - fieldToEnumClassName.put(field.getName(), enumClassName); - } - } - } - } catch (ClassNotFoundException _) { - // ignore - log.warn( - "Failed to resolve enum type for SortMappings field in {} ({}.java:{})", - javaClass.getFullName(), - javaClass.getSimpleName(), - getLineNumber(javaClass) - ); + runtimeClass = Class.forName(javaClass.getFullName()); + } catch (ClassNotFoundException | LinkageError _) { + violated(events, javaClass, javaClass, + "cannot be loaded by the architecture test, so its SortMappings fields cannot be validated"); + return; + } + + for (var field : sortMappingsFields) { + checkField(javaClass, runtimeClass, field, events); } - return new DetectedSortField(fieldToKeyNames, fieldToEnumClassName); } - private static void validateSortMappings( + private void checkField( JavaClass javaClass, - ConditionEvents events, - DetectedSortField mappings + Class runtimeClass, + JavaField field, + ConditionEvents events ) { - // Validate each SortMappings field against its associated enum - for (var entry : mappings.fieldToKeyNames().entrySet()) { - var fieldName = entry.getKey(); - var keyNames = entry.getValue(); - var enumClassName = mappings.fieldToEnumClassName().get(fieldName); - - if (enumClassName == null) { - // Cannot determine enum type for this field; skip validation - log.warn( - "Cannot determine enum type for SortMappings field {} in {} ({}.java:{})", - fieldName, - javaClass.getFullName(), - javaClass.getSimpleName(), - getLineNumber(javaClass) - ); - continue; - } + if (!field.getModifiers().contains(JavaModifier.STATIC)) { + violated(events, javaClass, field, + "SortMappings field %s must be static, otherwise its mappings cannot be read and validated" + .formatted(field.getName())); + return; + } - try { - var enumClass = Class.forName(enumClassName); - if (enumClass.isEnum()) { - var enumConstants = (Enum[]) enumClass.getEnumConstants(); - var missing = Stream.of(enumConstants) - .map(Enum::name) - .filter(name -> !keyNames.contains(name)) - .toList(); - if (!missing.isEmpty()) { - // Try to get a precise field line number for the message - var fieldLine = javaClass.getFields().stream() - .filter(f -> f.getName().equals(fieldName)) - .filter(f -> f.getRawType() - .isAssignableTo("it.aboutbits.springboot.toolbox.persistence.SortMappings")) - .findFirst() - .map(LineNumberUtil::getLineNumber) - .orElse(-1); - - var message = String.format( - "Class %s: SortMappings field %s is missing mappings for enum %s values %s (%s.java:%d)", - javaClass.getFullName(), - fieldName, - enumClass.getSimpleName(), - missing, - javaClass.getSimpleName(), - fieldLine - ); - events.add(SimpleConditionEvent.violated(javaClass, message)); - } - } - } catch (ClassNotFoundException _) { - // ignore - log.warn( - "Failed to resolve enum type for SortMappings field {} in {} ({}.java:{})", - fieldName, - javaClass.getFullName(), - javaClass.getSimpleName(), - getLineNumber(javaClass) - ); + Map mappings; + try { + var reflectField = runtimeClass.getDeclaredField(field.getName()); + reflectField.setAccessible(true); + var value = reflectField.get(null); + if (!(value instanceof Map readMappings)) { + violated(events, javaClass, field, + "SortMappings field %s did not yield a Map (got %s), so its mappings cannot be validated" + .formatted(field.getName(), value == null ? "null" : value.getClass().getName())); + return; } + mappings = readMappings; + } catch (ReflectiveOperationException | RuntimeException | LinkageError e) { + violated(events, javaClass, field, + "SortMappings field %s could not be read (%s: %s), so its mappings cannot be validated" + .formatted(field.getName(), e.getClass().getSimpleName(), e.getMessage())); + return; } + + var enumClassName = resolveEnumClassName(field, mappings); + if (enumClassName.isEmpty()) { + violated(events, javaClass, field, + "the Sort enum type of SortMappings field %s cannot be determined, so its mappings cannot be validated" + .formatted(field.getName())); + return; + } + + Class enumClass; + try { + enumClass = Class.forName(enumClassName.get()); + } catch (ClassNotFoundException | LinkageError _) { + violated(events, javaClass, field, + "the Sort enum %s of SortMappings field %s cannot be loaded, so its mappings cannot be validated" + .formatted(enumClassName.get(), field.getName())); + return; + } + + if (!enumClass.isEnum()) { + violated(events, javaClass, field, + "the key type %s of SortMappings field %s is not an enum, so its mappings cannot be validated" + .formatted(enumClass.getName(), field.getName())); + return; + } + + var mappedNames = enumNames(mappings.keySet()); + var missing = Stream.of((Enum[]) enumClass.getEnumConstants()) + .map(Enum::name) + .filter(name -> !mappedNames.contains(name)) + .toList(); + + if (!missing.isEmpty()) { + violated(events, javaClass, field, + "SortMappings field %s is missing mappings for enum %s values %s" + .formatted(field.getName(), enumClass.getSimpleName(), missing)); + } + } + + private static Optional resolveEnumClassName(JavaField field, Map mappings) { + // Prefer the declared generic type parameter, e.g. SortMappings + if (field.getType() instanceof JavaParameterizedType parameterizedType + && !parameterizedType.getActualTypeArguments().isEmpty()) { + return Optional.of(parameterizedType.getActualTypeArguments() + .getFirst() + .toErasure() + .getFullName()); + } + + // Fall back to the runtime type of any mapped key + return mappings.keySet() + .stream() + .filter(Enum.class::isInstance) + .map(key -> ((Enum) key).getDeclaringClass().getName()) + .findFirst(); + } + + private static Set enumNames(Set keys) { + return keys.stream() + .filter(Enum.class::isInstance) + .map(key -> ((Enum) key).name()) + .collect(Collectors.toSet()); } - private record DetectedSortField( - HashMap> fieldToKeyNames, - HashMap fieldToEnumClassName + private static void violated( + ConditionEvents events, + JavaClass javaClass, + Object violatingElement, + String detail ) { + var lineNumber = violatingElement instanceof JavaField field + ? getLineNumber(field) + : getLineNumber(javaClass); + + events.add(SimpleConditionEvent.violated( + violatingElement, + "Class %s: %s (%s.java:%d)".formatted( + javaClass.getFullName(), + detail, + javaClass.getSimpleName(), + lineNumber + ) + )); } } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupName.java b/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupName.java index 59aa60f..40bcc8e 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupName.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupName.java @@ -1,20 +1,20 @@ package it.aboutbits.archunit.toolbox.support; -import com.tngtech.archunit.junit.ArchIgnore; - import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** - * Use this annotation to ignore a group of tests in the architecture check. + * Marks a @Nested test class that only groups tests logically and therefore has no matching + * nested class in the production code. *

- * This annotation should be used on a @Nested test class. + * Must not be meta-annotated with ArchUnit's @ArchIgnore: the ArchUnit JUnit engine resolves + * meta-annotations, so that would skip every @ArchTest on the annotated class instead of + * exempting it from a single rule. The rules read this annotation by its own type. *

*/ @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) -@ArchIgnore(reason = "This is a @Nested test class to logically group tests with no matching production code nested class.") public @interface ArchIgnoreGroupName { } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpart.java b/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpart.java index 41a64ce..d5e7f1a 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpart.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpart.java @@ -1,20 +1,20 @@ package it.aboutbits.archunit.toolbox.support; -import com.tngtech.archunit.junit.ArchIgnore; - import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** - * Use this annotation to ignore a group of tests in the architecture check. + * Marks a test class that has no matching counterpart in the production code, for example a + * scenario test named after the behaviour it describes. *

- * This annotation should be used on a @Nested test class. + * Must not be meta-annotated with ArchUnit's @ArchIgnore: the ArchUnit JUnit engine resolves + * meta-annotations, so that would skip every @ArchTest on the annotated class instead of + * exempting it from a single rule. The rules read this annotation by its own type. *

*/ @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) -@ArchIgnore(reason = "This test class has no matching counterpart in the production code.") public @interface ArchIgnoreNoProductionCounterpart { } diff --git a/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java b/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java index 5acf9c7..572ae85 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java @@ -2,14 +2,24 @@ import com.tngtech.archunit.junit.AnalyzeClasses; import com.tngtech.archunit.junit.CacheMode; +import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; import org.jspecify.annotations.NullMarked; +/** + * The toolbox checked against its own base rules. + *

+ * Carries @ArchIgnoreNoProductionCounterpart because there is no production class named + * "Architecture". That the 11 rules below still run is also what pins that the annotation exempts a + * class from one rule rather than skipping every @ArchTest on it. + *

+ */ @SuppressWarnings("checkstyle:HideUtilityClassConstructor") @AnalyzeClasses( packages = ArchitectureTest.PACKAGE, cacheMode = CacheMode.PER_CLASS ) @NullMarked +@ArchIgnoreNoProductionCounterpart class ArchitectureTest implements BaseArchRuleCollection { static final String PACKAGE = "it.aboutbits.archunit.toolbox"; } diff --git a/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java b/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java new file mode 100644 index 0000000..8252e82 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java @@ -0,0 +1,75 @@ +package it.aboutbits.archunit.toolbox; + +import com.tngtech.archunit.core.domain.JavaClasses; +import it.aboutbits.archunit.toolbox.rule.base.RecordPropertiesMustBeAccessedViaAccessorArchRule; +import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.function.Consumer; +import java.util.stream.Stream; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +/** + * No rule may use {@code allowEmptyShould(true)}. + *

+ * A rule that selects nothing reports success, which is indistinguishable from a rule that is + * satisfied. That is how a broken rule survives unnoticed, so every rule that narrows its input must + * fail when the selection comes up empty. + *

+ */ +@NullMarked +@ArchIgnoreNoProductionCounterpart +class EmptySelectionTest { + private static final Rules RULES = new Rules(); + + static Stream rulesThatNarrowTheirInput() { + return Stream.of( + arguments("test classes are in the same package as their production code", + consumer(RULES::test_classes_should_be_in_the_same_package_as_their_production_code)), + arguments("test classes must be package private", + consumer(RULES::test_classes_must_be_package_private)), + arguments("test methods must be package private", + consumer(RULES::test_methods_must_be_package_private)), + arguments("nested test classes must be package private", + consumer(RULES::nested_test_classes_must_be_package_private)), + arguments("nested test classes match a production method name", + consumer(RULES::nested_test_classes_have_matching_production_method_name)), + arguments("record properties are accessed via accessor", + consumer(RecordPropertiesMustBeAccessedViaAccessorArchRule + .record_properties_must_be_accessed_via_accessor::check)), + arguments("controller request mappings must be security tested", + consumer(RULES::controller_methods_with_request_mapping_must_be_security_tested)), + arguments("sort mappings cover all sort enum values", + consumer(RULES::sort_mappings_cover_all_sort_enum_values)) + ); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("rulesThatNarrowTheirInput") + void a_rule_that_selects_nothing_fails_instead_of_reporting_success( + String ruleDescription, + Consumer rule + ) { + var barrenCodebase = fixture("barren"); + + var failure = violationOf(() -> rule.accept(barrenCodebase)); + + assertThat(failure) + .as("%s must not pass on a codebase it selects nothing from", ruleDescription) + .hasMessageContaining("failed to check any"); + } + + private static Consumer consumer(Consumer rule) { + return rule; + } + + private static final class Rules implements BaseArchRuleCollection, CommonArchRuleCollection { + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupNameTest.java b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupNameTest.java new file mode 100644 index 0000000..39e8b9b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupNameTest.java @@ -0,0 +1,18 @@ +package it.aboutbits.archunit.toolbox.support; + +import com.tngtech.archunit.junit.ArchIgnore; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class ArchIgnoreGroupNameTest { + /** See ArchIgnoreNoProductionCounterpartTest: a meta @ArchIgnore skips arch tests wholesale. */ + @Test + void the_annotation_is_not_meta_annotated_with_arch_ignore() { + assertThat(ArchIgnoreGroupName.class.isAnnotationPresent(ArchIgnore.class)) + .as("@ArchIgnore here would silently disable every arch rule on the annotated class") + .isFalse(); + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java new file mode 100644 index 0000000..561d94b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java @@ -0,0 +1,23 @@ +package it.aboutbits.archunit.toolbox.support; + +import com.tngtech.archunit.junit.ArchIgnore; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +@NullMarked +class ArchIgnoreNoProductionCounterpartTest { + /** + * ArchUnit's JUnit engine resolves meta-annotations, so meta-annotating this with @ArchIgnore + * makes it skip every @ArchTest on the annotated class - reported as success - rather than + * exempting the class from one rule. The rules read this annotation by its own type, so the + * meta-annotation buys nothing and costs all of them. + */ + @Test + void the_annotation_is_not_meta_annotated_with_arch_ignore() { + assertThat(ArchIgnoreNoProductionCounterpart.class.isAnnotationPresent(ArchIgnore.class)) + .as("@ArchIgnore here would silently disable every arch rule on the annotated class") + .isFalse(); + } +} From d52ba67731ac00f0b899b1b3f3d1287e60cbd7a2 Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Wed, 19 Aug 2026 12:31:40 +0200 Subject: [PATCH 4/9] 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 --- readme.md | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/readme.md b/readme.md index b0d1175..3737eb6 100644 --- a/readme.md +++ b/readme.md @@ -18,8 +18,7 @@ Add this library to the classpath by adding the following maven dependency. Vers ## 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 @@ -27,23 +26,48 @@ For example `ArchitectureTestBase`: 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 - } } ``` -In the static block you can configure some blacklists provided by the base class. +`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 { - ArchitectureTestBase.BLACKLISTED_CLASSES.remove("net.datafaker.Faker"); + +static { + BlacklistClassesArchRule.BLACKLISTED_CLASSES.remove("net.datafaker.Faker"); } ``` +The same applies to `ArchRuleConfig.TEST_CLASS_SUFFIXES` when a project introduces a new test type. + +### Every rule must be able to fail + +No rule uses `allowEmptyShould(true)`. A rule that selects nothing would otherwise report success, +which is indistinguishable from a rule that is satisfied - and that is how a broken rule survives +unnoticed. So a rule whose selection comes up empty fails, and the fix is either to remove the rule +collection you do not need or to add the code it is meant to check. + +### 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, and on your own `ArchitectureTest`. + ## Local Development To use this library as a local development dependency, you can simply refer to the version `BUILD-SNAPSHOT`. From e9af6dacc2e71101b0d78adee0a293fb971772c0 Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Wed, 19 Aug 2026 16:42:23 +0200 Subject: [PATCH 5/9] 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 --- readme.md | 17 ++++++--- .../toolbox/BaseArchRuleCollection.java | 2 ++ .../toolbox/CommonArchRuleCollection.java | 2 ++ ...zedPackagesMustContainClassesArchRule.java | 30 ++++++++++++++++ .../base/BlacklistAnnotationsArchRule.java | 1 + .../rule/base/BlacklistClassesArchRule.java | 1 + .../rule/base/BlacklistMethodsArchRule.java | 1 + .../rule/base/EnforceJspecifyArchRule.java | 1 + .../rule/base/NoSystemOutOrErrArchRule.java | 1 + ...tiesMustBeAccessedViaAccessorArchRule.java | 3 +- .../TestClassInCorrectPackageArchRule.java | 1 + .../base/TestClassVisibilityArchRule.java | 1 + .../base/TestMethodVisibilityArchRule.java | 1 + .../TestNestedClassMatchNameArchRule.java | 1 + .../TestNestedClassVisibilityArchRule.java | 1 + ...erRequestMappingsMustBeSecurityTested.java | 1 + .../SortMappingsExhaustiveArchRule.java | 1 + .../archunit/toolbox/EmptySelectionTest.java | 32 ++++++++++------- .../archunit/toolbox/RuleEvaluation.java | 11 ++++++ ...ackagesMustContainClassesArchRuleTest.java | 35 +++++++++++++++++++ 20 files changed, 125 insertions(+), 19 deletions(-) create mode 100644 src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java create mode 100644 src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java diff --git a/readme.md b/readme.md index 3737eb6..ee63357 100644 --- a/readme.md +++ b/readme.md @@ -47,12 +47,19 @@ static { The same applies to `ArchRuleConfig.TEST_CLASS_SUFFIXES` when a project introduces a new test type. -### Every rule must be able to fail +### Rules your project has no code for -No rule uses `allowEmptyShould(true)`. A rule that selects nothing would otherwise report success, -which is indistinguishable from a rule that is satisfied - and that is how a broken rule survives -unnoticed. So a rule whose selection comes up empty fails, and the fix is either to remove the rule -collection you do not need or to add the code it is meant to check. +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 diff --git a/src/main/java/it/aboutbits/archunit/toolbox/BaseArchRuleCollection.java b/src/main/java/it/aboutbits/archunit/toolbox/BaseArchRuleCollection.java index 8d9c61a..e85c9f2 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/BaseArchRuleCollection.java +++ b/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, diff --git a/src/main/java/it/aboutbits/archunit/toolbox/CommonArchRuleCollection.java b/src/main/java/it/aboutbits/archunit/toolbox/CommonArchRuleCollection.java index fd8e858..21a721a 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/CommonArchRuleCollection.java +++ b/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 { } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java new file mode 100644 index 0000000..c53a8dc --- /dev/null +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java @@ -0,0 +1,30 @@ +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." + ); + } + } +} diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRule.java index f3e5cb1..32b0453 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRule.java @@ -64,6 +64,7 @@ public interface BlacklistAnnotationsArchRule { default void no_blacklisted_annotations_are_used(JavaClasses classes) { classes() .should(new NotUseBlacklistedAnnotations()) + .allowEmptyShould(true) .check(classes); } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRule.java index 2e77443..4b4aeb7 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistClassesArchRule.java @@ -35,6 +35,7 @@ public boolean test(JavaClass javaClass) { } } ) + .allowEmptyShould(true) .check(classes); } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java index 90d5492..74d65a0 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java @@ -68,6 +68,7 @@ public interface BlacklistMethodsArchRule { default void no_blacklisted_methods_are_used(JavaClasses classes) { classes() .should(new NotUseBlacklistedMethods()) + .allowEmptyShould(true) .check(classes); } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/EnforceJspecifyArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/EnforceJspecifyArchRule.java index 7cfbe3c..1dc0bdb 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/EnforceJspecifyArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/EnforceJspecifyArchRule.java @@ -21,6 +21,7 @@ default void top_level_classes_must_be_annotated_with_jspecify(JavaClasses class .beAnnotatedWith(org.jspecify.annotations.NullMarked.class) .orShould() .beAnnotatedWith(org.jspecify.annotations.NullUnmarked.class) + .allowEmptyShould(true) .check(classes); } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRule.java index f290997..0c3766b 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/NoSystemOutOrErrArchRule.java @@ -20,6 +20,7 @@ public interface NoSystemOutOrErrArchRule { default void no_system_out_or_err_is_used(JavaClasses classes) { classes() .should(new NotUseSystemOutOrErr()) + .allowEmptyShould(true) .check(classes); } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRule.java index b7e56e3..970baa8 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRule.java @@ -45,5 +45,6 @@ public void check(JavaField field, ConditionEvents events) { } } } - }); + }) + .allowEmptyShould(true); } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java index dba6c1d..3ea805e 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java @@ -27,6 +27,7 @@ default void test_classes_should_be_in_the_same_package_as_their_production_code .and() .resideOutsideOfPackages(".._support..", ".._config..") .should(new BeInTheSamePackageAsTheProductionClass(classes)) + .allowEmptyShould(true) .check(classes); } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java index 88434d0..0a2d72a 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java @@ -19,6 +19,7 @@ default void test_classes_must_be_package_private(JavaClasses classes) { .resideOutsideOfPackages(".._support..", ".._config..") .should() .bePackagePrivate() + .allowEmptyShould(true) .check(classes); } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRule.java index 81d76dd..5b16356 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestMethodVisibilityArchRule.java @@ -32,6 +32,7 @@ public boolean test(JavaClass javaClass) { }) .should() .bePackagePrivate() + .allowEmptyShould(true) .check(classes); } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java index 0d31c94..1029adc 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java @@ -33,6 +33,7 @@ default void nested_test_classes_have_matching_production_method_name(JavaClasse */ .areNotAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart.class) .should(new HaveNestedClassesThatHaveAMatchingProductionMethodName(classes)) + .allowEmptyShould(true) .check(classes); } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRule.java index 3558a85..dbac531 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassVisibilityArchRule.java @@ -17,6 +17,7 @@ default void nested_test_classes_must_be_package_private(JavaClasses classes) { .areAnnotatedWith(org.junit.jupiter.api.Nested.class) .should() .bePackagePrivate() + .allowEmptyShould(true) .check(classes); } } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java index ad1a429..09b6b2a 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java @@ -44,6 +44,7 @@ public boolean test(JavaAnnotation javaAnnotation) { } }) .should(new BeSecurityTested()) + .allowEmptyShould(true) .check(classes); } diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java index bb5452b..8335cb3 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java @@ -31,6 +31,7 @@ default void sort_mappings_cover_all_sort_enum_values(JavaClasses classes) { .that() .areAnnotatedWith("it.aboutbits.springboot.toolbox.stereotype.Store") .should(new HaveExhaustiveSortMappingsIfPresent()) + .allowEmptyShould(true) .check(classes); } diff --git a/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java b/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java index 8252e82..d79b98e 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java @@ -12,16 +12,21 @@ import java.util.stream.Stream; import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; -import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.junit.jupiter.params.provider.Arguments.arguments; /** - * No rule may use {@code allowEmptyShould(true)}. + * A rule must not complain about code the project does not have. *

- * A rule that selects nothing reports success, which is indistinguishable from a rule that is - * satisfied. That is how a broken rule survives unnoticed, so every rule that narrows its input must - * fail when the selection comes up empty. + * Whether a project contains records, controllers, @Store classes or @Nested test classes is the + * project's business, so every rule tolerates a selection that comes up empty. That each rule can + * still fail is guaranteed by its own red test in this project, not by making consumers fail - an + * empty selection says nothing about whether a rule's logic works. The counterpart rule proved that: + * its selection was never empty, its condition was simply broken. + *

+ *

+ * The one genuinely dangerous case, nothing imported at all, is covered by + * AnalyzedPackagesMustContainClassesArchRule. *

*/ @NullMarked @@ -47,23 +52,24 @@ static Stream rulesThatNarrowTheirInput() { arguments("controller request mappings must be security tested", consumer(RULES::controller_methods_with_request_mapping_must_be_security_tested)), arguments("sort mappings cover all sort enum values", - consumer(RULES::sort_mappings_cover_all_sort_enum_values)) + consumer(RULES::sort_mappings_cover_all_sort_enum_values)), + arguments("top level classes must be annotated with jspecify", + consumer(RULES::top_level_classes_must_be_annotated_with_jspecify)) ); } @ParameterizedTest(name = "{0}") @MethodSource("rulesThatNarrowTheirInput") - void a_rule_that_selects_nothing_fails_instead_of_reporting_success( + void a_rule_accepts_a_project_that_has_no_code_it_applies_to( String ruleDescription, Consumer rule ) { + // One plain class: no test classes, no records, no controllers, no stores var barrenCodebase = fixture("barren"); - var failure = violationOf(() -> rule.accept(barrenCodebase)); - - assertThat(failure) - .as("%s must not pass on a codebase it selects nothing from", ruleDescription) - .hasMessageContaining("failed to check any"); + assertThatCode(() -> rule.accept(barrenCodebase)) + .as("%s must not fail a project that has no code it applies to", ruleDescription) + .doesNotThrowAnyException(); } private static Consumer consumer(Consumer rule) { diff --git a/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java b/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java index 99f2239..aa09aae 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java @@ -36,6 +36,17 @@ public static JavaClasses fixture(String subPackage) { return classes; } + /** An import that yielded nothing, as a consumer with a mistyped @AnalyzeClasses package gets. */ + public static JavaClasses noClassesImported() { + var classes = new ClassFileImporter().importPackages(FIXTURE_ROOT + "nosuchpackage"); + + if (!classes.isEmpty()) { + throw new IllegalStateException("Expected an empty import but got " + classes.size() + " classes"); + } + + return classes; + } + /** * Runs a rule that is expected to report at least one violation and returns the failure. * Failing here means the rule accepted a fixture that was built to violate it. diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java new file mode 100644 index 0000000..a1c04b1 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java @@ -0,0 +1,35 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +import static it.aboutbits.archunit.toolbox.RuleEvaluation.fixture; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.noClassesImported; +import static it.aboutbits.archunit.toolbox.RuleEvaluation.violationOf; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +@NullMarked +class AnalyzedPackagesMustContainClassesArchRuleTest implements AnalyzedPackagesMustContainClassesArchRule { + /** + * The one case every other rule deliberately tolerates: nothing was imported, so nothing can be + * checked and every rule would otherwise pass. + */ + @Test + void an_import_without_any_classes_is_reported() { + var classes = noClassesImported(); + + var failure = violationOf(() -> analyzed_packages_must_contain_classes(classes)); + + assertThat(failure) + .hasMessageContaining("No classes were imported") + .hasMessageContaining("@AnalyzeClasses"); + } + + @Test + void an_import_with_classes_is_accepted() { + var classes = fixture("barren"); + + assertThatCode(() -> analyzed_packages_must_contain_classes(classes)).doesNotThrowAnyException(); + } +} From 383cf46f91a2b15fc1b43a12e146471bc90b1221 Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Wed, 19 Aug 2026 17:08:45 +0200 Subject: [PATCH 6/9] 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

, and backticked annotation names for the @ 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 --- .../toolbox/config/ArchRuleConfig.java | 10 +-- ...zedPackagesMustContainClassesArchRule.java | 17 ++--- ...erRequestMappingsMustBeSecurityTested.java | 8 +- .../SortMappingsExhaustiveArchRule.java | 13 ++-- .../toolbox/support/ArchIgnoreGroupName.java | 15 ++-- .../ArchIgnoreNoProductionCounterpart.java | 15 ++-- .../archunit/toolbox/util/CodeUnitUtil.java | 13 ++-- .../archunit/toolbox/util/TestClassNames.java | 75 +++++++------------ .../archunit/fixture/barren/PlainClass.java | 6 +- .../AnnotatedConstructorParameter.java | 2 +- .../CallsFromFieldInitializer.java | 2 +- .../nestedclassname/badgroup/WidgetTest.java | 2 +- .../nestedclassname/badmethod/WidgetTest.java | 2 +- .../badnoproduction/WidgetTest.java | 2 +- .../goodoptout/ScenarioTest.java | 6 +- .../goodoptout/WidgetTest.java | 2 +- .../EnclosingReadsNestedRecordField.java | 2 +- .../WidgetControllerSecurityTest.java | 2 +- .../WidgetControllerSecurityTest.java | 6 +- .../WidgetControllerSecurityTest.java | 6 +- .../fixture/sortmappings/bad/WidgetStore.java | 2 +- .../badnonstatic/WidgetStore.java | 2 +- .../testclasspackage/bad/WidgetTest.java | 2 +- .../goodoptout/ScenarioTest.java | 2 +- .../goodoptout/WidgetTest.java | 2 +- .../archunit/toolbox/ArchitectureTest.java | 13 ++-- .../archunit/toolbox/EmptySelectionTest.java | 24 +++--- .../archunit/toolbox/RuleEvaluation.java | 27 +++---- ...ackagesMustContainClassesArchRuleTest.java | 6 +- .../BlacklistAnnotationsArchRuleTest.java | 6 +- .../base/BlacklistMethodsArchRuleTest.java | 8 +- ...MustBeAccessedViaAccessorArchRuleTest.java | 6 +- ...questMappingsMustBeSecurityTestedTest.java | 6 +- .../SortMappingsExhaustiveArchRuleTest.java | 6 +- .../support/ArchIgnoreGroupNameTest.java | 2 +- ...ArchIgnoreNoProductionCounterpartTest.java | 10 +-- .../toolbox/util/TestClassNamesTest.java | 20 ++--- .../archunit/ArchAllowDirectAccess.java | 6 +- .../toolbox/persistence/SortMappings.java | 6 +- .../springboot/toolbox/stereotype/Store.java | 6 +- src/test/java/net/datafaker/Faker.java | 2 +- src/test/java/org/junit/Ignore.java | 6 +- 42 files changed, 145 insertions(+), 231 deletions(-) diff --git a/src/main/java/it/aboutbits/archunit/toolbox/config/ArchRuleConfig.java b/src/main/java/it/aboutbits/archunit/toolbox/config/ArchRuleConfig.java index 136346b..3af3fe5 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/config/ArchRuleConfig.java +++ b/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. - *

- * 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 TEST_CLASS_SUFFIXES = new HashSet<>( Set.of( "Test", diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java index c53a8dc..0bd6f89 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java @@ -4,16 +4,13 @@ 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. - *

- */ +/// 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 { diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java index 09b6b2a..f3a745e 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java @@ -109,11 +109,9 @@ public void check(JavaMethod method, ConditionEvents events) { } } - /* - * The @Nested class named after the controller method, or a @Nested class grouped inside it - * (GetAll$WhenAdmin). Matching on a bare prefix would also accept an unrelated longer - * sibling, so getAll() would count as covered by a @Nested class named GetAllArchived. - */ + /// The `@Nested` class named after the controller method, or a `@Nested` class grouped inside it + /// (GetAll$WhenAdmin). Matching on a bare prefix would also accept an unrelated longer + /// sibling, so getAll() would count as covered by a `@Nested` class named GetAllArchived. private static boolean isExpectedNestedClass(String candidateName, String expectedName) { return candidateName.equals(expectedName) || candidateName.startsWith(expectedName + "$"); diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java index 8335cb3..c0d42eb 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRule.java @@ -35,14 +35,11 @@ default void sort_mappings_cover_all_sort_enum_values(JavaClasses classes) { .check(classes); } - /** - * Checks that every value of a Sort enum has a mapping. - *

- * Every case this cannot verify is reported as a violation rather than skipped. Reading a - * mapping requires reflection, and a mapping that cannot be read is indistinguishable from one - * that is exhaustive - so silence here means the rule quietly stops covering that field. - *

- */ + /// Checks that every value of a Sort enum has a mapping. + /// + /// Every case this cannot verify is reported as a violation rather than skipped. Reading a + /// mapping requires reflection, and a mapping that cannot be read is indistinguishable from one + /// that is exhaustive - so silence here means the rule quietly stops covering that field. class HaveExhaustiveSortMappingsIfPresent extends ArchCondition { public HaveExhaustiveSortMappingsIfPresent() { super("have SortMappings that map all values of the associated Sort enum"); diff --git a/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupName.java b/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupName.java index 40bcc8e..046d4fa 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupName.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupName.java @@ -5,15 +5,12 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -/** - * Marks a @Nested test class that only groups tests logically and therefore has no matching - * nested class in the production code. - *

- * Must not be meta-annotated with ArchUnit's @ArchIgnore: the ArchUnit JUnit engine resolves - * meta-annotations, so that would skip every @ArchTest on the annotated class instead of - * exempting it from a single rule. The rules read this annotation by its own type. - *

- */ +/// Marks a `@Nested` test class that only groups tests logically and therefore has no matching +/// nested class in the production code. +/// +/// Must not be meta-annotated with ArchUnit's `@ArchIgnore`: the ArchUnit JUnit engine resolves +/// meta-annotations, so that would skip every `@ArchTest` on the annotated class instead of +/// exempting it from a single rule. The rules read this annotation by its own type. @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) public @interface ArchIgnoreGroupName { diff --git a/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpart.java b/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpart.java index d5e7f1a..bcbb748 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpart.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpart.java @@ -5,15 +5,12 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -/** - * Marks a test class that has no matching counterpart in the production code, for example a - * scenario test named after the behaviour it describes. - *

- * Must not be meta-annotated with ArchUnit's @ArchIgnore: the ArchUnit JUnit engine resolves - * meta-annotations, so that would skip every @ArchTest on the annotated class instead of - * exempting it from a single rule. The rules read this annotation by its own type. - *

- */ +/// Marks a test class that has no matching counterpart in the production code, for example a +/// scenario test named after the behaviour it describes. +/// +/// Must not be meta-annotated with ArchUnit's `@ArchIgnore`: the ArchUnit JUnit engine resolves +/// meta-annotations, so that would skip every `@ArchTest` on the annotated class instead of +/// exempting it from a single rule. The rules read this annotation by its own type. @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) public @interface ArchIgnoreNoProductionCounterpart { diff --git a/src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java b/src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java index 4302256..44e08af 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java @@ -11,14 +11,11 @@ public final class CodeUnitUtil { private CodeUnitUtil() { } - /** - * Human readable kind of a code unit, for violation messages. - *

- * Rules that inspect bodies must iterate {@code getCodeUnits()} rather than {@code getMethods()}: - * the latter excludes constructors, and an instance field initializer is compiled into the - * constructor, so both are invisible to a rule that only looks at methods. - *

- */ + /// Human readable kind of a code unit, for violation messages. + /// + /// Rules that inspect bodies must iterate `getCodeUnits()` rather than `getMethods()`: + /// the latter excludes constructors, and an instance field initializer is compiled into the + /// constructor, so both are invisible to a rule that only looks at methods. public static String describeKind(JavaCodeUnit codeUnit) { return switch (codeUnit) { case JavaMethod _ -> "Method"; diff --git a/src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java b/src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java index 8cfa4f9..597316c 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java @@ -9,69 +9,54 @@ import static it.aboutbits.archunit.toolbox.config.ArchRuleConfig.TEST_CLASS_SUFFIXES; -/** - * Single source of truth for recognising a test class by its name suffix. - *

- * Every rule that selects test classes must go through {@link #testClasses()}, and every rule that - * derives a production class name must go through {@link #productionClassSimpleName(String)}. - * Hand-building the suffix regex per rule is what allowed a condition to be anchored differently - * from the selection that fed it, silently disabling the rule. - *

- */ +/// Single source of truth for recognising a test class by its name suffix. +/// +/// Every rule that selects test classes must go through [#testClasses()], and every rule that +/// derives a production class name must go through [#productionClassSimpleName(String)]. +/// Hand-building the suffix regex per rule is what allowed a condition to be anchored differently +/// from the selection that fed it, silently disabling the rule. @NullMarked public final class TestClassNames { private TestClassNames() { } - /** - * Matches the name of a test class: at least one character, then one of the configured suffixes. - *

- * The leading {@code .+} is load-bearing. {@link String#matches(String)} and ArchUnit's - * {@code haveNameMatching} both anchor at each end, so without it the pattern only matches a - * class named exactly {@code Test}. It is also what keeps such a class out of the selection, - * since it has no name left once the suffix is stripped. - *

- */ + /// Matches the name of a test class: at least one character, then one of the configured suffixes. + /// + /// The leading `.+` is load-bearing. [String#matches(String)] and ArchUnit's + /// `haveNameMatching` both anchor at each end, so without it the pattern only matches a + /// class named exactly `Test`. It is also what keeps such a class out of the selection, + /// since it has no name left once the suffix is stripped. public static String testClassNameRegex() { return ".+(" + suffixAlternation() + ")$"; } - /** - * Matches only the trailing suffix, for stripping it off a test class name. Deliberately not - * anchored at the start - this is used with {@link String#replaceAll(String, String)}, never - * with {@link String#matches(String)}. - */ + /// Matches only the trailing suffix, for stripping it off a test class name. Deliberately not + /// anchored at the start - this is used with [String#replaceAll(String, String)], never + /// with [String#matches(String)]. public static String suffixRegex() { return "(" + suffixAlternation() + ")$"; } - /** - * Whether a simple name is the name of a test class. - *

- * Requires a production class name to be left over once the suffix is stripped, so that the - * selection cannot disagree with {@link #productionClassSimpleName(String)}. "CacheTest" matches - * the pattern with "Cache" as the leading {@code .+}, yet stripping removes "CacheTest" whole and - * leaves nothing to look for. - *

- */ + /// Whether a simple name is the name of a test class. + /// + /// Requires a production class name to be left over once the suffix is stripped, so that the + /// selection cannot disagree with [#productionClassSimpleName(String)]. "CacheTest" matches + /// the pattern with "Cache" as the leading `.+`, yet stripping removes "CacheTest" whole and + /// leaves nothing to look for. public static boolean isTestClassName(String simpleName) { return simpleName.matches(testClassNameRegex()) && !productionClassSimpleName(simpleName).isEmpty(); } - /** - * The simple name of the production class a test class belongs to, e.g. {@code WidgetCacheTest} - * to {@code Widget}. - */ + /// The simple name of the production class a test class belongs to, e.g. `WidgetCacheTest` + /// to `Widget`. public static String productionClassSimpleName(String testClassSimpleName) { return testClassSimpleName.replaceAll(suffixRegex(), ""); } - /** - * Selects test classes by their simple name. Matching the simple name rather than the - * fully qualified name keeps the selection and the conditions that follow it in agreement: - * {@code some.pkg.Test} matches the fully qualified name but is not a test class. - */ + /// Selects test classes by their *simple* name. Matching the simple name rather than the + /// fully qualified name keeps the selection and the conditions that follow it in agreement: + /// `some.pkg.Test` matches the fully qualified name but is not a test class. public static DescribedPredicate testClasses() { return new DescribedPredicate<>("have a simple name matching '%s'".formatted(testClassNameRegex())) { @Override @@ -81,11 +66,9 @@ public boolean test(JavaClass javaClass) { }; } - /* - * Longest suffix first, then alphabetically. TEST_CLASS_SUFFIXES is a mutable HashSet, so - * without an explicit order the generated regex - and every rule description built from it - - * varies between JVM runs. - */ + /// Longest suffix first, then alphabetically. TEST_CLASS_SUFFIXES is a mutable HashSet, so + /// without an explicit order the generated regex - and every rule description built from it - + /// varies between JVM runs. private static String suffixAlternation() { var longestFirst = Comparator.comparingInt(String::length) .reversed() diff --git a/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java b/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java index 93a9c6d..05584e1 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java +++ b/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java @@ -2,10 +2,8 @@ import org.jspecify.annotations.NullMarked; -/** - * A codebase with no test classes, no records, no controllers and no stores. Used to pin that a rule - * whose selection comes up empty fails instead of reporting success. - */ +/// A codebase with no test classes, no records, no controllers and no stores. Used to pin that a rule +/// whose selection comes up empty fails instead of reporting success. @NullMarked public class PlainClass { public String value() { diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badctorparam/AnnotatedConstructorParameter.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badctorparam/AnnotatedConstructorParameter.java index e7dea28..025c4bf 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badctorparam/AnnotatedConstructorParameter.java +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistannotations/badctorparam/AnnotatedConstructorParameter.java @@ -1,6 +1,6 @@ package it.aboutbits.archunit.fixture.blacklistannotations.badctorparam; -/** The canonical Lombok position, and the one a rule iterating only getMethods() cannot see. */ +/// The canonical Lombok position, and the one a rule iterating only getMethods() cannot see. public class AnnotatedConstructorParameter { private final String value; diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badfieldinit/CallsFromFieldInitializer.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badfieldinit/CallsFromFieldInitializer.java index 53cfa2b..32e1c60 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badfieldinit/CallsFromFieldInitializer.java +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badfieldinit/CallsFromFieldInitializer.java @@ -4,7 +4,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; -/** An instance field initializer is compiled into the constructor. */ +/// An instance field initializer is compiled into the constructor. public class CallsFromFieldInitializer { private final AbstractThrowableAssert assertion = assertThatThrownBy(() -> { throw new IllegalStateException(); diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/WidgetTest.java index cad94a1..20a9a77 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/WidgetTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badgroup/WidgetTest.java @@ -2,7 +2,7 @@ import org.junit.jupiter.api.Nested; -/** The group implies a production class Widget$DeleteAction, which does not exist. */ +/// The group implies a production class Widget$DeleteAction, which does not exist. class WidgetTest { @Nested class DeleteAction { diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/WidgetTest.java index 8009d03..c1729bc 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/WidgetTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badmethod/WidgetTest.java @@ -2,7 +2,7 @@ import org.junit.jupiter.api.Nested; -/** Widget has no doSomethingElse() method. */ +/// Widget has no doSomethingElse() method. class WidgetTest { @Nested class DoSomethingElse { diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badnoproduction/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badnoproduction/WidgetTest.java index 27483ab..0d8c7cd 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badnoproduction/WidgetTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/badnoproduction/WidgetTest.java @@ -2,7 +2,7 @@ import org.junit.jupiter.api.Nested; -/** No Widget class at all, so there is no method name to match DoWork against. */ +/// No Widget class at all, so there is no method name to match DoWork against. class WidgetTest { @Nested class DoWork { diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java index 0401854..c9f1769 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java @@ -3,10 +3,8 @@ import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; import org.junit.jupiter.api.Nested; -/** - * Declares that it has no production counterpart, so its @Nested classes have no production methods - * to be matched against either. - */ +/// Declares that it has no production counterpart, so its `@Nested` classes have no production methods +/// to be matched against either. @ArchIgnoreNoProductionCounterpart class ScenarioTest { @Nested diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/WidgetTest.java index f47da21..27b4936 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/WidgetTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/WidgetTest.java @@ -2,7 +2,7 @@ import org.junit.jupiter.api.Nested; -/** A conforming test class, so the rule has something to select. */ +/// A conforming test class, so the rule has something to select. class WidgetTest { @Nested class DoWork { diff --git a/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/badnested/EnclosingReadsNestedRecordField.java b/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/badnested/EnclosingReadsNestedRecordField.java index 64ae7cc..c0e0235 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/badnested/EnclosingReadsNestedRecordField.java +++ b/src/test/java/it/aboutbits/archunit/fixture/recordaccessor/badnested/EnclosingReadsNestedRecordField.java @@ -4,7 +4,7 @@ public class EnclosingReadsNestedRecordField { record Money(long amount) { } - /** Nestmates share access to private members, so this compiles to a direct field read. */ + /// Nestmates share access to private members, so this compiles to a direct field read. public long readDirectly(Money money) { return money.amount; } diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetControllerSecurityTest.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetControllerSecurityTest.java index 7000f8c..2d7e652 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetControllerSecurityTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badnonested/WidgetControllerSecurityTest.java @@ -1,5 +1,5 @@ package it.aboutbits.archunit.fixture.securitytested.badnonested; -/** The security test class exists but covers nothing. */ +/// The security test class exists but covers nothing. class WidgetControllerSecurityTest { } diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java index 729ebeb..a77ac72 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java @@ -2,10 +2,8 @@ import org.junit.jupiter.api.Nested; -/** - * Only getAllArchived() is security tested. A prefix match would let this @Nested class stand in - * for getAll() as well. - */ +/// Only getAllArchived() is security tested. A prefix match would let this `@Nested` class stand in +/// for getAll() as well. class WidgetControllerSecurityTest { @Nested class GetAllArchived { diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java index 283b479..54e427e 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java @@ -3,10 +3,8 @@ import it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName; import org.junit.jupiter.api.Nested; -/** - * getAll() is covered by a @Nested class grouped inside the method-named class. Also exercises - * @Controller rather than @RestController. - */ +/// getAll() is covered by a `@Nested` class grouped inside the method-named class. Also exercises +/// `@Controller` rather than `@RestController`. class WidgetControllerSecurityTest { @Nested @ArchIgnoreGroupName diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetStore.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetStore.java index 0d92f80..2f7e6c6 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetStore.java +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/bad/WidgetStore.java @@ -3,7 +3,7 @@ import it.aboutbits.springboot.toolbox.persistence.SortMappings; import it.aboutbits.springboot.toolbox.stereotype.Store; -/** CREATED_AT has no mapping. */ +/// CREATED_AT has no mapping. @Store("widget") public class WidgetStore { static final SortMappings SORT_MAPPINGS = SortMappings.of(WidgetSort.NAME); diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetStore.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetStore.java index 7811f27..13b73f2 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetStore.java +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnonstatic/WidgetStore.java @@ -3,7 +3,7 @@ import it.aboutbits.springboot.toolbox.persistence.SortMappings; import it.aboutbits.springboot.toolbox.stereotype.Store; -/** A non-static field cannot be read reflectively, so its mappings cannot be validated. */ +/// A non-static field cannot be read reflectively, so its mappings cannot be validated. @Store("widget") public class WidgetStore { private final SortMappings sortMappings = SortMappings.of(WidgetSort.NAME); diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/bad/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/bad/WidgetTest.java index 1146717..852094e 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/bad/WidgetTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/bad/WidgetTest.java @@ -1,5 +1,5 @@ package it.aboutbits.archunit.fixture.testclasspackage.bad; -/** No Widget class in this package. */ +/// No Widget class in this package. class WidgetTest { } diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java index 696227a..65b2089 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java @@ -2,7 +2,7 @@ import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; -/** Named after the behaviour it describes, with no production class of its own. */ +/// Named after the behaviour it describes, with no production class of its own. @ArchIgnoreNoProductionCounterpart class ScenarioTest { } diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/WidgetTest.java index 45a3184..d2fb162 100644 --- a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/WidgetTest.java +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/WidgetTest.java @@ -1,5 +1,5 @@ package it.aboutbits.archunit.fixture.testclasspackage.goodoptout; -/** A conforming test class, so the rule has something to select. */ +/// A conforming test class, so the rule has something to select. class WidgetTest { } diff --git a/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java b/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java index 572ae85..3a13f6c 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java @@ -5,14 +5,11 @@ import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; import org.jspecify.annotations.NullMarked; -/** - * The toolbox checked against its own base rules. - *

- * Carries @ArchIgnoreNoProductionCounterpart because there is no production class named - * "Architecture". That the 11 rules below still run is also what pins that the annotation exempts a - * class from one rule rather than skipping every @ArchTest on it. - *

- */ +/// The toolbox checked against its own base rules. +/// +/// Carries `@ArchIgnoreNoProductionCounterpart` because there is no production class named +/// "Architecture". That the 11 rules below still run is also what pins that the annotation exempts a +/// class from one rule rather than skipping every `@ArchTest` on it. @SuppressWarnings("checkstyle:HideUtilityClassConstructor") @AnalyzeClasses( packages = ArchitectureTest.PACKAGE, diff --git a/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java b/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java index d79b98e..5ff9438 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java @@ -15,20 +15,16 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.junit.jupiter.params.provider.Arguments.arguments; -/** - * A rule must not complain about code the project does not have. - *

- * Whether a project contains records, controllers, @Store classes or @Nested test classes is the - * project's business, so every rule tolerates a selection that comes up empty. That each rule can - * still fail is guaranteed by its own red test in this project, not by making consumers fail - an - * empty selection says nothing about whether a rule's logic works. The counterpart rule proved that: - * its selection was never empty, its condition was simply broken. - *

- *

- * The one genuinely dangerous case, nothing imported at all, is covered by - * AnalyzedPackagesMustContainClassesArchRule. - *

- */ +/// A rule must not complain about code the project does not have. +/// +/// Whether a project contains records, controllers, `@Store` classes or `@Nested` test classes is the +/// project's business, so every rule tolerates a selection that comes up empty. That each rule can +/// still fail is guaranteed by its own red test in this project, not by making consumers fail - an +/// empty selection says nothing about whether a rule's logic works. The counterpart rule proved that: +/// its selection was never empty, its condition was simply broken. +/// +/// The one genuinely dangerous case, nothing imported at all, is covered by +/// AnalyzedPackagesMustContainClassesArchRule. @NullMarked @ArchIgnoreNoProductionCounterpart class EmptySelectionTest { diff --git a/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java b/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java index aa09aae..88dcc06 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java @@ -6,13 +6,10 @@ import java.util.regex.Pattern; -/** - * Imports a rule fixture and inspects what a rule reports about it. - *

- * Rules are invoked through their own {@code default} method, so a test exercises exactly what a - * consumer gets, including the message text. - *

- */ +/// Imports a rule fixture and inspects what a rule reports about it. +/// +/// Rules are invoked through their own `default` method, so a test exercises exactly what a +/// consumer gets, including the message text. @NullMarked public final class RuleEvaluation { private static final String FIXTURE_ROOT = "it.aboutbits.archunit.fixture."; @@ -21,10 +18,8 @@ public final class RuleEvaluation { private RuleEvaluation() { } - /** - * Imports one fixture package, failing loudly if it is empty: a mistyped package would otherwise - * make every assertion about it pass for the wrong reason. - */ + /// Imports one fixture package, failing loudly if it is empty: a mistyped package would otherwise + /// make every assertion about it pass for the wrong reason. public static JavaClasses fixture(String subPackage) { var packageName = FIXTURE_ROOT + subPackage; var classes = new ClassFileImporter().importPackages(packageName); @@ -36,7 +31,7 @@ public static JavaClasses fixture(String subPackage) { return classes; } - /** An import that yielded nothing, as a consumer with a mistyped @AnalyzeClasses package gets. */ + /// An import that yielded nothing, as a consumer with a mistyped `@AnalyzeClasses` package gets. public static JavaClasses noClassesImported() { var classes = new ClassFileImporter().importPackages(FIXTURE_ROOT + "nosuchpackage"); @@ -47,10 +42,8 @@ public static JavaClasses noClassesImported() { return classes; } - /** - * Runs a rule that is expected to report at least one violation and returns the failure. - * Failing here means the rule accepted a fixture that was built to violate it. - */ + /// Runs a rule that is expected to report at least one violation and returns the failure. + /// Failing here means the rule accepted a fixture that was built to violate it. public static AssertionError violationOf(Runnable ruleCheck) { try { ruleCheck.run(); @@ -61,7 +54,7 @@ public static AssertionError violationOf(Runnable ruleCheck) { throw new AssertionError("Expected the rule to report a violation, but it reported success."); } - /** The number of violations ArchUnit reported, read back from its failure message. */ + /// The number of violations ArchUnit reported, read back from its failure message. public static int violationCount(AssertionError failure) { var matcher = VIOLATION_COUNT.matcher(String.valueOf(failure.getMessage())); diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java index a1c04b1..e40f832 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java @@ -11,10 +11,8 @@ @NullMarked class AnalyzedPackagesMustContainClassesArchRuleTest implements AnalyzedPackagesMustContainClassesArchRule { - /** - * The one case every other rule deliberately tolerates: nothing was imported, so nothing can be - * checked and every rule would otherwise pass. - */ + /// The one case every other rule deliberately tolerates: nothing was imported, so nothing can be + /// checked and every rule would otherwise pass. @Test void an_import_without_any_classes_is_reported() { var classes = noClassesImported(); diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java index 3106873..5607455 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java @@ -42,10 +42,8 @@ void a_blacklisted_annotation_on_a_field_is_reported() { assertThat(failure).hasMessageContaining("Field value").hasMessageContaining("lombok.NonNull"); } - /** - * The position that matters most in practice, and the one a rule looking only at getMethods() - * cannot see. - */ + /// The position that matters most in practice, and the one a rule looking only at getMethods() + /// cannot see. @Test void a_blacklisted_annotation_on_a_constructor_parameter_is_reported() { var failure = violationOf( diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java index e1d7f51..c263e05 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java @@ -30,7 +30,7 @@ void a_blacklisted_call_from_a_constructor_is_reported() { .hasMessageContaining("assertThatThrownBy"); } - /** An instance field initializer is compiled into the constructor, so it needs the same reach. */ + /// An instance field initializer is compiled into the constructor, so it needs the same reach. @Test void a_blacklisted_call_from_an_instance_field_initializer_is_reported() { var failure = violationOf(() -> no_blacklisted_methods_are_used(fixture("blacklistmethods.badfieldinit"))); @@ -56,10 +56,8 @@ void an_allowed_assertion_is_accepted() { no_blacklisted_methods_are_used(fixture("blacklistmethods.good")); } - /** - * A blacklist entry naming a method that does not exist can never match, so it reads as coverage - * without providing any. - */ + /// A blacklist entry naming a method that does not exist can never match, so it reads as coverage + /// without providing any. @Test void the_blacklist_does_not_name_assertj_methods_that_do_not_exist() { assertThat(BLACKLISTED_METHODS) diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java index 3f6e46b..b64a41c 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java @@ -11,10 +11,8 @@ @NullMarked class RecordPropertiesMustBeAccessedViaAccessorArchRuleTest { - /** - * Only reachable for a nested record: nestmates share access to private members, so the read - * compiles to a direct field access rather than an accessor call. - */ + /// Only reachable for a nested record: nestmates share access to private members, so the read + /// compiles to a direct field access rather than an accessor call. @Test void a_direct_read_of_a_record_field_from_outside_the_record_is_reported() { var classes = fixture("recordaccessor.badnested"); diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java index efd95b9..6125699 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java @@ -34,9 +34,7 @@ void a_security_test_class_without_the_nested_method_class_is_reported() { assertThat(failure).hasMessageContaining("does not contain a @Nested test class named GetAll"); } - /** - * getAll() must not be considered covered by the @Nested class belonging to getAllArchived(). - */ + /// getAll() must not be considered covered by the `@Nested` class belonging to getAllArchived(). @Test void a_mapped_method_covered_only_by_a_longer_named_sibling_is_reported() { var classes = fixture("securitytested.badprefix"); @@ -55,7 +53,7 @@ void a_mapped_method_with_a_matching_nested_test_class_is_accepted() { controller_methods_with_request_mapping_must_be_security_tested(fixture("securitytested.good")); } - /** A @Nested class grouped inside the method-named class still counts as coverage. */ + /// A `@Nested` class grouped inside the method-named class still counts as coverage. @Test void a_mapped_method_covered_by_a_nested_group_is_accepted() { controller_methods_with_request_mapping_must_be_security_tested(fixture("securitytested.goodnestedgroup")); diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java index 6eac599..fbf4e15 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java @@ -22,10 +22,8 @@ void a_sort_enum_value_without_a_mapping_is_reported() { .hasMessageContaining("CREATED_AT"); } - /** - * A non-static field cannot be read reflectively, so it used to be skipped with nothing but a - * discarded log warning. - */ + /// A non-static field cannot be read reflectively, so it used to be skipped with nothing but a + /// discarded log warning. @Test void a_non_static_sort_mappings_field_is_reported() { var classes = fixture("sortmappings.badnonstatic"); diff --git a/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupNameTest.java b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupNameTest.java index 39e8b9b..d6bd59b 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupNameTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreGroupNameTest.java @@ -8,7 +8,7 @@ @NullMarked class ArchIgnoreGroupNameTest { - /** See ArchIgnoreNoProductionCounterpartTest: a meta @ArchIgnore skips arch tests wholesale. */ + /// See ArchIgnoreNoProductionCounterpartTest: a meta `@ArchIgnore` skips arch tests wholesale. @Test void the_annotation_is_not_meta_annotated_with_arch_ignore() { assertThat(ArchIgnoreGroupName.class.isAnnotationPresent(ArchIgnore.class)) diff --git a/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java index 561d94b..85e666b 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java @@ -8,12 +8,10 @@ @NullMarked class ArchIgnoreNoProductionCounterpartTest { - /** - * ArchUnit's JUnit engine resolves meta-annotations, so meta-annotating this with @ArchIgnore - * makes it skip every @ArchTest on the annotated class - reported as success - rather than - * exempting the class from one rule. The rules read this annotation by its own type, so the - * meta-annotation buys nothing and costs all of them. - */ + /// ArchUnit's JUnit engine resolves meta-annotations, so meta-annotating this with `@ArchIgnore` + /// makes it skip every `@ArchTest` on the annotated class - reported as success - rather than + /// exempting the class from one rule. The rules read this annotation by its own type, so the + /// meta-annotation buys nothing and costs all of them. @Test void the_annotation_is_not_meta_annotated_with_arch_ignore() { assertThat(ArchIgnoreNoProductionCounterpart.class.isAnnotationPresent(ArchIgnore.class)) diff --git a/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java b/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java index 797ee8a..bff3e77 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java @@ -15,21 +15,17 @@ void a_name_ending_in_a_configured_suffix_is_a_test_class_name() { assertThat(TestClassNames.isTestClassName("WidgetSecurityTest")).isTrue(); } - /** - * The regression guard for the defect that disabled the counterpart rule outright: the condition - * rebuilt the pattern without the leading ".+", and String.matches anchors both ends, so only a - * class named exactly "Test" got through. - */ + /// The regression guard for the defect that disabled the counterpart rule outright: the condition + /// rebuilt the pattern without the leading ".+", and String.matches anchors both ends, so only a + /// class named exactly "Test" got through. @Test void the_pattern_is_not_satisfied_by_the_bare_suffix_alone() { assertThat(TestClassNames.isTestClassName("Test")).isFalse(); assertThat("WidgetTest".matches(TestClassNames.testClassNameRegex())).isTrue(); } - /** - * "CacheTest" matches the pattern with "Cache" as the leading ".+", but stripping removes - * "CacheTest" whole, so there would be no production class name left to look for. - */ + /// "CacheTest" matches the pattern with "Cache" as the leading ".+", but stripping removes + /// "CacheTest" whole, so there would be no production class name left to look for. @Test void a_name_that_strips_down_to_nothing_is_not_a_test_class_name() { assertThat(TestClassNames.isTestClassName("CacheTest")).isFalse(); @@ -52,10 +48,8 @@ void the_production_class_name_is_the_name_without_its_suffix() { assertThat(TestClassNames.productionClassSimpleName("WidgetSecurityTest")).isEqualTo("Widget"); } - /** - * TEST_CLASS_SUFFIXES is a mutable HashSet, so without an explicit ordering the generated regex - * and every rule description built from it would vary between JVM runs. - */ + /// TEST_CLASS_SUFFIXES is a mutable HashSet, so without an explicit ordering the generated regex + /// and every rule description built from it would vary between JVM runs. @Test void the_generated_pattern_has_a_stable_order() { assertThat(TestClassNames.testClassNameRegex()) diff --git a/src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java b/src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java index 2a21048..ed1c0f1 100644 --- a/src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java +++ b/src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java @@ -5,10 +5,8 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -/** - * Stub of the spring-boot-toolbox opt-out annotation. Verified against the real artifact: RUNTIME - * retention, targets TYPE, FIELD and RECORD_COMPONENT. - */ +/// Stub of the spring-boot-toolbox opt-out annotation. Verified against the real artifact: RUNTIME +/// retention, targets TYPE, FIELD and RECORD_COMPONENT. @Target({ElementType.TYPE, ElementType.FIELD, ElementType.RECORD_COMPONENT}) @Retention(RetentionPolicy.RUNTIME) public @interface ArchAllowDirectAccess { diff --git a/src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java b/src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java index bcf308e..703700b 100644 --- a/src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java +++ b/src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java @@ -2,10 +2,8 @@ import java.util.HashMap; -/** - * Stub of the spring-boot-toolbox type. Verified against the real artifact: it extends HashMap keyed - * by the Sort enum, which is what the rule relies on when it reads the mappings reflectively. - */ +/// Stub of the spring-boot-toolbox type. Verified against the real artifact: it extends HashMap keyed +/// by the Sort enum, which is what the rule relies on when it reads the mappings reflectively. public class SortMappings> extends HashMap { @SafeVarargs public static > SortMappings of(T... keys) { diff --git a/src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java b/src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java index a8d25fb..88da7de 100644 --- a/src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java +++ b/src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java @@ -5,10 +5,8 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -/** - * Stub of the spring-boot-toolbox stereotype. spring-boot-toolbox depends on archunit-toolbox, so it - * cannot be a dependency here. Verified against the real artifact: RUNTIME retention, TYPE target. - */ +/// Stub of the spring-boot-toolbox stereotype. spring-boot-toolbox depends on archunit-toolbox, so it +/// cannot be a dependency here. Verified against the real artifact: RUNTIME retention, TYPE target. @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME) public @interface Store { diff --git a/src/test/java/net/datafaker/Faker.java b/src/test/java/net/datafaker/Faker.java index ada3e8d..abfd031 100644 --- a/src/test/java/net/datafaker/Faker.java +++ b/src/test/java/net/datafaker/Faker.java @@ -1,6 +1,6 @@ package net.datafaker; -/** Stub of the blacklisted net.datafaker.Faker. */ +/// Stub of the blacklisted net.datafaker.Faker. public class Faker { public String name() { return "stub"; diff --git a/src/test/java/org/junit/Ignore.java b/src/test/java/org/junit/Ignore.java index a9fcb71..afc06e7 100644 --- a/src/test/java/org/junit/Ignore.java +++ b/src/test/java/org/junit/Ignore.java @@ -5,10 +5,8 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -/** - * Stub of the blacklisted JUnit 4 annotation, so a rule test can pin a real entry of - * BLACKLISTED_ANNOTATIONS without putting JUnit 4 on this library's classpath. - */ +/// Stub of the blacklisted JUnit 4 annotation, so a rule test can pin a real entry of +/// BLACKLISTED_ANNOTATIONS without putting JUnit 4 on this library's classpath. @Target({ElementType.TYPE, ElementType.METHOD}) @Retention(RetentionPolicy.RUNTIME) public @interface Ignore { From 3ad7191b3a165ae9809146c32190f7952ab2bff5 Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Wed, 19 Aug 2026 17:56:40 +0200 Subject: [PATCH 7/9] 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 --- readme.md | 21 ++++++++++++++- .../rule/base/BlacklistMethodsArchRule.java | 1 + .../TestClassInCorrectPackageArchRule.java | 19 +++++++++++--- .../TestNestedClassMatchNameArchRule.java | 10 +++---- ...erRequestMappingsMustBeSecurityTested.java | 4 +-- .../CallsJunitAssertThrowsExactly.java | 11 ++++++++ .../goodmetagroup/TestGroup.java | 16 ++++++++++++ .../nestedclassname/goodmetagroup/Widget.java | 6 +++++ .../goodmetagroup/WidgetTest.java | 16 ++++++++++++ .../goodmetaoptout/BusinessScenario.java | 15 +++++++++++ .../goodmetaoptout/ScenarioTest.java | 12 +++++++++ .../goodmetaoptout/Widget.java | 6 +++++ .../goodmetaoptout/WidgetTest.java | 10 +++++++ .../badmetagroup/TestGroup.java | 14 ++++++++++ .../badmetagroup/WidgetController.java | 12 +++++++++ .../WidgetControllerSecurityTest.java | 12 +++++++++ .../badarchitecture/ArchitectureTest.java | 6 +++++ .../goodmetaoptout/BusinessScenario.java | 16 ++++++++++++ .../goodmetaoptout/ScenarioTest.java | 6 +++++ .../goodmetaoptout/Widget.java | 4 +++ .../goodmetaoptout/WidgetTest.java | 5 ++++ .../witharchitecture/Widget.java | 4 +++ .../witharchitecture/WidgetTest.java | 6 +++++ .../_architecture/ArchitectureTest.java | 5 ++++ .../base/BlacklistMethodsArchRuleTest.java | 21 +++++++++++++++ ...TestClassInCorrectPackageArchRuleTest.java | 26 +++++++++++++++++++ .../TestNestedClassMatchNameArchRuleTest.java | 12 +++++++++ ...questMappingsMustBeSecurityTestedTest.java | 13 ++++++++++ 28 files changed, 297 insertions(+), 12 deletions(-) create mode 100644 src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badjunitassertion/CallsJunitAssertThrowsExactly.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/TestGroup.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/BusinessScenario.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/ScenarioTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/TestGroup.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/WidgetController.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/WidgetControllerSecurityTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/badarchitecture/ArchitectureTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/BusinessScenario.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/ScenarioTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/_architecture/ArchitectureTest.java diff --git a/readme.md b/readme.md index ee63357..14f9445 100644 --- a/readme.md +++ b/readme.md @@ -73,7 +73,26 @@ Two annotations exempt a class from a specific rule. Neither is meta-annotated w | `@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, and on your own `ArchitectureTest`. +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 + +@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. + +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 diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java index 74d65a0..251183e 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRule.java @@ -24,6 +24,7 @@ public interface BlacklistMethodsArchRule { // We should use `assertThatExceptionOfType(...).isThrownBy(...)` instead of `assertThatThrownBy(...)` "org.assertj.core.api.Assertions.assertThatThrownBy", "org.junit.jupiter.api.Assertions.assertThrows", + "org.junit.jupiter.api.Assertions.assertThrowsExactly", "org.junit.jupiter.api.Assertions.assertDoesNotThrow", // assertThat (allowed is only org.assertj.core.api.Assertions.assertThat) "org.assertj.core.api.AssertionsForClassTypes.assertThat", diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java index 3ea805e..f75633d 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java @@ -19,13 +19,24 @@ public interface TestClassInCorrectPackageArchRule { default void test_classes_should_be_in_the_same_package_as_their_production_code(JavaClasses classes) { classes().that(TestClassNames.testClasses()) .and() - .areNotAnnotatedWith(org.junit.jupiter.api.Disabled.class) + .areNotMetaAnnotatedWith(org.junit.jupiter.api.Disabled.class) .and() - .areNotAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) + .areNotMetaAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) .and() - .areNotAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart.class) + /* + * Meta-annotated, not annotated: a project marks its scenario tests with one + * stereotype of its own that carries this annotation, rather than repeating the + * annotation on every class. ArchUnit counts a direct annotation as meta-annotated, + * so annotating a single class still works. + */ + .areNotMetaAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart.class) .and() - .resideOutsideOfPackages(".._support..", ".._config..") + /* + * An architecture test is named after no production class by definition. Excluded by + * package here, but deliberately not in TestClassVisibilityArchRule: being package + * private is just as achievable for an architecture test as for any other test. + */ + .resideOutsideOfPackages(".._support..", ".._config..", ".._architecture..") .should(new BeInTheSamePackageAsTheProductionClass(classes)) .allowEmptyShould(true) .check(classes); diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java index 1029adc..55ddd31 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java @@ -23,15 +23,15 @@ public interface TestNestedClassMatchNameArchRule { default void nested_test_classes_have_matching_production_method_name(JavaClasses classes) { classes().that(TestClassNames.testClasses()) .and() - .areNotAnnotatedWith(org.junit.jupiter.api.Disabled.class) + .areNotMetaAnnotatedWith(org.junit.jupiter.api.Disabled.class) .and() - .areNotAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) + .areNotMetaAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) .and() /* * A test class that declares it has no production counterpart has no production * methods to match its @Nested classes against either. */ - .areNotAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart.class) + .areNotMetaAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart.class) .should(new HaveNestedClassesThatHaveAMatchingProductionMethodName(classes)) .allowEmptyShould(true) .check(classes); @@ -53,7 +53,7 @@ public void check(JavaClass testClass, ConditionEvents events) { .stream() .filter(clazz -> clazz.getName().startsWith(testClass.getName() + "$") && clazz.isAnnotatedWith(org.junit.jupiter.api.Nested.class) - && !clazz.isAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class) + && !clazz.isMetaAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class) && !clazz.getName().endsWith("$Validation") ) .collect(Collectors.toSet()); @@ -83,7 +83,7 @@ public void check(JavaClass testClass, ConditionEvents events) { .stream() .anyMatch(clazz -> clazz.getName().startsWith(nestedClass.getName() + "$") && clazz.isAnnotatedWith(org.junit.jupiter.api.Nested.class) - && !clazz.isAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class) + && !clazz.isMetaAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class) && !clazz.getName().endsWith("$Validation") ) ) { diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java index f3a745e..87c86bd 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java @@ -91,8 +91,8 @@ public void check(JavaMethod method, ConditionEvents events) { .stream() .anyMatch(clazz -> isExpectedNestedClass(clazz.getName(), expectedNestedClassName) && clazz.isAnnotatedWith(org.junit.jupiter.api.Nested.class) - && !clazz.isAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) - && !clazz.isAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class) + && !clazz.isMetaAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) + && !clazz.isMetaAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class) ); if (!nestedMethodTestClassFound) { diff --git a/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badjunitassertion/CallsJunitAssertThrowsExactly.java b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badjunitassertion/CallsJunitAssertThrowsExactly.java new file mode 100644 index 0000000..38eeaad --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/blacklistmethods/badjunitassertion/CallsJunitAssertThrowsExactly.java @@ -0,0 +1,11 @@ +package it.aboutbits.archunit.fixture.blacklistmethods.badjunitassertion; + +import org.junit.jupiter.api.Assertions; + +public class CallsJunitAssertThrowsExactly { + public void check() { + Assertions.assertThrowsExactly(IllegalStateException.class, () -> { + throw new IllegalStateException(); + }); + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/TestGroup.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/TestGroup.java new file mode 100644 index 0000000..ca96802 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/TestGroup.java @@ -0,0 +1,16 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodmetagroup; + +import it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/// A project's own marker for a purely organisational @Nested class, carrying the opt-out as a +/// meta-annotation. +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@ArchIgnoreGroupName +public @interface TestGroup { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/Widget.java new file mode 100644 index 0000000..e500d63 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/Widget.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodmetagroup; + +public class Widget { + public void doWork() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/WidgetTest.java new file mode 100644 index 0000000..bbee129 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetagroup/WidgetTest.java @@ -0,0 +1,16 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodmetagroup; + +import org.junit.jupiter.api.Nested; + +class WidgetTest { + /// Matches Widget.doWork(), so the rule has a nested class to actually check. + @Nested + class DoWork { + } + + /// Groups tests only. Widget has no someGrouping() method, and must not be expected to. + @Nested + @TestGroup + class SomeGrouping { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/BusinessScenario.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/BusinessScenario.java new file mode 100644 index 0000000..ecf6332 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/BusinessScenario.java @@ -0,0 +1,15 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodmetaoptout; + +import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/// A project's own test stereotype, carrying the opt-out as a meta-annotation. +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@ArchIgnoreNoProductionCounterpart +public @interface BusinessScenario { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/ScenarioTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/ScenarioTest.java new file mode 100644 index 0000000..db46c6f --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/ScenarioTest.java @@ -0,0 +1,12 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodmetaoptout; + +import org.junit.jupiter.api.Nested; + +/// Opted out through the project's stereotype: its @Nested classes have no production methods to +/// be matched against either. +@BusinessScenario +class ScenarioTest { + @Nested + class SomeBehaviour { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/Widget.java new file mode 100644 index 0000000..7c5ddfa --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/Widget.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodmetaoptout; + +public class Widget { + public void doWork() { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/WidgetTest.java new file mode 100644 index 0000000..c007758 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodmetaoptout/WidgetTest.java @@ -0,0 +1,10 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodmetaoptout; + +import org.junit.jupiter.api.Nested; + +/// A conforming test class, so the rule has something to select. +class WidgetTest { + @Nested + class DoWork { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/TestGroup.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/TestGroup.java new file mode 100644 index 0000000..98a15ac --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/TestGroup.java @@ -0,0 +1,14 @@ +package it.aboutbits.archunit.fixture.securitytested.badmetagroup; + +import it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@ArchIgnoreGroupName +public @interface TestGroup { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/WidgetController.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/WidgetController.java new file mode 100644 index 0000000..483b2f6 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/WidgetController.java @@ -0,0 +1,12 @@ +package it.aboutbits.archunit.fixture.securitytested.badmetagroup; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class WidgetController { + @GetMapping("/widgets") + public String getAll() { + return "[]"; + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/WidgetControllerSecurityTest.java b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/WidgetControllerSecurityTest.java new file mode 100644 index 0000000..22e08de --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badmetagroup/WidgetControllerSecurityTest.java @@ -0,0 +1,12 @@ +package it.aboutbits.archunit.fixture.securitytested.badmetagroup; + +import org.junit.jupiter.api.Nested; + +/// GetAll is marked, through the project's own stereotype, as organisational rather than a test of +/// getAll(). So getAll() is not covered and must be reported. +class WidgetControllerSecurityTest { + @Nested + @TestGroup + class GetAll { + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/badarchitecture/ArchitectureTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/badarchitecture/ArchitectureTest.java new file mode 100644 index 0000000..962b57e --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/badarchitecture/ArchitectureTest.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.testclasspackage.badarchitecture; + +/// The same name, outside an architecture package. The exemption comes from the package, not from +/// the class being called ArchitectureTest, so this one is still reported. +class ArchitectureTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/BusinessScenario.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/BusinessScenario.java new file mode 100644 index 0000000..859332c --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/BusinessScenario.java @@ -0,0 +1,16 @@ +package it.aboutbits.archunit.fixture.testclasspackage.goodmetaoptout; + +import it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/// A project's own test stereotype, carrying the opt-out as a meta-annotation so it is declared +/// once rather than repeated on every scenario test. +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@ArchIgnoreNoProductionCounterpart +public @interface BusinessScenario { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/ScenarioTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/ScenarioTest.java new file mode 100644 index 0000000..ea24676 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/ScenarioTest.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.testclasspackage.goodmetaoptout; + +/// Opted out through the project's stereotype, not by carrying the annotation itself. +@BusinessScenario +class ScenarioTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/Widget.java new file mode 100644 index 0000000..972018c --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/Widget.java @@ -0,0 +1,4 @@ +package it.aboutbits.archunit.fixture.testclasspackage.goodmetaoptout; + +public class Widget { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/WidgetTest.java new file mode 100644 index 0000000..72255cd --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodmetaoptout/WidgetTest.java @@ -0,0 +1,5 @@ +package it.aboutbits.archunit.fixture.testclasspackage.goodmetaoptout; + +/// A conforming test class, so the rule has something to select. +class WidgetTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/Widget.java new file mode 100644 index 0000000..88f39e3 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/Widget.java @@ -0,0 +1,4 @@ +package it.aboutbits.archunit.fixture.testclasspackage.witharchitecture; + +public class Widget { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/WidgetTest.java new file mode 100644 index 0000000..0925359 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/WidgetTest.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.testclasspackage.witharchitecture; + +/// A conforming test class, so the rule selects something and the architecture test below is not +/// exempted merely by an empty selection. +class WidgetTest { +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/_architecture/ArchitectureTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/_architecture/ArchitectureTest.java new file mode 100644 index 0000000..eb8455d --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/witharchitecture/_architecture/ArchitectureTest.java @@ -0,0 +1,5 @@ +package it.aboutbits.archunit.fixture.testclasspackage.witharchitecture._architecture; + +/// Excluded by its package: an architecture test is named after no production class by definition. +class ArchitectureTest { +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java index c263e05..e61a30e 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java @@ -56,6 +56,27 @@ void an_allowed_assertion_is_accepted() { no_blacklisted_methods_are_used(fixture("blacklistmethods.good")); } + @Test + void a_blacklisted_junit_assertion_is_reported() { + var failure = violationOf( + () -> no_blacklisted_methods_are_used(fixture("blacklistmethods.badjunitassertion"))); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure) + .hasMessageContaining("org.junit.jupiter.api.Assertions.assertThrowsExactly"); + } + + /// Removing the AssertJ entries below must not drop the house rule itself: these three methods + /// exist on JUnit's Assertions and have to stay blacklisted under that owner. + @Test + void the_blacklist_names_the_junit_assertion_methods_under_their_real_owner() { + assertThat(BLACKLISTED_METHODS).contains( + "org.junit.jupiter.api.Assertions.assertThrows", + "org.junit.jupiter.api.Assertions.assertThrowsExactly", + "org.junit.jupiter.api.Assertions.assertDoesNotThrow" + ); + } + /// A blacklist entry naming a method that does not exist can never match, so it reads as coverage /// without providing any. @Test diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java index 7efacb3..bfccf9c 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java @@ -32,4 +32,30 @@ void a_test_class_next_to_its_production_class_is_accepted() { void a_test_class_annotated_as_having_no_production_counterpart_is_accepted() { test_classes_should_be_in_the_same_package_as_their_production_code(fixture("testclasspackage.goodoptout")); } + + /// The opt-out has to be usable once, on a project's own test stereotype, rather than repeated on + /// every scenario test. + @Test + void a_test_class_opted_out_through_a_meta_annotation_is_accepted() { + test_classes_should_be_in_the_same_package_as_their_production_code( + fixture("testclasspackage.goodmetaoptout")); + } + + @Test + void an_architecture_test_in_its_own_package_is_accepted() { + test_classes_should_be_in_the_same_package_as_their_production_code( + fixture("testclasspackage.witharchitecture")); + } + + /// The exemption is the package, not the name: the rule no longer hardcodes "ArchitectureTest". + @Test + void an_architecture_test_outside_an_architecture_package_is_reported() { + var classes = fixture("testclasspackage.badarchitecture"); + + var failure = violationOf( + () -> test_classes_should_be_in_the_same_package_as_their_production_code(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("testclasspackage.badarchitecture.Architecture"); + } } diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java index c7c7368..bce05e1 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java @@ -55,4 +55,16 @@ void a_nested_test_class_matching_a_production_method_is_accepted() { void a_test_class_annotated_as_having_no_production_counterpart_is_accepted() { nested_test_classes_have_matching_production_method_name(fixture("nestedclassname.goodoptout")); } + + /// The opt-out has to be usable once, on a project's own test stereotype. + @Test + void a_test_class_opted_out_through_a_meta_annotation_is_accepted() { + nested_test_classes_have_matching_production_method_name(fixture("nestedclassname.goodmetaoptout")); + } + + /// Same for the group marker: a project names its own grouping stereotype once. + @Test + void a_nested_group_marked_through_a_meta_annotation_is_accepted() { + nested_test_classes_have_matching_production_method_name(fixture("nestedclassname.goodmetagroup")); + } } diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java index 6125699..807cd25 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java @@ -58,4 +58,17 @@ void a_mapped_method_with_a_matching_nested_test_class_is_accepted() { void a_mapped_method_covered_by_a_nested_group_is_accepted() { controller_methods_with_request_mapping_must_be_security_tested(fixture("securitytested.goodnestedgroup")); } + + /// A class marked as organisational through a project's own stereotype is not coverage, so the + /// method it is named after is still uncovered. + @Test + void a_mapped_method_covered_only_by_a_group_marked_class_is_reported() { + var classes = fixture("securitytested.badmetagroup"); + + var failure = violationOf( + () -> controller_methods_with_request_mapping_must_be_security_tested(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("does not contain a @Nested test class named GetAll"); + } } From 059cb4f7c6a81ccfdb6b629cfde556856d54eabc Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Mon, 31 Aug 2026 14:19:06 +0200 Subject: [PATCH 8/9] 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 --- .../base/AnalyzedPackagesMustContainClassesArchRule.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java index 0bd6f89..22625f8 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java @@ -18,10 +18,9 @@ public interface AnalyzedPackagesMustContainClassesArchRule { @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." - ); + throw new AssertionError(""" + No classes were imported, so none of the architecture rules checked anything. + Verify the packages passed to @AnalyzeClasses."""); } } } From 74ed23755bbc168613b00801ae37c12d0816766a Mon Sep 17 00:00:00 2001 From: Andreas Hufler Date: Mon, 31 Aug 2026 14:50:35 +0200 Subject: [PATCH 9/9] 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 --- readme.md | 25 +++++++++++++++++++ .../TestClassInCorrectPackageArchRule.java | 8 +++--- .../TestNestedClassMatchNameArchRule.java | 11 ++++---- .../nestedclassname/goodgroup/Widget.java | 8 ++++++ .../nestedclassname/goodgroup/WidgetTest.java | 14 +++++++++++ .../sortmappings/badnullvalue/WidgetSort.java | 6 +++++ .../badnullvalue/WidgetStore.java | 14 +++++++++++ .../TestNestedClassMatchNameArchRuleTest.java | 7 ++++++ .../SortMappingsExhaustiveArchRuleTest.java | 12 +++++++++ 9 files changed, 94 insertions(+), 11 deletions(-) create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodgroup/Widget.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodgroup/WidgetTest.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnullvalue/WidgetSort.java create mode 100644 src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnullvalue/WidgetStore.java diff --git a/readme.md b/readme.md index 14f9445..91ef9b1 100644 --- a/readme.md +++ b/readme.md @@ -16,6 +16,25 @@ Add this library to the classpath by adding the following maven dependency. Vers ``` +## 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 Implement one of the provided rule collections in your own architecture test. @@ -90,6 +109,12 @@ 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. diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java index f75633d..d846e09 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java @@ -61,11 +61,9 @@ public void check(JavaClass testClass, ConditionEvents events) { var productionClassSimpleName = TestClassNames.productionClassSimpleName(testClass.getSimpleName()); var productionClassFullName = testClass.getPackageName() + "." + productionClassSimpleName; - var productionClass = allClasses.stream() - .filter(clazz -> clazz.getFullName().equals(productionClassFullName)) - .findFirst(); - - if (productionClass.isEmpty()) { + // JavaClasses is map-backed by fully qualified name, so this is a lookup rather than a + // scan of every imported class per test class. + if (!allClasses.contain(productionClassFullName)) { var message = "Test class <%s> does not have a matching production class <%s> in the same package (%s.java:0)".formatted( testClass.getFullName(), productionClassFullName, diff --git a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java index 55ddd31..c364f6c 100644 --- a/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRule.java @@ -122,16 +122,15 @@ public void check(JavaClass testClass, ConditionEvents events) { enclosingClassSuffix.orElse("") ); - var productionClassOptional = allClasses.stream() - .filter(clazz -> clazz.getFullName().equals(productionClassName)) - .findFirst(); - /* * Reported whether or not the @Nested class is inside a @Nested group. Without a * production class there is nothing to match the name against, so staying silent * here means the @Nested class is never checked at all. + * + * JavaClasses is map-backed by fully qualified name, so this is a lookup rather than + * a scan of every imported class per @Nested class. */ - if (productionClassOptional.isEmpty()) { + if (!allClasses.contain(productionClassName)) { var message = "The @Nested test class <%s> (%s.java:%s)%ndoes not have a matching production class <%s>".formatted( nestedClass.getName(), nestedClassBaseClassSimpleName, @@ -140,7 +139,7 @@ public void check(JavaClass testClass, ConditionEvents events) { ); events.add(SimpleConditionEvent.violated(nestedClass, message)); } else { - var productionClass = productionClassOptional.get(); + var productionClass = allClasses.get(productionClassName); var methodExists = productionClass.getMethods() .stream() diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodgroup/Widget.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodgroup/Widget.java new file mode 100644 index 0000000..8d8a42d --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodgroup/Widget.java @@ -0,0 +1,8 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodgroup; + +public class Widget { + public static class DeleteAction { + public void deleteAll() { + } + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodgroup/WidgetTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodgroup/WidgetTest.java new file mode 100644 index 0000000..141590a --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodgroup/WidgetTest.java @@ -0,0 +1,14 @@ +package it.aboutbits.archunit.fixture.nestedclassname.goodgroup; + +import org.junit.jupiter.api.Nested; + +/// The @Nested group maps onto the production nested class Widget$DeleteAction, so the lookup has to +/// resolve a fully qualified name containing a '$'. +class WidgetTest { + @Nested + class DeleteAction { + @Nested + class DeleteAll { + } + } +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnullvalue/WidgetSort.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnullvalue/WidgetSort.java new file mode 100644 index 0000000..23c5d6b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnullvalue/WidgetSort.java @@ -0,0 +1,6 @@ +package it.aboutbits.archunit.fixture.sortmappings.badnullvalue; + +public enum WidgetSort { + NAME, + CREATED_AT +} diff --git a/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnullvalue/WidgetStore.java b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnullvalue/WidgetStore.java new file mode 100644 index 0000000..d789115 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/sortmappings/badnullvalue/WidgetStore.java @@ -0,0 +1,14 @@ +package it.aboutbits.archunit.fixture.sortmappings.badnullvalue; + +import it.aboutbits.springboot.toolbox.persistence.SortMappings; +import it.aboutbits.springboot.toolbox.stereotype.Store; + +/// A field that reads back as null yields no mappings to compare, so the rule cannot verify it. +@Store("widget") +public class WidgetStore { + static final SortMappings SORT_MAPPINGS = null; + + public Object mappings() { + return SORT_MAPPINGS; + } +} diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java index bce05e1..1f57f5c 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java @@ -46,6 +46,13 @@ void a_nested_test_class_whose_production_class_is_missing_entirely_is_reported( .hasMessageContaining("does not have a matching production class"); } + /// Pins that a production class nested inside another is found: its fully qualified name + /// contains a '$', and the lookup is by that name. + @Test + void a_nested_group_matching_a_production_nested_class_is_accepted() { + nested_test_classes_have_matching_production_method_name(fixture("nestedclassname.goodgroup")); + } + @Test void a_nested_test_class_matching_a_production_method_is_accepted() { nested_test_classes_have_matching_production_method_name(fixture("nestedclassname.good")); diff --git a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java index fbf4e15..c4169a2 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java @@ -34,6 +34,18 @@ void a_non_static_sort_mappings_field_is_reported() { assertThat(failure).hasMessageContaining("must be static"); } + /// Reading the field succeeds but yields nothing to compare, so there is no basis on which to + /// call the mappings exhaustive. + @Test + void a_sort_mappings_field_that_reads_back_as_null_is_reported() { + var classes = fixture("sortmappings.badnullvalue"); + + var failure = violationOf(() -> sort_mappings_cover_all_sort_enum_values(classes)); + + assertThat(violationCount(failure)).isEqualTo(1); + assertThat(failure).hasMessageContaining("did not yield a Map (got null)"); + } + @Test void exhaustive_sort_mappings_are_accepted() { sort_mappings_cover_all_sort_enum_values(fixture("sortmappings.good"));