Skip to content

Commit 0aef3db

Browse files
SirCotareclaude
SirCotare
and
claude
committed
match test class names and code unit bodies in one shared place
Two root causes, each shared by several rules. The suffix regex was hand-built in five places, in two shapes, against two different inputs. The counterpart rule's condition rebuilt it without the leading ".+" and matched it against getSimpleName() with String.matches, which anchors both ends - so only a class named exactly "Test" got past the guard and every real test class returned before the counterpart lookup ever ran. The rule has therefore never reported anything. TestClassNames now owns the pattern, the stripping and the selection predicate. The redundant guard in the condition is gone rather than corrected: the selection already guarantees the suffix, and re-deriving it in the condition is what let the two drift apart. Selection also matches the simple name and requires a non-empty production name to be left over, so it can no longer accept a name the stripping turns into nothing ("CacheTest" matched with "Cache" as the ".+"). Three rules walked getMethods(), which excludes constructors - and with them every instance field initializer, since that is compiled into the constructor. So a blacklisted annotation on a constructor parameter, the canonical Lombok position, was invisible. All three now walk getCodeUnits(), which also subsumes the separate static-initializer branches two of them carried. Also drops three blacklist entries naming AssertJ methods that do not exist (assertThrows, assertThrowsExactly, assertDoesNotThrow are JUnit's). An entry that can never match reads as coverage without providing any. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 052069a commit 0aef3db

9 files changed

Lines changed: 293 additions & 101 deletions

File tree

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.util.Set;
1313

1414
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
15+
import static it.aboutbits.archunit.toolbox.util.CodeUnitUtil.describeKind;
1516
import static it.aboutbits.archunit.toolbox.util.LineNumberUtil.getLineNumber;
1617

1718
@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
@@ -87,33 +88,36 @@ public void check(JavaClass javaClass, ConditionEvents events) {
8788
}
8889
}
8990

