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/readme.md b/readme.md index b0d1175..91ef9b1 100644 --- a/readme.md +++ b/readme.md @@ -16,10 +16,28 @@ 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 -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 +45,80 @@ 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 - } +`BaseArchRuleCollection` holds the rules that apply to any Java project. `CommonArchRuleCollection` +adds rules for Spring MVC controllers and for `SortMappings`, so implement it only in a project that +has them. + +The blacklists are mutable, so a project can drop an entry it disagrees with: + +```java + +static { + BlacklistClassesArchRule.BLACKLISTED_CLASSES.remove("net.datafaker.Faker"); } ``` -In the static block you can configure some blacklists provided by the base class. +The same applies to `ArchRuleConfig.TEST_CLASS_SUFFIXES` when a project introduces a new test type. + +### Rules your project has no code for + +Every rule tolerates a selection that comes up empty, so a rule simply passes on a project it does +not apply to. Whether a project has records, controllers, `@Store` classes or `@Nested` test classes +is the project's business, not something this library requires. + +That each rule can actually fail is guaranteed by a red test per rule in this repository, rather than +by making your build fail over code you do not have. An empty selection says nothing about whether a +rule's logic works. + +One case is a real problem though, and `analyzed_packages_must_contain_classes` covers it: if the +packages given to `@AnalyzeClasses` are mistyped or have moved, nothing is imported and every other +rule would pass without looking at a single class. That fails, once, with a message naming the cause. + +### Opting out + +Two annotations exempt a class from a specific rule. Neither is meta-annotated with ArchUnit's +`@ArchIgnore`: the ArchUnit JUnit engine resolves meta-annotations, so that would skip *every* +`@ArchTest` on the annotated class and report success rather than exempting it from one rule. + +| annotation | put it on | exempts from | +|---|---|---| +| `@ArchIgnoreNoProductionCounterpart` | a test class | needing a production class of the same name in the same package, and having its `@Nested` classes matched against production methods | +| `@ArchIgnoreGroupName` | a `@Nested` test class | needing a production method of the same name, for a class that only groups tests | + +Use `@ArchIgnoreNoProductionCounterpart` for a test named after the behaviour it describes rather than +after a production class. + +Both are read as meta-annotations, so a project declares its intent once on its own stereotype instead +of repeating the annotation on every class: ```java - static { - ArchitectureTestBase.BLACKLISTED_CLASSES.remove("net.datafaker.Faker"); + +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@ArchIgnoreNoProductionCounterpart +public @interface BusinessTest { } ``` +Annotating a single class directly still works — ArchUnit counts a direct annotation as +meta-annotated. + +The same applies to `@Disabled` and ArchUnit's `@ArchIgnore`, which these rules also honour: a +stereotype that carries either of them exempts every class using it. That matches how JUnit and the +ArchUnit engine themselves read those two annotations — a class whose tests do not run is not held to +naming rules — but it does mean a stereotype can exempt more than it appears to, so keep an eye on +what your own test annotations carry. + +Architecture tests need neither: any class in a package named `_architecture` is exempt from the +production-counterpart rule, alongside the existing `_support` and `_config` exclusions. Use the +annotation for the one-off that lives elsewhere. + ## Local Development To use this library as a local development dependency, you can simply refer to the version `BUILD-SNAPSHOT`. 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/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 new file mode 100644 index 0000000..22625f8 --- /dev/null +++ b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRule.java @@ -0,0 +1,26 @@ +package it.aboutbits.archunit.toolbox.rule.base; + +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.junit.ArchTest; +import org.jspecify.annotations.NullMarked; + +/// Checks that the analyzed packages contain any classes at all. +/// +/// Every other rule tolerates an empty selection, because whether a project has records, controllers +/// or `@Nested` test classes is the project's business and not something this library gets to require. +/// That leaves exactly one dangerous case: a mistyped or moved package in `@AnalyzeClasses` imports +/// nothing, and every rule then passes without looking at a single class. This rule is what turns that +/// into a failure, once, with a message that names the actual problem. +@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"}) +@NullMarked +public interface AnalyzedPackagesMustContainClassesArchRule { + @SuppressWarnings({"unused", "checkstyle:MethodName", "java:S100"}) + @ArchTest + default void analyzed_packages_must_contain_classes(JavaClasses classes) { + if (classes.isEmpty()) { + throw new AssertionError(""" + No classes were imported, so none of the architecture rules checked anything. + Verify the packages passed to @AnalyzeClasses."""); + } + } +} 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..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 @@ -12,6 +12,7 @@ import java.util.Set; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; +import static it.aboutbits.archunit.toolbox.util.CodeUnitUtil.describeKind; import static it.aboutbits.archunit.toolbox.util.LineNumberUtil.getLineNumber; @SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"}) @@ -63,6 +64,7 @@ public interface BlacklistAnnotationsArchRule { default void no_blacklisted_annotations_are_used(JavaClasses classes) { classes() .should(new NotUseBlacklistedAnnotations()) + .allowEmptyShould(true) .check(classes); } @@ -87,33 +89,36 @@ public void check(JavaClass javaClass, ConditionEvents events) { } } - // Check annotations on methods and their parameters - for (var method : javaClass.getMethods()) { - // Check method annotations - for (var annotation : method.getAnnotations()) { + // getCodeUnits() covers methods, constructors and the static initializer. getMethods() + // would miss constructors, and with them the most common position of all: a blacklisted + // annotation on a constructor parameter. + for (var codeUnit : javaClass.getCodeUnits()) { + for (var annotation : codeUnit.getAnnotations()) { if (BLACKLISTED_ANNOTATIONS.contains(annotation.getRawType().getFullName())) { var message = String.format( - "Method %s is annotated with blacklisted annotation @%s (%s.java:%d)", - method.getFullName(), + "%s %s is annotated with blacklisted annotation @%s (%s.java:%d)", + describeKind(codeUnit), + codeUnit.getFullName(), annotation.getRawType().getFullName(), javaClass.getSimpleName(), - getLineNumber(method) + getLineNumber(codeUnit) ); - events.add(SimpleConditionEvent.violated(method, message)); + events.add(SimpleConditionEvent.violated(codeUnit, message)); } } - // Check method parameter annotations - for (var parameter : method.getParameters()) { + + for (var parameter : codeUnit.getParameters()) { for (var annotation : parameter.getAnnotations()) { if (BLACKLISTED_ANNOTATIONS.contains(annotation.getRawType().getFullName())) { var message = String.format( - "Parameter %s of method %s is annotated with blacklisted annotation @%s (%s.java:%d)", + "Parameter %s of %s %s is annotated with blacklisted annotation @%s (%s.java:%d)", parameter.getIndex(), - method.getFullName(), + describeKind(codeUnit).toLowerCase(java.util.Locale.ROOT), + codeUnit.getFullName(), annotation.getRawType().getFullName(), javaClass.getSimpleName(), - getLineNumber(method) - ); // Parameter doesn't have its own SLOC, use method's + getLineNumber(codeUnit) + ); // Parameter doesn't have its own SLOC, use the code unit's events.add(SimpleConditionEvent.violated(parameter, message)); } } 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 863e875..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 @@ -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,10 +23,8 @@ 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.assertThrowsExactly", "org.junit.jupiter.api.Assertions.assertDoesNotThrow", // assertThat (allowed is only org.assertj.core.api.Assertions.assertThat) "org.assertj.core.api.AssertionsForClassTypes.assertThat", @@ -70,6 +69,7 @@ public interface BlacklistMethodsArchRule { default void no_blacklisted_methods_are_used(JavaClasses classes) { classes() .should(new NotUseBlacklistedMethods()) + .allowEmptyShould(true) .check(classes); } @@ -80,47 +80,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/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 79b99f4..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 @@ -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"}) @@ -19,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); } @@ -33,44 +35,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/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 6481dd7..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 @@ -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,18 +17,26 @@ 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) + ")$") + classes().that(TestClassNames.testClasses()) .and() - .doNotHaveSimpleName("ArchitectureTest") + .areNotMetaAnnotatedWith(org.junit.jupiter.api.Disabled.class) .and() - .areNotAnnotatedWith(org.junit.jupiter.api.Disabled.class) + .areNotMetaAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) .and() - .areNotAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.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() - .areNotAnnotatedWith(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); @@ -44,23 +52,18 @@ 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(); - - 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/TestClassVisibilityArchRule.java b/src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java index d601051..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 @@ -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,12 +14,12 @@ 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() .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..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 @@ -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,12 +21,17 @@ 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) + .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. + */ + .areNotMetaAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart.class) .should(new HaveNestedClassesThatHaveAMatchingProductionMethodName(classes)) .allowEmptyShould(true) .check(classes); @@ -48,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()); @@ -78,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") ) ) { @@ -113,16 +118,19 @@ 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("") ); - var productionClassOptional = allClasses.stream() - .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. + * + * 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 (!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, @@ -130,10 +138,8 @@ public void check(JavaClass testClass, ConditionEvents events) { productionClassName ); events.add(SimpleConditionEvent.violated(nestedClass, message)); - } - - if (productionClassOptional.isPresent()) { - var productionClass = productionClassOptional.get(); + } else { + var productionClass = allClasses.get(productionClassName); var methodExists = productionClass.getMethods() .stream() 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..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 @@ -44,6 +44,7 @@ public boolean test(JavaAnnotation javaAnnotation) { } }) .should(new BeSecurityTested()) + .allowEmptyShould(true) .check(classes); } @@ -80,17 +81,18 @@ 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) + && !clazz.isMetaAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class) + && !clazz.isMetaAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class) ); if (!nestedMethodTestClassFound) { @@ -106,5 +108,13 @@ 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..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 @@ -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) { @@ -33,7 +35,11 @@ default void sort_mappings_cover_all_sort_enum_values(JavaClasses classes) { .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 +47,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..046d4fa 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,17 @@ 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. - *

