Skip to content

Commit e9af6da

Browse files
SirCotareclaude
andcommitted
allow rules a project has no code for, and guard the import instead
Failing on an empty selection was the wrong call. Seven of the eight rules that narrow their input have a legitimate empty case - no records, no controllers, no @Store classes, nobody using @nested - and one of them described this repository before this branch: a project whose only test is the ArchitectureTest has no @test methods at all. Whether that code exists is the project's business. allowEmptyShould(true) was never what protected against a dead rule either. It fires on what a consumer's code happens to contain and says nothing about whether a rule's logic works: the counterpart rule that started this had a perfectly non-empty selection and a broken condition. The guarantee is the red test per rule in this project, so all 13 rules now tolerate an empty selection. That leaves one case worth failing on. A mistyped or moved package in @AnalyzeClasses imports nothing, and every rule then passes without looking at a single class. AnalyzedPackagesMustContainClassesArchRule turns that into one failure naming the cause, instead of five rules passing silently and eight reporting that they checked nothing. It is in both collections, so it applies wherever the toolbox is used. EmptySelectionTest is inverted to match: it now pins that every rule accepts a project with nothing for it to check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent d52ba67 commit e9af6da

20 files changed

Lines changed: 125 additions & 19 deletions

readme.md

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,19 @@ static {
4747

4848
The same applies to `ArchRuleConfig.TEST_CLASS_SUFFIXES` when a project introduces a new test type.
4949

50-
### Every rule must be able to fail
50+
### Rules your project has no code for
5151

52-
No rule uses `allowEmptyShould(true)`. A rule that selects nothing would otherwise report success,
53-
which is indistinguishable from a rule that is satisfied - and that is how a broken rule survives
54-
unnoticed. So a rule whose selection comes up empty fails, and the fix is either to remove the rule
55-
collection you do not need or to add the code it is meant to check.
52+
Every rule tolerates a selection that comes up empty, so a rule simply passes on a project it does
53+
not apply to. Whether a project has records, controllers, `@Store` classes or `@Nested` test classes
54+
is the project's business, not something this library requires.
55+
56+
That each rule can actually fail is guaranteed by a red test per rule in this repository, rather than
57+
by making your build fail over code you do not have. An empty selection says nothing about whether a
58+
rule's logic works.
59+
60+
One case is a real problem though, and `analyzed_packages_must_contain_classes` covers it: if the
61+
packages given to `@AnalyzeClasses` are mistyped or have moved, nothing is imported and every other
62+
rule would pass without looking at a single class. That fails, once, with a message naming the cause.
5663

5764
### Opting out
5865

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package it.aboutbits.archunit.toolbox;
22

3+
import it.aboutbits.archunit.toolbox.rule.base.AnalyzedPackagesMustContainClassesArchRule;
34
import it.aboutbits.archunit.toolbox.rule.base.BlacklistAnnotationsArchRule;
45
import it.aboutbits.archunit.toolbox.rule.base.BlacklistClassesArchRule;
56
import it.aboutbits.archunit.toolbox.rule.base.BlacklistMethodsArchRule;
@@ -15,6 +16,7 @@
1516

1617
@NullMarked
1718
public interface BaseArchRuleCollection extends
19+
AnalyzedPackagesMustContainClassesArchRule,
1820
BlacklistAnnotationsArchRule,
1921
BlacklistClassesArchRule,
2022
BlacklistMethodsArchRule,
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package it.aboutbits.archunit.toolbox;
22

3+
import it.aboutbits.archunit.toolbox.rule.base.AnalyzedPackagesMustContainClassesArchRule;
34
import it.aboutbits.archunit.toolbox.rule.common.ControllerRequestMappingsMustBeSecurityTested;
45
import it.aboutbits.archunit.toolbox.rule.common.SortMappingsExhaustiveArchRule;
56
import org.jspecify.annotations.NullMarked;
67

78
@NullMarked
89
public interface CommonArchRuleCollection extends
10+
AnalyzedPackagesMustContainClassesArchRule,
911
ControllerRequestMappingsMustBeSecurityTested,
1012
SortMappingsExhaustiveArchRule {
1113
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package it.aboutbits.archunit.toolbox.rule.base;
2+
3+
import com.tngtech.archunit.core.domain.JavaClasses;
4+
import com.tngtech.archunit.junit.ArchTest;
5+
import org.jspecify.annotations.NullMarked;
6+
7+
/**
8+
* Checks that the analyzed packages contain any classes at all.
9+
* <p>
10+
* Every other rule tolerates an empty selection, because whether a project has records, controllers
11+
* or @Nested test classes is the project's business and not something this library gets to require.
12+
* That leaves exactly one dangerous case: a mistyped or moved package in @AnalyzeClasses imports
13+
* nothing, and every rule then passes without looking at a single class. This rule is what turns that
14+
* into a failure, once, with a message that names the actual problem.
15+
* </p>
16+
*/
17+
@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
18+
@NullMarked
19+
public interface AnalyzedPackagesMustContainClassesArchRule {
20+
@SuppressWarnings({"unused", "checkstyle:MethodName", "java:S100"})
21+
@ArchTest
22+
default void analyzed_packages_must_contain_classes(JavaClasses classes) {
23+
if (classes.isEmpty()) {
24+
throw new AssertionError(
25+
"No classes were imported, so none of the architecture rules checked anything. "
26+
+ "Verify the packages passed to @AnalyzeClasses."
27+
);
28+
}
29+
}
30+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public interface BlacklistAnnotationsArchRule {
6464
default void no_blacklisted_annotations_are_used(JavaClasses classes) {
6565
classes()
6666
.should(new NotUseBlacklistedAnnotations())
67+
.allowEmptyShould(true)
6768
.check(classes);
6869
}
6970

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public boolean test(JavaClass javaClass) {
3535
}
3636
}
3737
)
38+
.allowEmptyShould(true)
3839
.check(classes);
3940
}
4041
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ public interface BlacklistMethodsArchRule {
6868
default void no_blacklisted_methods_are_used(JavaClasses classes) {
6969
classes()
7070
.should(new NotUseBlacklistedMethods())
71+
.allowEmptyShould(true)
7172
.check(classes);
7273
}
7374

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ default void top_level_classes_must_be_annotated_with_jspecify(JavaClasses class
2121
.beAnnotatedWith(org.jspecify.annotations.NullMarked.class)
2222
.orShould()
2323
.beAnnotatedWith(org.jspecify.annotations.NullUnmarked.class)
24+
.allowEmptyShould(true)
2425
.check(classes);
2526
}
2627
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public interface NoSystemOutOrErrArchRule {
2020
default void no_system_out_or_err_is_used(JavaClasses classes) {
2121
classes()
2222
.should(new NotUseSystemOutOrErr())
23+
.allowEmptyShould(true)
2324
.check(classes);
2425
}
2526

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,6 @@ public void check(JavaField field, ConditionEvents events) {
4545
}
4646
}
4747
}
48-
});
48+
})
49+
.allowEmptyShould(true);
4950
}

0 commit comments

Comments
 (0)