Skip to content

Commit 49acd3f

Browse files
marcphilippmarcphilipp
committed
Generate deprecation issue for private lifecycle methods
Issue: #242
1 parent 050c132 commit 49acd3f

File tree

2 files changed

+82
-44
lines changed

2 files changed

+82
-44
lines changed

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java

+34-13
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.lang.annotation.Annotation;
1717
import java.lang.reflect.Method;
1818
import java.util.List;
19+
import java.util.function.Consumer;
1920
import java.util.function.Function;
2021
import java.util.function.Predicate;
2122

@@ -87,6 +88,7 @@ private static List<Method> findMethodsAndCheckVoidReturnType(Class<?> testClass
8788
DiscoveryIssueReporter issueReporter, Predicate<? super Method> additionalCondition) {
8889

8990
return findAnnotatedMethods(testClass, annotationType, traversalMode).stream() //
91+
.peek(isNotPrivate(annotationType, issueReporter)) //
9092
.filter(allOf(returnsPrimitiveVoid(annotationType, issueReporter), additionalCondition)) //
9193
.collect(toUnmodifiableList());
9294
}
@@ -97,7 +99,7 @@ private static Predicate<Method> isStatic(Class<? extends Annotation> annotation
9799
String message = String.format(
98100
"@%s method '%s' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).",
99101
annotationType.getSimpleName(), method.toGenericString());
100-
return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
102+
return createIssue(Severity.ERROR, message, method);
101103
});
102104
}
103105

@@ -106,7 +108,17 @@ private static Predicate<Method> isNotStatic(Class<? extends Annotation> annotat
106108
return DiscoveryIssueReportingPredicate.from(ModifierSupport::isNotStatic, issueReporter, method -> {
107109
String message = String.format("@%s method '%s' must not be static.", annotationType.getSimpleName(),
108110
method.toGenericString());
109-
return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
111+
return createIssue(Severity.ERROR, message, method);
112+
});
113+
}
114+
115+
private static Consumer<Method> isNotPrivate(Class<? extends Annotation> annotationType,
116+
DiscoveryIssueReporter issueReporter) {
117+
return DiscoveryIssueReportingPredicate.from(ModifierSupport::isNotPrivate, issueReporter, method -> {
118+
String message = String.format(
119+
"@%s method '%s' should not be private. This will be disallowed in a future release.",
120+
annotationType.getSimpleName(), method.toGenericString());
121+
return createIssue(Severity.DEPRECATION, message, method);
110122
});
111123
}
112124

@@ -115,10 +127,14 @@ private static Predicate<Method> returnsPrimitiveVoid(Class<? extends Annotation
115127
return DiscoveryIssueReportingPredicate.from(ReflectionUtils::returnsPrimitiveVoid, issueReporter, method -> {
116128
String message = String.format("@%s method '%s' must not return a value.", annotationType.getSimpleName(),
117129
method.toGenericString());
118-
return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
130+
return createIssue(Severity.ERROR, message, method);
119131
});
120132
}
121133

134+
private static DiscoveryIssue createIssue(Severity severity, String message, Method method) {
135+
return DiscoveryIssue.builder(severity, message).source(MethodSource.from(method)).build();
136+
}
137+
122138
@SafeVarargs
123139
@SuppressWarnings("varargs")
124140
private static <T> Predicate<T> allOf(Predicate<? super T>... predicates) {
@@ -134,19 +150,19 @@ private static <T> Predicate<T> allOf(Predicate<? super T>... predicates) {
134150
};
135151
}
136152

