Skip to content

Commit f2bd87a

Browse files
cushonError Prone Team
cushon
authored and
Error Prone Team
committed
Suggest Objects.requireNonNull instead of if/throw
PiperOrigin-RevId: 934930985
1 parent a924abd commit f2bd87a

4 files changed

Lines changed: 359 additions & 0 deletions

File tree

‎core/src/main/java/com/google/errorprone/bugpatterns/nullness/RequireNonNullRefactoring.java‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/*
2+
* Copyright 2026 The Error Prone 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+
* http://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 com.google.errorprone.bugpatterns.nullness;
18+
19+
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
20+
import static com.google.errorprone.matchers.Description.NO_MATCH;
21+
import static com.google.errorprone.suppliers.Suppliers.typeFromString;
22+
import static com.google.errorprone.util.ASTHelpers.constValue;
23+
import static com.google.errorprone.util.ASTHelpers.getType;
24+
import static com.google.errorprone.util.ASTHelpers.isSameType;
25+
import static com.google.errorprone.util.ASTHelpers.stripParentheses;
26+
27+
import com.google.errorprone.BugPattern;
28+
import com.google.errorprone.VisitorState;
29+
import com.google.errorprone.bugpatterns.BugChecker;
30+
import com.google.errorprone.bugpatterns.BugChecker.IfTreeMatcher;
31+
import com.google.errorprone.fixes.SuggestedFix;
32+
import com.google.errorprone.fixes.SuggestedFixes;
33+
import com.google.errorprone.matchers.Description;
34+
import com.google.errorprone.suppliers.Supplier;
35+
import com.sun.source.tree.BinaryTree;
36+
import com.sun.source.tree.BlockTree;
37+
import com.sun.source.tree.ExpressionTree;
38+
import com.sun.source.tree.IfTree;
39+
import com.sun.source.tree.NewClassTree;
40+
import com.sun.source.tree.StatementTree;
41+
import com.sun.source.tree.ThrowTree;
42+
import com.sun.source.tree.Tree.Kind;
43+
import com.sun.tools.javac.code.Type;
44+
import java.util.List;
45+
import org.jspecify.annotations.Nullable;
46+
47+
/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
48+
@BugPattern(
49+
summary = "Refactor explicit null checks to Objects.requireNonNull",
50+
severity = SUGGESTION)
51+
public final class RequireNonNullRefactoring extends BugChecker implements IfTreeMatcher {
52+
53+
private static final Supplier<Type> NULL_POINTER_EXCEPTION =
54+
typeFromString("java.lang.NullPointerException");
55+
56+
@Override
57+
public Description matchIf(IfTree ifTree, VisitorState state) {
58+
if (ifTree.getElseStatement() != null) {
59+
return NO_MATCH;
60+
}
61+
ExpressionTree targetExpr = getNullCheckedExpression(ifTree.getCondition());
62+
if (targetExpr == null) {
63+
return NO_MATCH;
64+
}
65+
ThrowTree throwTree = getThrowStatement(ifTree.getThenStatement());
66+
if (throwTree == null) {
67+
return NO_MATCH;
68+
}
69+
ExpressionTree thrownExpr = stripParentheses(throwTree.getExpression());
70+
if (!(thrownExpr instanceof NewClassTree newClassTree)) {
71+
return NO_MATCH;
72+
}
73+
if (!isSameType(getType(newClassTree), NULL_POINTER_EXCEPTION.get(state), state)) {
74+
return NO_MATCH;
75+
}
76+
List<? extends ExpressionTree> arguments = newClassTree.getArguments();
77+
@Nullable ExpressionTree messageExpr;
78+
if (arguments.isEmpty()) {
79+
messageExpr = null;
80+
} else if (arguments.size() == 1) {
81+
messageExpr = arguments.get(0);
82+
if (constValue(messageExpr, String.class) == null) {
83+
return NO_MATCH;
84+
}
85+
} else {
86+
return NO_MATCH;
87+
}
88+
SuggestedFix.Builder fix = SuggestedFix.builder();
89+
buildFix(fix, ifTree, targetExpr, messageExpr, state);
90+
return describeMatch(ifTree, fix.build());
91+
}
92+
93+
private static void buildFix(
94+
SuggestedFix.Builder fix,
95+
IfTree ifTree,
96+
ExpressionTree targetExpr,
97+
@Nullable ExpressionTree messageExpr,
98+
VisitorState state) {
99+
String requireNonNullQualified =
100+
SuggestedFixes.qualifyStaticImport("java.util.Objects.requireNonNull", fix, state);
101+
String targetExprSource = state.getSourceForNode(targetExpr);
102+
String replacement =
103+
String.format(
104+
"%s(%s%s);",
105+
requireNonNullQualified,
106+
targetExprSource,
107+
messageExpr == null ? "" : ", " + state.getSourceForNode(messageExpr));
108+
fix.replace(ifTree, replacement);
109+
}
110+
111+
private static @Nullable ExpressionTree getNullCheckedExpression(ExpressionTree condition) {
112+
condition = stripParentheses(condition);
113+
if (condition.getKind() != Kind.EQUAL_TO) {
114+
return null;
115+
}
116+
BinaryTree binary = (BinaryTree) condition;
117+
if (binary.getLeftOperand().getKind() == Kind.NULL_LITERAL) {
118+
return binary.getRightOperand();
119+
}
120+
if (binary.getRightOperand().getKind() == Kind.NULL_LITERAL) {
121+
return binary.getLeftOperand();
122+
}
123+
return null;
124+
}
125+
126+
private static @Nullable ThrowTree getThrowStatement(StatementTree then) {
127+
while (then instanceof BlockTree block) {
128+
var statements = block.getStatements();
129+
if (statements.size() != 1) {
130+
return null;
131+
}
132+
then = statements.getFirst();
133+
}
134+
return then instanceof ThrowTree throwTree ? throwTree : null;
135+
}
136+
}

‎core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@
612612
import com.google.errorprone.bugpatterns.nullness.NullableWildcard;
613613
import com.google.errorprone.bugpatterns.nullness.ParameterMissingNullable;
614614
import com.google.errorprone.bugpatterns.nullness.RedundantNullCheck;
615+
import com.google.errorprone.bugpatterns.nullness.RequireNonNullRefactoring;
615616
import com.google.errorprone.bugpatterns.nullness.ReturnMissingNullable;
616617
import com.google.errorprone.bugpatterns.nullness.UnnecessaryCheckNotNull;
617618
import com.google.errorprone.bugpatterns.nullness.UnsafeWildcard;
@@ -1323,6 +1324,7 @@ public static ScannerSupplier warningChecks() {
13231324
RedundantThrows.class,
13241325
RefersToDaggerCodegen.class,
13251326
RemoveUnusedImports.class,
1327+
RequireNonNullRefactoring.class,
13261328
ReturnMissingNullable.class,
13271329
ReturnsNullCollection.class,
13281330
ScopeOnModule.class,

‎core/src/test/java/com/google/errorprone/bugpatterns/nullness/RequireNonNullRefactoringTest.java‎

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
/*
2+
* Copyright 2026 The Error Prone 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+
* http://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 com.google.errorprone.bugpatterns.nullness;
18+
19+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
20+
import org.junit.Test;
21+
import org.junit.runner.RunWith;
22+
import org.junit.runners.JUnit4;
23+
24+
/** Tests for {@link RequireNonNullRefactoring}. */
25+
@RunWith(JUnit4.class)
26+
public final class RequireNonNullRefactoringTest {
27+
28+
private final BugCheckerRefactoringTestHelper testHelper =
29+
BugCheckerRefactoringTestHelper.newInstance(RequireNonNullRefactoring.class, getClass());
30+
31+
@Test
32+
public void refactoringNoMessage() {
33+
testHelper
34+
.addInputLines(
35+
"Test.java",
36+
"""
37+
class Test {
38+
void foo(Object bar) {
39+
if (bar == null) {
40+
throw new NullPointerException();
41+
}
42+
}
43+
}
44+
""")
45+
.addOutputLines(
46+
"Test.java",
47+
"""
48+
import static java.util.Objects.requireNonNull;
49+
50+
class Test {
51+
void foo(Object bar) {
52+
requireNonNull(bar);
53+
}
54+
}
55+
""")
56+
.doTest();
57+
}
58+
59+
@Test
60+
public void refactoringConstantMessage() {
61+
testHelper
62+
.addInputLines(
63+
"Test.java",
64+
"""
65+
class Test {
66+
void foo(Object bar) {
67+
if (null == bar) {
68+
throw new NullPointerException("bar must not be null");
69+
}
70+
}
71+
}
72+
""")
73+
.addOutputLines(
74+
"Test.java",
75+
"""
76+
import static java.util.Objects.requireNonNull;
77+
78+
class Test {
79+
void foo(Object bar) {
80+
requireNonNull(bar, "bar must not be null");
81+
}
82+
}
83+
""")
84+
.doTest();
85+
}
86+
87+
@Test
88+
public void noBraces() {
89+
testHelper
90+
.addInputLines(
91+
"Test.java",
92+
"""
93+
class Test {
94+
void foo(Object bar) {
95+
if (bar == null) throw new NullPointerException();
96+
}
97+
}
98+
""")
99+
.addOutputLines(
100+
"Test.java",
101+
"""
102+
import static java.util.Objects.requireNonNull;
103+
104+
class Test {
105+
void foo(Object bar) {
106+
requireNonNull(bar);
107+
}
108+
}
109+
""")
110+
.doTest();
111+
}
112+
113+
@Test
114+
public void negativeWrongException() {
115+
testHelper
116+
.addInputLines(
117+
"Test.java",
118+
"""
119+
class Test {
120+
void foo(Object bar) {
121+
if (bar == null) {
122+
throw new IllegalArgumentException();
123+
}
124+
}
125+
}
126+
""")
127+
.expectUnchanged()
128+
.doTest();
129+
}
130+
131+
@Test
132+
public void negativeWrongCondition() {
133+
testHelper
134+
.addInputLines(
135+
"Test.java",
136+
"""
137+
class Test {
138+
void foo(Object bar) {
139+
if (bar != null) {
140+
throw new NullPointerException();
141+
}
142+
}
143+
}
144+
""")
145+
.expectUnchanged()
146+
.doTest();
147+
}
148+
149+
@Test
150+
public void negativeMultipleStatements() {
151+
testHelper
152+
.addInputLines(
153+
"Test.java",
154+
"""
155+
class Test {
156+
void foo(Object bar) {
157+
if (bar == null) {
158+
System.out.println("bar is null!");
159+
throw new NullPointerException();
160+
}
161+
}
162+
}
163+
""")
164+
.expectUnchanged()
165+
.doTest();
166+
}
167+
168+
@Test
169+
public void negativeNonConstantMessage() {
170+
testHelper
171+
.addInputLines(
172+
"Test.java",
173+
"""
174+
class Test {
175+
void foo(Object bar) {
176+
String msg = "bar is null";
177+
if (bar == null) {
178+
throw new NullPointerException(msg);
179+
}
180+
}
181+
}
182+
""")
183+
.expectUnchanged()
184+
.doTest();
185+
}
186+
187+
@Test
188+
public void negativeHasElse() {
189+
testHelper
190+
.addInputLines(
191+
"Test.java",
192+
"""
193+
class Test {
194+
void foo(Object bar) {
195+
if (bar == null) {
196+
throw new NullPointerException();
197+
} else {
198+
System.out.println("ok");
199+
}
200+
}
201+
}
202+
""")
203+
.expectUnchanged()
204+
.doTest();
205+
}
206+
}

‎docs/bugpattern/nullness/RequireNonNullRefactoring.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
Consider using `requireNonNull(value)` which is shorter and more readable than
2+
3+
```
4+
if (value == null) {
5+
throw new NullPointerException(...);
6+
}
7+
```
8+
9+
The main reason to prefer `requireNonNull` is for readability, but it has no
10+
performance downsides and may have a small performance benefit.
11+
12+
The method is annotated with `@ForceInline` and receives special treatment from
13+
the JVM to ensure it is inlined into equivalent code to the `if`/`throw`. It
14+
also results in slightly smaller Java bytecode, which can improve other JIT
15+
inlining decisions.

0 commit comments

Comments
 (0)