Skip to content

Commit 0f4b3bd

Browse files
stefanodallapalmastefanodtimtebeekgithub-actions[bot]
authored
Add configurable null-checking methods to AnnotateNullableParameters (#737)
* Add additionalNullCheckingMethods parameter to AnnotateNullableParameters * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Slight polish --------- Co-authored-by: stefanod <[email protected]> Co-authored-by: Tim te Beek <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tim te Beek <[email protected]>
1 parent 49cd6a0 commit 0f4b3bd

File tree

2 files changed

+143
-25
lines changed

2 files changed

+143
-25
lines changed

‎src/main/java/org/openrewrite/staticanalysis/AnnotateNullableParameters.java‎

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package org.openrewrite.staticanalysis;
1717

1818
import lombok.EqualsAndHashCode;
19-
import lombok.RequiredArgsConstructor;
2019
import lombok.Value;
2120
import org.jspecify.annotations.Nullable;
2221
import org.openrewrite.*;
@@ -32,24 +31,13 @@
3231

3332
import java.util.*;
3433

34+
import static java.util.stream.Collectors.toList;
35+
3536
@EqualsAndHashCode(callSuper = false)
3637
@Value
3738
public class AnnotateNullableParameters extends Recipe {
3839

3940
private static final String DEFAULT_NULLABLE_ANN_CLASS = "org.jspecify.annotations.Nullable";
40-
private static final List<MethodMatcher> NULL_SAFETY_METHOD_MATCHERS = Arrays.asList(
41-
new MethodMatcher("com.google.common.base.Strings isNullOrEmpty(..)"), // Guava
42-
new MethodMatcher("java.util.Objects isNull(..)"),
43-
new MethodMatcher("java.util.Objects nonNull(..)"),
44-
new MethodMatcher("org.apache.commons.lang3.StringUtils isBlank(..)"),
45-
new MethodMatcher("org.apache.commons.lang3.StringUtils isEmpty(..)"),
46-
new MethodMatcher("org.apache.commons.lang3.StringUtils isNotBlank(..)"),
47-
new MethodMatcher("org.apache.commons.lang3.StringUtils isNotEmpty(..)"),
48-
new MethodMatcher("org.springframework.util.ObjectUtils hasText(..)"),
49-
new MethodMatcher("org.springframework.util.StringUtils isEmpty(..)"), // Deprecated
50-
new MethodMatcher("org.springframework.util.StringUtils hasLength(..)"),
51-
new MethodMatcher("org.springframework.util.StringUtils hasText(..)")
52-
);
5341

5442
@Option(displayName = "`@Nullable` annotation class",
5543
description = "The fully qualified name of the @Nullable annotation. The annotation should be meta annotated with `@Target(TYPE_USE)`. Defaults to `org.jspecify.annotations.Nullable`",
@@ -58,19 +46,28 @@ public class AnnotateNullableParameters extends Recipe {
5846
@Nullable
5947
String nullableAnnotationClass;
6048

49+
@Option(
50+
displayName = "Additional null-checking methods",
51+
description = "A list of method patterns (in OpenRewrite MethodMatcher format) that should be considered as null-checking methods. " +
52+
"These will be added to the built-in list of known null-checking methods. " +
53+
"Use `..` for any parameters, e.g., `com.mycompany.utils.StringUtil isEmpty(..)` or `com.mycompany.utils.CollectionUtil isNullOrEmpty(java.util.Collection)`",
54+
example = "com.mycompany.utils.StringUtil isEmpty(..), com.mycompany.utils.CollectionUtil isNullOrEmpty(..)",
55+
required = false)
56+
@Nullable
57+
List<String> additionalNullCheckingMethods;
58+
6159
@Override
6260
public String getDisplayName() {
6361
return "Annotate null-checked method parameters with `@Nullable`";
6462
}
6563

6664
@Override
6765
public String getDescription() {
68-
69-
return "Add `@Nullable` to parameters of public methods that are explicitly checked for `null`. " +
70-
"By default `org.jspecify.annotations.Nullable` is used, but through the `nullableAnnotationClass` option a custom annotation can be provided. " +
71-
"When providing a custom `nullableAnnotationClass` that annotation should be meta annotated with `@Target(TYPE_USE)`. " +
72-
"This recipe scans for methods that do not already have parameters annotated with `@Nullable` annotation and checks their usages " +
73-
"for potential null checks.";
66+
return "Add `@Nullable` to parameters of public methods that are explicitly checked for `null`. "
67+
+ "By default `org.jspecify.annotations.Nullable` is used, but through the `nullableAnnotationClass` option a custom annotation can be provided. "
68+
+ "When providing a custom `nullableAnnotationClass` that annotation should be meta annotated with `@Target(TYPE_USE)`. "
69+
+ "This recipe scans for methods that do not already have parameters annotated with `@Nullable` annotation and checks their usages "
70+
+ "for potential null checks. Additional null-checking methods can be specified via the `additionalNullCheckingMethods` option.";
7471
}
7572

7673
@Override
@@ -100,7 +97,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl
10097
}
10198

10299
Map<J.VariableDeclarations, J.Identifier> candidateIdentifiers = buildIdentifierMap(findCandidateParameters(md, fullyQualifiedName));
103-
Set<J.Identifier> nullCheckedIdentifiers = new NullCheckVisitor(candidateIdentifiers.values())
100+
Set<J.Identifier> nullCheckedIdentifiers = new NullCheckVisitor(candidateIdentifiers.values(), additionalNullCheckingMethods)
104101
.reduce(md.getBody(), new HashSet<>());
105102

106103
maybeAddImport(fullyQualifiedName);
@@ -173,10 +170,36 @@ private Map<J.VariableDeclarations, J.Identifier> buildIdentifierMap(List<J.Vari
173170
* <li>Negated null-checking method calls (!Objects.isNull, !StringUtils.isBlank, etc.)</li>
174171
* </ul>
175172
*/
176-
@RequiredArgsConstructor
177173
private static class NullCheckVisitor extends JavaIsoVisitor<Set<J.Identifier>> {
174+
private static final List<MethodMatcher> NULL_SAFETY_METHOD_MATCHERS = Arrays.asList(
175+
new MethodMatcher("com.google.common.base.Strings isNullOrEmpty(..)"), // Guava
176+
new MethodMatcher("java.util.Objects isNull(..)"),
177+
new MethodMatcher("java.util.Objects nonNull(..)"),
178+
new MethodMatcher("org.apache.commons.lang3.StringUtils isBlank(..)"),
179+
new MethodMatcher("org.apache.commons.lang3.StringUtils isEmpty(..)"),
180+
new MethodMatcher("org.apache.commons.lang3.StringUtils isNotBlank(..)"),
181+
new MethodMatcher("org.apache.commons.lang3.StringUtils isNotEmpty(..)"),
182+
new MethodMatcher("org.springframework.util.ObjectUtils hasText(..)"),
183+
new MethodMatcher("org.springframework.util.StringUtils isEmpty(..)"), // Deprecated
184+
new MethodMatcher("org.springframework.util.StringUtils hasLength(..)"),
185+
new MethodMatcher("org.springframework.util.StringUtils hasText(..)")
186+
);
178187

179188
private final Collection<J.Identifier> identifiers;
189+
private final Collection<MethodMatcher> nullCheckingMethodMatchers;
190+
191+
public NullCheckVisitor(Collection<J.Identifier> identifiers, @Nullable Collection<String> additionalNullCheckingMethods) {
192+
this.identifiers = identifiers;
193+
if (additionalNullCheckingMethods == null) {
194+
nullCheckingMethodMatchers = NULL_SAFETY_METHOD_MATCHERS;
195+
} else {
196+
nullCheckingMethodMatchers = new ArrayList<>(NULL_SAFETY_METHOD_MATCHERS);
197+
nullCheckingMethodMatchers.addAll(
198+
additionalNullCheckingMethods.stream()
199+
.map(MethodMatcher::new)
200+
.collect(toList()));
201+
}
202+
}
180203

181204
@Override
182205
public J.If visitIf(J.If iff, Set<J.Identifier> nullCheckedParams) {
@@ -236,7 +259,7 @@ private void handleUnary(J.Unary unary, Set<J.Identifier> nullCheckedParams) {
236259
}
237260

238261
private boolean isKnownNullMethodChecker(J.MethodInvocation methodInvocation) {
239-
for (MethodMatcher m : NULL_SAFETY_METHOD_MATCHERS) {
262+
for (MethodMatcher m : nullCheckingMethodMatchers) {
240263
if (m.matches(methodInvocation)) {
241264
return true;
242265
}

‎src/test/java/org/openrewrite/staticanalysis/AnnotateNullableParametersTest.java‎

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,64 @@
2121
import org.junit.jupiter.params.provider.CsvSource;
2222
import org.junit.jupiter.params.provider.ValueSource;
2323
import org.openrewrite.DocumentExample;
24+
import org.openrewrite.java.JavaParser;
2425
import org.openrewrite.test.RecipeSpec;
2526
import org.openrewrite.test.RewriteTest;
2627
import org.openrewrite.test.SourceSpec;
2728

29+
import java.util.List;
30+
2831
import static org.openrewrite.java.Assertions.java;
2932

3033
class AnnotateNullableParametersTest implements RewriteTest {
3134

3235
@Override
3336
public void defaults(RecipeSpec spec) {
34-
spec.recipe(new AnnotateNullableParameters(null));
37+
spec.recipe(new AnnotateNullableParameters(null, null))
38+
.parser(
39+
JavaParser.fromJavaVersion()
40+
.dependsOn(
41+
// language=java
42+
"""
43+
package org.jspecify.annotations;
44+
public @interface Nullable {}
45+
""",
46+
// language=java
47+
"""
48+
package org.openrewrite.jgit.annotations;
49+
public @interface Nullable {}
50+
""",
51+
// language=java
52+
"""
53+
package org.apache.commons.lang3;
54+
55+
public interface StringUtils {
56+
boolean isBlank(String s);
57+
boolean isEmpty(String s);
58+
boolean isNotBlank(String s);
59+
boolean isNotEmpty(String s);
60+
}
61+
""",
62+
// language=java
63+
"""
64+
package org.springframework.util;
65+
66+
public interface ObjectUtils {
67+
boolean hasText(String s);
68+
boolean isEmpty(String s);
69+
boolean hasLength(String s);
70+
boolean hasText(String s);
71+
}
72+
""",
73+
// language=java
74+
"""
75+
package org.my.util;
76+
77+
public interface Text {
78+
boolean hasText(String s);
79+
boolean isEmptyOrNull(String s);
80+
}
81+
"""));
3582
}
3683

3784

@@ -479,7 +526,7 @@ public PersonBuilder setSurname(@Nullable String surname) {
479526
@Test
480527
void provideCustomNullableAnnotation() {
481528
rewriteRun(
482-
spec -> spec.recipe(new AnnotateNullableParameters("org.openrewrite.jgit.annotations.Nullable")),
529+
spec -> spec.recipe(new AnnotateNullableParameters("org.openrewrite.jgit.annotations.Nullable", null)),
483530
//language=java
484531
java(
485532
"""
@@ -513,4 +560,52 @@ public PersonBuilder setName(@Nullable String name) {
513560
)
514561
);
515562
}
563+
564+
@Test
565+
void provideAdditionalNullCheckingMethods() {
566+
List<String> additionalNullCheckingMethods = List.of("org.my.util.Text hasText(java.lang.String)", "org.my.util.Text isEmptyOrNull(java.lang.String)");
567+
rewriteRun(
568+
spec -> spec.recipe(new AnnotateNullableParameters(null, additionalNullCheckingMethods)),
569+
//language=java
570+
java(
571+
"""
572+
import org.my.util.Text;
573+
574+
public class PersonBuilder {
575+
private String name = "Unknown";
576+
private String lastName = "Unknown";
577+
578+
public PersonBuilder setInfo(String name, String lastName) {
579+
if (Text.hasText(name)) {
580+
this.name = name.substring(0, 1).toUpperCase() + name.substring(1);
581+
}
582+
if (!Text.isEmptyOrNull(lastName)) {
583+
this.lastName = lastName.substring(0, 1).toUpperCase() + lastName.substring(1);
584+
}
585+
return this;
586+
}
587+
}
588+
""",
589+
"""
590+
import org.jspecify.annotations.Nullable;
591+
import org.my.util.Text;
592+
593+
public class PersonBuilder {
594+
private String name = "Unknown";
595+
private String lastName = "Unknown";
596+
597+
public PersonBuilder setInfo(@Nullable String name, @Nullable String lastName) {
598+
if (Text.hasText(name)) {
599+
this.name = name.substring(0, 1).toUpperCase() + name.substring(1);
600+
}
601+
if (!Text.isEmptyOrNull(lastName)) {
602+
this.lastName = lastName.substring(0, 1).toUpperCase() + lastName.substring(1);
603+
}
604+
return this;
605+
}
606+
}
607+
"""
608+
)
609+
);
610+
}
516611
}

0 commit comments

Comments
 (0)