Skip to content

Commit 2fac772

Browse files
wilkinsonawilkinsona
committed
Enforce Spring team's nullability style and conventions
- JSpecify annotations are used to express nullability - Type use annotations (@nullable and @nonnull) are positioned immediately before the type Closes gh-465
1 parent 40f2ff5 commit 2fac772

18 files changed

+393
-2
lines changed

‎spring-javaformat/spring-javaformat-checkstyle/src/main/java/io/spring/javaformat/checkstyle/check/SpringNullabilityCheck.java‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/*
2+
* Copyright 2017-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.spring.javaformat.checkstyle.check;
18+
19+
import java.util.ArrayList;
20+
import java.util.Arrays;
21+
import java.util.HashSet;
22+
import java.util.List;
23+
import java.util.Set;
24+
25+
import com.puppycrawl.tools.checkstyle.api.DetailAST;
26+
import com.puppycrawl.tools.checkstyle.api.FullIdent;
27+
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
28+
29+
/**
30+
* Checks compliance with Spring team's nullability conventions. JSpecify annotations
31+
* should be used to express nullability and type-use annotations ({@code @Nullable}
32+
* and {@code @NonNull}) should appear immediately before a type.
33+
*
34+
* @author Andy Wilkinson
35+
*/
36+
public class SpringNullabilityCheck extends AbstractSpringCheck {
37+
38+
private final Set<String> unwantedNullabilityImports = new HashSet<>();
39+
40+
private final List<DetailAST> modifiers = new ArrayList<>();
41+
42+
@Override
43+
public int[] getAcceptableTokens() {
44+
return new int[] { TokenTypes.IMPORT, TokenTypes.MODIFIERS };
45+
}
46+
47+
@Override
48+
public void beginTree(DetailAST rootAST) {
49+
this.modifiers.clear();
50+
this.unwantedNullabilityImports.clear();
51+
}
52+
53+
@Override
54+
public void finishTree(DetailAST rootAST) {
55+
this.modifiers.forEach(this::visitModifiers);
56+
}
57+
58+
@Override
59+
public void visitToken(DetailAST ast) {
60+
switch (ast.getType()) {
61+
case TokenTypes.IMPORT:
62+
visitImport(ast);
63+
break;
64+
case TokenTypes.MODIFIERS:
65+
this.modifiers.add(ast);
66+
break;
67+
}
68+
}
69+
70+
private void visitImport(DetailAST ast) {
71+
FullIdent ident = FullIdent.createFullIdentBelow(ast);
72+
if (!isFullyQualifiedJSpecifyAnnotation(ident)) {
73+
String simpleName = simpleNameOf(ident);
74+
for (JSpecifyAnnotation annotation: JSpecifyAnnotation.values()) {
75+
if (annotation.replaces.contains(simpleName)) {
76+
log(ident.getLineNo(), ident.getColumnNo(), "nullability.bannedImport", ident.getText(), annotation.name);
77+
this.unwantedNullabilityImports.add(simpleName);
78+
}
79+
}
80+
}
81+
}
82+
83+
private boolean isFullyQualifiedJSpecifyAnnotation(FullIdent ident) {
84+
for (JSpecifyAnnotation annotation: JSpecifyAnnotation.values()) {
85+
if (ident.getText().equals(annotation.name)) {
86+
return true;
87+
}
88+
}
89+
return false;
90+
}
91+
92+
private String simpleNameOf(FullIdent ident) {
93+
String identText = ident.getText();
94+
return identText.substring(identText.lastIndexOf(".") + 1);
95+
}
96+
97+
private void visitModifiers(DetailAST ast) {
98+
DetailAST annotation = ast.findFirstToken(TokenTypes.ANNOTATION);
99+
if (annotation != null) {
100+
DetailAST ident = annotation.findFirstToken(TokenTypes.IDENT);
101+
if (ident != null) {
102+
String identText = ident.getText();
103+
DetailAST lastChild = ast.getLastChild();
104+
if (isJSpecifyAnnotation(ident) && !annotation.equals(lastChild)) {
105+
log(annotation.getLineNo(), annotation.getColumnNo(), "nullability.annotationLocation", identText);
106+
}
107+
}
108+
}
109+
}
110+
111+
private boolean isJSpecifyAnnotation(DetailAST ident) {
112+
for (JSpecifyAnnotation annotation: JSpecifyAnnotation.values()) {
113+
if (ident.getText().equals(annotation.simpleName) && !this.unwantedNullabilityImports.contains(ident.getText())) {
114+
return true;
115+
}
116+
}
117+
return false;
118+
}
119+
120+
private enum JSpecifyAnnotation {
121+
122+
NULLABLE("Nullable", "Nullable"),
123+
124+
NON_NULL("NonNull", "NonNull", "Nonnull");
125+
126+
private final String simpleName;
127+
128+
private final String name;
129+
130+
private Set<String> replaces;
131+
132+
JSpecifyAnnotation(String simpleName, String... replaces) {
133+
this.simpleName = simpleName;
134+
this.name = "org.jspecify.annotations." + simpleName;
135+
this.replaces = new HashSet<>(Arrays.asList(replaces));
136+
}
137+
138+
}
139+
140+
}