90-
// Check annotations on methods and their parameters
91-
for (var method : javaClass.getMethods()) {
92-
// Check method annotations
93-
for (var annotation : method.getAnnotations()) {
91+
// getCodeUnits() covers methods, constructors and the static initializer. getMethods()
92+
// would miss constructors, and with them the most common position of all: a blacklisted
93+
// annotation on a constructor parameter.
94+
for (var codeUnit : javaClass.getCodeUnits()) {
95+
for (var annotation : codeUnit.getAnnotations()) {
9496
if (BLACKLISTED_ANNOTATIONS.contains(annotation.getRawType().getFullName())) {
9597
var message = String.format(
96-
"Method %s is annotated with blacklisted annotation @%s (%s.java:%d)",
97-
method.getFullName(),
98+
"%s %s is annotated with blacklisted annotation @%s (%s.java:%d)",
99+
describeKind(codeUnit),
100+
codeUnit.getFullName(),
98101
annotation.getRawType().getFullName(),
99102
javaClass.getSimpleName(),
100-
getLineNumber(method)
103+
getLineNumber(codeUnit)
101104
);
102-
events.add(SimpleConditionEvent.violated(method, message));
105+
events.add(SimpleConditionEvent.violated(codeUnit, message));
103106
}
104107
}
105-
// Check method parameter annotations
106-
for (var parameter : method.getParameters()) {
108+
109+
for (var parameter : codeUnit.getParameters()) {
107110
for (var annotation : parameter.getAnnotations()) {
108111
if (BLACKLISTED_ANNOTATIONS.contains(annotation.getRawType().getFullName())) {
109112
var message = String.format(
110-
"Parameter %s of method %s is annotated with blacklisted annotation @%s (%s.java:%d)",
113+
"Parameter %s of %s %s is annotated with blacklisted annotation @%s (%s.java:%d)",
111114
parameter.getIndex(),
112-
method.getFullName(),
115+
describeKind(codeUnit).toLowerCase(java.util.Locale.ROOT),
116+
codeUnit.getFullName(),
113117
annotation.getRawType().getFullName(),
114118
javaClass.getSimpleName(),
115-
getLineNumber(method)
116-
); // Parameter doesn't have its own SLOC, use method's
119+
getLineNumber(codeUnit)
120+
); // Parameter doesn't have its own SLOC, use the code unit's
117121
events.add(SimpleConditionEvent.violated(parameter, message));
118122
}
119123
}

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

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.util.Set;
1313

1414
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
15+
import static it.aboutbits.archunit.toolbox.util.CodeUnitUtil.describeKind;
1516
import static it.aboutbits.archunit.toolbox.util.LineNumberUtil.getLineNumber;
1617

1718
@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
@@ -22,9 +23,6 @@ public interface BlacklistMethodsArchRule {
2223
Set.of(
2324
// We should use `assertThatExceptionOfType(...).isThrownBy(...)` instead of `assertThatThrownBy(...)`
2425
"org.assertj.core.api.Assertions.assertThatThrownBy",
25-
"org.assertj.core.api.Assertions.assertThrows",
26-
"org.assertj.core.api.Assertions.assertThrowsExactly",
27-
"org.assertj.core.api.Assertions.assertDoesNotThrow",
2826
"org.junit.jupiter.api.Assertions.assertThrows",
2927
"org.junit.jupiter.api.Assertions.assertDoesNotThrow",
3028
// assertThat (allowed is only org.assertj.core.api.Assertions.assertThat)
@@ -80,47 +78,30 @@ public NotUseBlacklistedMethods() {
8078

8179
@Override
8280
public void check(JavaClass javaClass, ConditionEvents events) {
83-
// Check all method calls from this class
84-
for (var method : javaClass.getMethods()) {
85-
for (var methodCall : method.getMethodCallsFromSelf()) {
81+
// getCodeUnits() covers methods, constructors and the static initializer. getMethods()
82+
// would miss constructors, and with them every instance field initializer.
83+
for (var codeUnit : javaClass.getCodeUnits()) {
84+
for (var methodCall : codeUnit.getMethodCallsFromSelf()) {
8685
var fullMethodName = "%s.%s".formatted(
8786
methodCall.getTargetOwner().getFullName(),
8887
methodCall.getTarget().getName()
8988
);
9089

91-
if (BLACKLISTED_METHODS.contains(fullMethodName)) {
92-
var message = String.format(
93-
"Method %s calls blacklisted method %s (%s.java:%d)",
94-
method.getFullName(),
95-
fullMethodName,
96-
javaClass.getSimpleName(),
97-
getLineNumber(methodCall)
98-
);
99-
events.add(SimpleConditionEvent.violated(method, message));
90+
if (!BLACKLISTED_METHODS.contains(fullMethodName)) {
91+
continue;
10092
}
101-
}
102-
}
10393

104-
// Check static initializers for method calls
105-
javaClass.getStaticInitializer().ifPresent(staticInitializer -> {
106-
for (var methodCall : staticInitializer.getMethodCallsFromSelf()) {
107-
var fullMethodName = "%s.%s".formatted(
108-
methodCall.getTargetOwner().getFullName(),
109-
methodCall.getTarget().getName()
94+
var message = String.format(
95+
"%s %s calls blacklisted method %s (%s.java:%d)",
96+
describeKind(codeUnit),
97+
codeUnit.getFullName(),
98+
fullMethodName,
99+
javaClass.getSimpleName(),
100+
getLineNumber(methodCall)
110101
);
111-
112-
if (BLACKLISTED_METHODS.contains(fullMethodName)) {
113-
var message = String.format(
114-
"Static initializer in %s calls blacklisted method %s (%s.java:%d)",
115-
javaClass.getFullName(),
116-
fullMethodName,
117-
javaClass.getSimpleName(),
118-
getLineNumber(methodCall)
119-
);
120-
events.add(SimpleConditionEvent.violated(staticInitializer, message));
121-
}
102+
events.add(SimpleConditionEvent.violated(codeUnit, message));
122103
}
123-
});
104+
}
124105
}
125106
}
126107
}

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

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.jspecify.annotations.NullMarked;
1010

1111
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
12+
import static it.aboutbits.archunit.toolbox.util.CodeUnitUtil.describeKind;
1213
import static it.aboutbits.archunit.toolbox.util.LineNumberUtil.getLineNumber;
1314

1415
@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
@@ -33,44 +34,27 @@ public NotUseSystemOutOrErr() {
3334

3435
@Override
3536
public void check(JavaClass javaClass, ConditionEvents events) {
36-
checkCodeUnits(javaClass, events);
37-
javaClass.getStaticInitializer().ifPresent(staticInitializer -> {
38-
for (var fieldAccess : staticInitializer.getFieldAccesses()) {
39-
if (isSystemOutOrErr(
37+
// getCodeUnits() covers methods, constructors and the static initializer. getMethods()
38+
// would miss constructors, and with them every instance field initializer.
39+
for (var codeUnit : javaClass.getCodeUnits()) {
40+
for (var fieldAccess : codeUnit.getFieldAccesses()) {
41+
if (!isSystemOutOrErr(
4042
fieldAccess.getTargetOwner().getFullName(),
4143
fieldAccess.getTarget().getName()
4244
)) {
43-
var message = String.format(
44-
"Static initializer in %s accesses %s.%s (%s.java:%d)",
45-
javaClass.getFullName(),
46-
SYSTEM_CLASS,
47-
fieldAccess.getTarget().getName(),
48-
javaClass.getSimpleName(),
49-
getLineNumber(fieldAccess)
50-
);
51-
events.add(SimpleConditionEvent.violated(staticInitializer, message));
45+
continue;
5246
}
53-
}
54-
});
55-
}
5647

57-
private void checkCodeUnits(JavaClass javaClass, ConditionEvents events) {
58-
for (var method : javaClass.getMethods()) {
59-
for (var fieldAccess : method.getFieldAccesses()) {
60-
if (isSystemOutOrErr(
61-
fieldAccess.getTargetOwner().getFullName(),
62-
fieldAccess.getTarget().getName()
63-
)) {
64-
var message = String.format(
65-
"Method %s accesses %s.%s (%s.java:%d)",
66-
method.getFullName(),
67-
SYSTEM_CLASS,
68-
fieldAccess.getTarget().getName(),
69-
javaClass.getSimpleName(),
70-
getLineNumber(fieldAccess)
71-
);
72-
events.add(SimpleConditionEvent.violated(method, message));
73-
}
48+
var message = String.format(
49+
"%s %s accesses %s.%s (%s.java:%d)",
50+
describeKind(codeUnit),
51+
codeUnit.getFullName(),
52+
SYSTEM_CLASS,
53+
fieldAccess.getTarget().getName(),
54+
javaClass.getSimpleName(),
55+
getLineNumber(fieldAccess)
56+
);
57+
events.add(SimpleConditionEvent.violated(codeUnit, message));
7458
}
7559
}
7660
}

‎src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassInCorrectPackageArchRule.java‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,18 @@
66
import com.tngtech.archunit.lang.ArchCondition;
77
import com.tngtech.archunit.lang.ConditionEvents;
88
import com.tngtech.archunit.lang.SimpleConditionEvent;
9+
import it.aboutbits.archunit.toolbox.util.TestClassNames;
910
import org.jspecify.annotations.NullMarked;
1011

1112
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
12-
import static it.aboutbits.archunit.toolbox.config.ArchRuleConfig.TEST_CLASS_SUFFIXES;
1313

1414
@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
1515
@NullMarked
1616
public interface TestClassInCorrectPackageArchRule {
1717
@SuppressWarnings({"unused", "checkstyle:MethodName", "java:S100"})
1818
@ArchTest
1919
default void test_classes_should_be_in_the_same_package_as_their_production_code(JavaClasses classes) {
20-
classes().that()
21-
.haveNameMatching(".+(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$")
22-
.and()
23-
.doNotHaveSimpleName("ArchitectureTest")
20+
classes().that(TestClassNames.testClasses())
2421
.and()
2522
.areNotAnnotatedWith(org.junit.jupiter.api.Disabled.class)
2623
.and()
@@ -30,7 +27,6 @@ default void test_classes_should_be_in_the_same_package_as_their_production_code
3027
.and()
3128
.resideOutsideOfPackages(".._support..", ".._config..")
3229
.should(new BeInTheSamePackageAsTheProductionClass(classes))
33-
.allowEmptyShould(true)
3430
.check(classes);
3531
}
3632

@@ -44,18 +40,15 @@ public BeInTheSamePackageAsTheProductionClass(JavaClasses allClasses) {
4440

4541
@Override
4642
public void check(JavaClass testClass, ConditionEvents events) {
47-
var testClassSuffixRegex = "(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$";
48-
49-
var testClassName = testClass.getSimpleName();
50-
if (!testClassName.matches(testClassSuffixRegex)) {
51-
return;
52-
}
53-
54-
// Derive the production class name
55-
var productionClassSimpleName = testClassName.replaceAll(testClassSuffixRegex, "");
43+
/*
44+
* No suffix guard here on purpose. The selection above already guarantees the suffix,
45+
* and re-deriving it in the condition is what previously disabled this rule outright:
46+
* the guard rebuilt the regex without the leading ".+", and String.matches anchors both
47+
* ends, so every test class returned before ever looking for its production class.
48+
*/
49+
var productionClassSimpleName = TestClassNames.productionClassSimpleName(testClass.getSimpleName());
5650
var productionClassFullName = testClass.getPackageName() + "." + productionClassSimpleName;
5751

58-
// Check if the production class exists in the same package
5952
var productionClass = allClasses.stream()
6053
.filter(clazz -> clazz.getFullName().equals(productionClassFullName))
6154
.findFirst();

‎src/main/java/it/aboutbits/archunit/toolbox/rule/base/TestClassVisibilityArchRule.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
import com.tngtech.archunit.core.domain.JavaClasses;
44
import com.tngtech.archunit.junit.ArchTest;
5+
import it.aboutbits.archunit.toolbox.util.TestClassNames;
56
import org.jspecify.annotations.NullMarked;
67

78
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
8-
import static it.aboutbits.archunit.toolbox.config.ArchRuleConfig.TEST_CLASS_SUFFIXES;
99

1010
@SuppressWarnings({"checkstyle:InterfaceIsType", "java:S1214"})
1111
@NullMarked
@@ -14,8 +14,7 @@ public interface TestClassVisibilityArchRule {
1414
@ArchTest
1515
default void test_classes_must_be_package_private(JavaClasses classes) {
1616
classes()
17-
.that()
18-
.haveNameMatching(".+(" + String.join("|", TEST_CLASS_SUFFIXES) + ")$")
17+
.that(TestClassNames.testClasses())
1918
.and()
2019
.resideOutsideOfPackages(".._support..", ".._config..")
2120
.should()

‎src/main/java/it/aboutbits/archunit/toolbox/util/CodeUnitUtil.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package it.aboutbits.archunit.toolbox.util;
2+
3+
import com.tngtech.archunit.core.domain.JavaCodeUnit;
4+
import com.tngtech.archunit.core.domain.JavaConstructor;
5+
import com.tngtech.archunit.core.domain.JavaMethod;
6+
import com.tngtech.archunit.core.domain.JavaStaticInitializer;
7+
import org.jspecify.annotations.NullMarked;
8+
9+
@NullMarked
10+
public final class CodeUnitUtil {
11+
private CodeUnitUtil() {
12+
}
13+
14+
/**
15+
* Human readable kind of a code unit, for violation messages.
16+
* <p>
17+
* Rules that inspect bodies must iterate {@code getCodeUnits()} rather than {@code getMethods()}:
18+
* the latter excludes constructors, and an instance field initializer is compiled into the
19+
* constructor, so both are invisible to a rule that only looks at methods.
20+
* </p>
21+
*/
22+
public static String describeKind(JavaCodeUnit codeUnit) {
23+
return switch (codeUnit) {
24+
case JavaMethod _ -> "Method";
25+
case JavaConstructor _ -> "Constructor";
26+
case JavaStaticInitializer _ -> "Static initializer";
27+
default -> "Code unit";
28+
};
29+
}
30+
}

0 commit comments

Comments
 (0)