137-
private abstract static class DiscoveryIssueReportingPredicate<T> implements Predicate<T> {
153+
private abstract static class DiscoveryIssueReportingPredicate<T> implements Predicate<T>, Consumer<T> {
138154

139155
static <T> DiscoveryIssueReportingPredicate<T> from(Predicate<T> predicate, DiscoveryIssueReporter reporter,
140156
Function<T, DiscoveryIssue> issueBuilder) {
141157
return new DiscoveryIssueReportingPredicate<T>(reporter) {
142158
@Override
143-
protected boolean checkCondition(T t) {
144-
return predicate.test(t);
159+
protected boolean checkCondition(T value) {
160+
return predicate.test(value);
145161
}
146162

147163
@Override
148-
protected DiscoveryIssue createIssue(T t) {
149-
return issueBuilder.apply(t);
164+
protected DiscoveryIssue createIssue(T value) {
165+
return issueBuilder.apply(value);
150166
}
151167
};
152168
}
@@ -158,17 +174,22 @@ protected DiscoveryIssueReportingPredicate(DiscoveryIssueReporter reporter) {
158174
}
159175

160176
@Override
161-
public final boolean test(T t) {
162-
if (checkCondition(t)) {
177+
public final boolean test(T value) {
178+
if (checkCondition(value)) {
163179
return true;
164180
}
165-
reporter.reportIssue(createIssue(t));
181+
reporter.reportIssue(createIssue(value));
166182
return false;
167183
}
168184

169-
protected abstract boolean checkCondition(T t);
185+
@Override
186+
public void accept(T value) {
187+
test(value);
188+
}
189+
190+
protected abstract boolean checkCondition(T value);
170191

171-
protected abstract DiscoveryIssue createIssue(T t);
192+
protected abstract DiscoveryIssue createIssue(T value);
172193
}
173194

174195
}

‎jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java

+48-31
Original file line numberDiff line numberDiff line change
@@ -44,58 +44,78 @@ class LifecycleMethodUtilsTests {
4444

4545
@Test
4646
void findNonVoidBeforeAllMethodsWithStandardLifecycle() throws Exception {
47-
var methods = findBeforeAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true, discoveryIssues::add);
47+
var methods = findBeforeAllMethods(TestCaseWithInvalidLifecycleMethods.class, true, discoveryIssues::add);
4848
assertThat(methods).isEmpty();
4949

50+
var methodSource = MethodSource.from(TestCaseWithInvalidLifecycleMethods.class.getDeclaredMethod("cc"));
5051
var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
51-
"@BeforeAll method 'java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.cc()' must not return a value.") //
52-
.source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("cc"))) //
52+
"@BeforeAll method 'private java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.cc()' must not return a value.") //
53+
.source(methodSource) //
5354
.build();
5455
var notStaticIssue = DiscoveryIssue.builder(Severity.ERROR,
55-
"@BeforeAll method 'java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.cc()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
56-
.source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("cc"))) //
56+
"@BeforeAll method 'private java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.cc()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
57+
.source(methodSource) //
5758
.build();
58-
assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue);
59+
var privateIssue = DiscoveryIssue.builder(Severity.DEPRECATION,
60+
"@BeforeAll method 'private java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.cc()' should not be private. This will be disallowed in a future release.") //
61+
.source(methodSource) //
62+
.build();
63+
assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue, privateIssue);
5964
}
6065

6166
@Test
6267
void findNonVoidAfterAllMethodsWithStandardLifecycle() throws Exception {
63-
var methods = findAfterAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true, discoveryIssues::add);
68+
var methods = findAfterAllMethods(TestCaseWithInvalidLifecycleMethods.class, true, discoveryIssues::add);
6469
assertThat(methods).isEmpty();
6570

71+
var methodSource = MethodSource.from(TestCaseWithInvalidLifecycleMethods.class.getDeclaredMethod("dd"));
6672
var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
67-
"@AfterAll method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.dd()' must not return a value.") //
68-
.source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("dd"))) //
73+
"@AfterAll method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.dd()' must not return a value.") //
74+
.source(methodSource) //
6975
.build();
7076
var notStaticIssue = DiscoveryIssue.builder(Severity.ERROR,
71-
"@AfterAll method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.dd()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
72-
.source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("dd"))) //
77+
"@AfterAll method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.dd()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
78+
.source(methodSource) //
79+
.build();
80+
var privateIssue = DiscoveryIssue.builder(Severity.DEPRECATION,
81+
"@AfterAll method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.dd()' should not be private. This will be disallowed in a future release.") //
82+
.source(methodSource) //
7383
.build();
74-
assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue);
84+
assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue, privateIssue);
7585
}
7686

