Skip to content

Commit 630fab9

Browse files
SirCotareclaude
andcommitted
report what a rule cannot verify instead of passing
Removes the remaining ways a rule stayed quiet. allowEmptyShould(true) is gone everywhere. A rule that selects nothing reported success, indistinguishable from a rule that is satisfied, which is what let a dead rule survive. EmptySelectionTest pins this for all eight rules that narrow their input. Consequence to be aware of: CommonArchRuleCollection now fails in a project with no controllers or no @Store classes, so implement it only where those exist. The @nested name rule skipped a nested class silently whenever the production class was missing and the nested class was not inside a group - so the case with nothing to compare against was the one case never reported. It now reports, and in exchange honours @ArchIgnoreNoProductionCounterpart: a test class that declares it has no production counterpart has no production methods either. The security-test rule accepted any @nested class *starting with* the method name, so getAll() counted as covered by GetAllArchived. It now requires the name to match exactly or to continue with "$", which keeps grouped @nested classes working. The SortMappings rule swallowed every failure to read a field into a log.warn - and the build has no SLF4J provider, so the warning was discarded outright. A non-static field, an unreadable field, an unresolvable enum type and a value that is not a Map are all violations now. Non-static in particular is reported as such, since it can never be validated. Finally, @ArchIgnoreNoProductionCounterpart and @ArchIgnoreGroupName no longer meta-annotate ArchUnit's @ArchIgnore. The ArchUnit JUnit engine resolves meta-annotations, so annotating a test class skipped *every* @archtest on it and still reported BUILD SUCCESS - verified: putting it on this project's own ArchitectureTest turned 11 rules into 11 skips. Both annotations are read by their own type, so the meta-annotation bought nothing. ArchitectureTest now carries it in place of the name that was hardcoded into the counterpart rule, which also keeps the annotation exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 0aef3db commit 630fab9

11 files changed

Lines changed: 303 additions & 167 deletions

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ public boolean test(JavaClass javaClass) {
3232
})
3333
.should()
3434
.bePackagePrivate()
35-
.allowEmptyShould(true)
3635
.check(classes);
3736
}
3837
}

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

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
import com.tngtech.archunit.lang.ArchCondition;
88
import com.tngtech.archunit.lang.ConditionEvents;
99
import com.tngtech.archunit.lang.SimpleConditionEvent;
10+
import it.aboutbits.archunit.toolbox.util.TestClassNames;
1011
import org.jspecify.annotations.NullMarked;
1112

1213
import java.util.stream.Collectors;
1314

1415
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
15-
import static it.aboutbits.archunit.toolbox.config.ArchRuleConfig.TEST_CLASS_SUFFIXES;
1616
import static it.aboutbits.archunit.toolbox.util.LineNumberUtil.getLineNumber;
1717

1818
@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
@@ -21,14 +21,18 @@ public interface TestNestedClassMatchNameArchRule {
2121
@SuppressWarnings({"unused", "checkstyle:MethodName", "java:S100"})
2222
@ArchTest
2323
default void nested_test_classes_have_matching_production_method_name(JavaClasses classes) {
24-
classes().that()
25-
.haveNameMatching(".+(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$")
24+
classes().that(TestClassNames.testClasses())
2625
.and()
2726
.areNotAnnotatedWith(org.junit.jupiter.api.Disabled.class)
2827
.and()
2928
.areNotAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class)
29+
.and()
30+
/*
31+
* A test class that declares it has no production counterpart has no production
32+
* methods to match its @Nested classes against either.
33+
*/
34+
.areNotAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreNoProductionCounterpart.class)
3035
.should(new HaveNestedClassesThatHaveAMatchingProductionMethodName(classes))
31-
.allowEmptyShould(true)
3236
.check(classes);
3337
}
3438

@@ -113,26 +117,28 @@ public void check(JavaClass testClass, ConditionEvents events) {
113117

114118
var productionClassName = "%s.%s%s".formatted(
115119
testClass.getPackageName(),
116-
testClass.getSimpleName()
117-
.replaceAll("(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$", ""),
120+
TestClassNames.productionClassSimpleName(testClass.getSimpleName()),
118121
enclosingClassSuffix.orElse("")
119122
);
120123

121124
var productionClassOptional = allClasses.stream()
122125
.filter(clazz -> clazz.getFullName().equals(productionClassName))
123126
.findFirst();
124127

125-
if (productionClassOptional.isEmpty() && enclosingClassSuffix.isPresent()) {
128+
/*
129+
* Reported whether or not the @Nested class is inside a @Nested group. Without a
130+
* production class there is nothing to match the name against, so staying silent
131+
* here means the @Nested class is never checked at all.
132+
*/
133+
if (productionClassOptional.isEmpty()) {
126134
var message = "The @Nested test class <%s> (%s.java:%s)%ndoes not have a matching production class <%s>".formatted(
127135
nestedClass.getName(),
128136
nestedClassBaseClassSimpleName,
129137
nestedClassLineNumber,
130138
productionClassName
131139
);
132140
events.add(SimpleConditionEvent.violated(nestedClass, message));
133-
}
134-
135-
if (productionClassOptional.isPresent()) {
141+
} else {
136142
var productionClass = productionClassOptional.get();
137143

138144
var methodExists = productionClass.getMethods()

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ default void nested_test_classes_must_be_package_private(JavaClasses classes) {
1717
.areAnnotatedWith(org.junit.jupiter.api.Nested.class)
1818
.should()
1919
.bePackagePrivate()
20-
.allowEmptyShould(true)
2120
.check(classes);
2221
}
2322
}

src/main/java/it/aboutbits/archunit/toolbox/rule/common/ControllerRequestMappingsMustBeSecurityTested.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,15 @@ public void check(JavaMethod method, ConditionEvents events) {
8080
return;
8181
}
8282

83+
var expectedNestedClassName = "%s$%s".formatted(
84+
securityTestClass.getName(),
85+
expectedNestedMethodClassName
86+
);
87+
8388
var nestedMethodTestClassFound = securityTestClass.getPackage()
8489
.getClasses()
8590
.stream()
86-
.anyMatch(clazz -> clazz.getName()
87-
.startsWith("%s$%s".formatted(
88-
securityTestClass.getName(),
89-
expectedNestedMethodClassName
90-
))
91+
.anyMatch(clazz -> isExpectedNestedClass(clazz.getName(), expectedNestedClassName)
9192
&& clazz.isAnnotatedWith(org.junit.jupiter.api.Nested.class)
9293
&& !clazz.isAnnotatedWith(com.tngtech.archunit.junit.ArchIgnore.class)
9394
&& !clazz.isAnnotatedWith(it.aboutbits.archunit.toolbox.support.ArchIgnoreGroupName.class)
@@ -106,5 +107,15 @@ public void check(JavaMethod method, ConditionEvents events) {
106107
));
107108
}
108109
}
110+
111+
/*
112+
* The @Nested class named after the controller method, or a @Nested class grouped inside it
113+
* (GetAll$WhenAdmin). Matching on a bare prefix would also accept an unrelated longer
114+
* sibling, so getAll() would count as covered by a @Nested class named GetAllArchived.
115+
*/
116+
private static boolean isExpectedNestedClass(String candidateName, String expectedName) {
117+
return candidateName.equals(expectedName)
118+
|| candidateName.startsWith(expectedName + "$");
119+
}
109120
}
110121
}

0 commit comments

Comments
 (0)