- * This annotation should be used on a @Nested test class. - *

- */ +/// 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) -@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..bcbb748 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,17 @@ 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. - *

- * This annotation should be used on a @Nested test class. - *

- */ +/// 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) -@ArchIgnore(reason = "This test class has no matching counterpart in the production code.") 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 new file mode 100644 index 0000000..44e08af --- /dev/null +++ b/src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java @@ -0,0 +1,27 @@ +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 `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"; + 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..597316c --- /dev/null +++ b/src/main/java/it/aboutbits/archunit/toolbox/util/TestClassNames.java @@ -0,0 +1,81 @@ +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 [#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 `.+` 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 [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 [#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. `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: + /// `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/fixture/barren/PlainClass.java b/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java new file mode 100644 index 0000000..05584e1 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/barren/PlainClass.java @@ -0,0 +1,12 @@ +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..025c4bf --- /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..32e1c60 --- /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/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/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..20a9a77 --- /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..c1729bc --- /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..0d8c7cd --- /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/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/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/nestedclassname/goodoptout/ScenarioTest.java b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java new file mode 100644 index 0000000..c9f1769 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/nestedclassname/goodoptout/ScenarioTest.java @@ -0,0 +1,13 @@ +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..27b4936 --- /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..c0e0235 --- /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/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/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..2d7e652 --- /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..a77ac72 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/badprefix/WidgetControllerSecurityTest.java @@ -0,0 +1,11 @@ +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..54e427e --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/fixture/securitytested/goodnestedgroup/WidgetControllerSecurityTest.java @@ -0,0 +1,16 @@ +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..2f7e6c6 --- /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..13b73f2 --- /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/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/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..852094e --- /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/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/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/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/goodoptout/ScenarioTest.java b/src/test/java/it/aboutbits/archunit/fixture/testclasspackage/goodoptout/ScenarioTest.java new file mode 100644 index 0000000..65b2089 --- /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..d2fb162 --- /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/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/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/ArchitectureTest.java b/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java index 5acf9c7..3a13f6c 100644 --- a/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java +++ b/src/test/java/it/aboutbits/archunit/toolbox/ArchitectureTest.java @@ -2,14 +2,21 @@ 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..5ff9438 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/EmptySelectionTest.java @@ -0,0 +1,77 @@ +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 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. +@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)), + 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_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"); + + 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) { + return rule; + } + + private static final class Rules implements BaseArchRuleCollection, CommonArchRuleCollection { + } +} 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..88dcc06 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/RuleEvaluation.java @@ -0,0 +1,67 @@ +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 `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; + } + + /// 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. + 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/AnalyzedPackagesMustContainClassesArchRuleTest.java b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java new file mode 100644 index 0000000..e40f832 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/AnalyzedPackagesMustContainClassesArchRuleTest.java @@ -0,0 +1,33 @@ +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(); + } +} 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..5607455 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistAnnotationsArchRuleTest.java @@ -0,0 +1,62 @@ +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..e61a30e --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/BlacklistMethodsArchRuleTest.java @@ -0,0 +1,91 @@ +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")); + } + + @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 + 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..b64a41c --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/RecordPropertiesMustBeAccessedViaAccessorArchRuleTest.java @@ -0,0 +1,37 @@ +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..bfccf9c --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRuleTest.java @@ -0,0 +1,61 @@ +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")); + } + + /// 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/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..1f57f5c --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/base/TestNestedClassMatchNameArchRuleTest.java @@ -0,0 +1,77 @@ +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"); + } + + /// 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")); + } + + @Test + 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/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..807cd25 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTestedTest.java @@ -0,0 +1,74 @@ +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")); + } + + /// 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"); + } +} 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..c4169a2 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/rule/common/SortMappingsExhaustiveArchRuleTest.java @@ -0,0 +1,53 @@ +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"); + } + + /// 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")); + } +} 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..d6bd59b --- /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..85e666b --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/support/ArchIgnoreNoProductionCounterpartTest.java @@ -0,0 +1,21 @@ +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(); + } +} 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/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/archunit/toolbox/util/TestClassNamesTest.java b/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java new file mode 100644 index 0000000..bff3e77 --- /dev/null +++ b/src/test/java/it/aboutbits/archunit/toolbox/util/TestClassNamesTest.java @@ -0,0 +1,59 @@ +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)$"); + } +} 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..ed1c0f1 --- /dev/null +++ b/src/test/java/it/aboutbits/springboot/toolbox/archunit/ArchAllowDirectAccess.java @@ -0,0 +1,14 @@ +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..703700b --- /dev/null +++ b/src/test/java/it/aboutbits/springboot/toolbox/persistence/SortMappings.java @@ -0,0 +1,16 @@ +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..88da7de --- /dev/null +++ b/src/test/java/it/aboutbits/springboot/toolbox/stereotype/Store.java @@ -0,0 +1,14 @@ +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..abfd031 --- /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..afc06e7 --- /dev/null +++ b/src/test/java/org/junit/Ignore.java @@ -0,0 +1,13 @@ +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 { +}