7787
@Test
7888
void findNonVoidBeforeEachMethodsWithStandardLifecycle() throws Exception {
79-
var methods = findBeforeEachMethods(TestCaseWithNonVoidLifecyleMethods.class, discoveryIssues::add);
89+
var methods = findBeforeEachMethods(TestCaseWithInvalidLifecycleMethods.class, discoveryIssues::add);
8090
assertThat(methods).isEmpty();
8191

82-
var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
83-
"@BeforeEach method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.aa()' must not return a value.") //
84-
.source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("aa"))) //
92+
var methodSource = MethodSource.from(TestCaseWithInvalidLifecycleMethods.class.getDeclaredMethod("aa"));
93+
var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
94+
"@BeforeEach method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.aa()' must not return a value.") //
95+
.source(methodSource) //
8596
.build();
86-
assertThat(discoveryIssues).containsExactly(expectedIssue);
97+
var privateIssue = DiscoveryIssue.builder(Severity.DEPRECATION,
98+
"@BeforeEach method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.aa()' should not be private. This will be disallowed in a future release.") //
99+
.source(methodSource) //
100+
.build();
101+
assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, privateIssue);
87102
}
88103

89104
@Test
90105
void findNonVoidAfterEachMethodsWithStandardLifecycle() throws Exception {
91-
var methods = findAfterEachMethods(TestCaseWithNonVoidLifecyleMethods.class, discoveryIssues::add);
106+
var methods = findAfterEachMethods(TestCaseWithInvalidLifecycleMethods.class, discoveryIssues::add);
92107
assertThat(methods).isEmpty();
93108

94-
var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
95-
"@AfterEach method 'int org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.bb()' must not return a value.") //
96-
.source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("bb"))) //
109+
var methodSource = MethodSource.from(TestCaseWithInvalidLifecycleMethods.class.getDeclaredMethod("bb"));
110+
var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
111+
"@AfterEach method 'private int org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.bb()' must not return a value.") //
112+
.source(methodSource) //
97113
.build();
98-
assertThat(discoveryIssues).containsExactly(expectedIssue);
114+
var privateIssue = DiscoveryIssue.builder(Severity.DEPRECATION,
115+
"@AfterEach method 'private int org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.bb()' should not be private. This will be disallowed in a future release.") //
116+
.source(methodSource) //
117+
.build();
118+
assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, privateIssue);
99119
}
100120

101121
@Test
@@ -228,29 +248,26 @@ void eight() {
228248

229249
}
230250

231-
class TestCaseWithNonVoidLifecyleMethods {
251+
@SuppressWarnings("JUnitMalformedDeclaration")
252+
class TestCaseWithInvalidLifecycleMethods {
232253

233-
@SuppressWarnings("JUnitMalformedDeclaration")
234254
@BeforeEach
235-
String aa() {
255+
private String aa() {
236256
return null;
237257
}
238258

239-
@SuppressWarnings("JUnitMalformedDeclaration")
240259
@AfterEach
241-
int bb() {
260+
private int bb() {
242261
return 1;
243262
}
244263

245-
@SuppressWarnings("JUnitMalformedDeclaration")
246264
@BeforeAll
247-
Double cc() {
265+
private Double cc() {
248266
return null;
249267
}
250268

251-
@SuppressWarnings("JUnitMalformedDeclaration")
252269
@AfterAll
253-
String dd() {
270+
private String dd() {
254271
return "";
255272
}
256273

0 commit comments

Comments
 (0)