‎spring-javaformat/spring-javaformat-checkstyle/src/main/resources/io/spring/javaformat/checkstyle/check/messages.properties‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ lambda.unnecessaryParen=Lambda argument has unnecessary parentheses.
3232
methodorder.outOfOrder=Method ''{0}'' is out of order, expected {1}.
3333
methodvisibility.publicMethod=Method ''{0}'' in private class should not be public.
3434
nothis.unexpected=Reference to instance variable ''{0}'' should not use \"this.\".
35+
nullability.bannedImport=Nullability should be expressed using JSpecify. Replace ''{0}'' with ''{1}''.
36+
nullability.annotationLocation=''{0}'' should only be used immediately before a type.
3537
ternary.equalOperator=Ternary operation should use != when testing.
3638
ternary.missingParen=Ternary operation missing parentheses. Use the form \"(a != b) ? y : n\".
3739
testfilename.wrongName=Test classes should have a name ending with 'Tests.java'.

‎spring-javaformat/spring-javaformat-checkstyle/src/main/resources/io/spring/javaformat/checkstyle/spring-checkstyle.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@
161161
<module name="io.spring.javaformat.checkstyle.check.SpringLeadingWhitespaceCheck" />
162162
<module name="io.spring.javaformat.checkstyle.check.SpringMethodOrderCheck" />
163163
<module name="io.spring.javaformat.checkstyle.check.SpringMethodVisibilityCheck" />
164+
<module name="io.spring.javaformat.checkstyle.check.SpringNullabilityCheck" />
164165
<module name="io.spring.javaformat.checkstyle.check.SpringParenPadCheck" />
165166
</module>
166167
</module>

‎spring-javaformat/spring-javaformat-checkstyle/src/test/java/io/spring/javaformat/checkstyle/SpringConfigurationLoaderTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public void loadShouldLoadChecks() {
5050
assertThat(checks).hasSize(5);
5151
TreeWalker treeWalker = (TreeWalker) checks.toArray()[4];
5252
Set<?> ordinaryChecks = (Set<?>) Extractors.byName("ordinaryChecks").extract(treeWalker);
53-
assertThat(ordinaryChecks).hasSize(62);
53+
assertThat(ordinaryChecks).hasSize(63);
5454
Set<?> commentChecks = (Set<?>) Extractors.byName("commentChecks").extract(treeWalker);
5555
assertThat(commentChecks).hasSize(6);
5656
}
@@ -64,7 +64,7 @@ public void loadWithExcludeShouldExcludeChecks() {
6464
assertThat(checks).hasSize(5);
6565
TreeWalker treeWalker = (TreeWalker) checks.toArray()[4];
6666
Set<?> ordinaryChecks = (Set<?>) Extractors.byName("ordinaryChecks").extract(treeWalker);
67-
assertThat(ordinaryChecks).hasSize(61);
67+
assertThat(ordinaryChecks).hasSize(62);
6868
Set<?> commentChecks = (Set<?>) Extractors.byName("commentChecks").extract(treeWalker);
6969
assertThat(commentChecks).hasSize(5);
7070
}

‎spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/NullabilityBannedNonNull.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
+NullabilityBannedNonNull.java:17:8: Nullability should be expressed using JSpecify. Replace 'javax.annotation.Nonnull' with 'org.jspecify.annotations.NonNull'. [SpringNullability]
2+
+NullabilityBannedNonNull.java:19:8: Nullability should be expressed using JSpecify. Replace 'org.checkerframework.checker.nullness.qual.NonNull' with 'org.jspecify.annotations.NonNull'. [SpringNullability]
3+
+2 errors

‎spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/NullabilityBannedNullable.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
+Nullability should be expressed using JSpecify. Replace 'javax.annotation.Nullable' with 'org.jspecify.annotations.Nullable'. [SpringNullability]

‎spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/NullabilityNullableNotPrecedingFieldType.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
+NullabilityNullableNotPrecedingFieldType.java:26:17: 'Nullable' should only be used immediately before a type. [SpringNullability]

‎spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/NullabilityNullableNotPrecedingParameterType.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
NullabilityNullableNotPrecedingParameterType.java:26:30: 'Nullable' should only be used immediately before a type. [SpringNullability]

‎spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/NullabilityNullableNotPrecedingReturnType.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
+NullabilityNullableNotPrecedingReturnType.java:26:9: 'Nullable' should only be used immediately before a type. [SpringNullability]

‎spring-javaformat/spring-javaformat-checkstyle/src/test/resources/check/NullabilityNullableOnSeparateLine.txt‎

Whitespace-only changes.

0 commit comments

Comments
 (0)