Skip to content

Commit 401e795

Browse files
DidierBessetWohops
authored andcommitted
SONARJAVA-1460 - Nullability constraint on an object checked with Objects null checking methods
1 parent ee1db26 commit 401e795

7 files changed

Lines changed: 162 additions & 0 deletions

File tree

java-squid/src/main/java/org/sonar/java/se/ConstraintManager.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import com.google.common.base.Preconditions;
2323
import com.google.common.collect.ImmutableList;
24+
import org.sonar.java.se.symbolicvalues.NullCheckSymbolicValue;
2425
import org.sonar.java.se.symbolicvalues.RelationalSymbolicValue;
2526
import org.sonar.java.se.symbolicvalues.SymbolicValue;
2627
import org.sonar.plugins.java.api.semantic.Symbol;
@@ -103,6 +104,14 @@ public SymbolicValue createMethodSymbolicValue(MethodInvocationTree syntaxNode,
103104
SymbolicValue leftOp = values.get(0);
104105
SymbolicValue rightOp = values.get(1);
105106
result.computedFrom(ImmutableList.of(rightOp, leftOp));
107+
} else if (ExplodedGraphWalker.isObjectsMethod(syntaxNode, "isNull", 1)) {
108+
result = new NullCheckSymbolicValue(counter, true);
109+
SymbolicValue operand = values.get(1);
110+
result.computedFrom(ImmutableList.of(operand));
111+
} else if (ExplodedGraphWalker.isObjectsMethod(syntaxNode, "nonNull", 1)) {
112+
result = new NullCheckSymbolicValue(counter, false);
113+
SymbolicValue operand = values.get(1);
114+
result.computedFrom(ImmutableList.of(operand));
106115
} else {
107116
result = createDefaultSymbolicValue(syntaxNode);
108117
}

java-squid/src/main/java/org/sonar/java/se/ExplodedGraphWalker.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ public class ExplodedGraphWalker extends BaseTreeVisitor {
8181
private static final int MAX_STEPS = 10000;
8282
private static final Logger LOG = LoggerFactory.getLogger(ExplodedGraphWalker.class);
8383
private static final Set<String> THIS_SUPER = ImmutableSet.of("this", "super");
84+
public static final String OBJECT_CLASS_NAME = "java.lang.Object";
8485

8586
private static final boolean DEBUG_MODE_ACTIVATED = false;
8687
private static final int MAX_EXEC_PROGRAM_POINT = 2;
@@ -574,6 +575,17 @@ private static boolean isLocalMethodInvocation(MethodInvocationTree tree) {
574575
return false;
575576
}
576577

578+
public static boolean isObjectsMethod(MethodInvocationTree syntaxNode, String methodName, int numberOfParameter) {
579+
Symbol symbol = syntaxNode.symbol();
580+
if (symbol.isMethodSymbol()) {
581+
Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) symbol;
582+
if (methodSymbol.owner().type().is("java.util.Objects") && methodName.equals(methodSymbol.name())) {
583+
return methodSymbol.parameterTypes().size() == numberOfParameter;
584+
}
585+
}
586+
return false;
587+
}
588+
577589
private void resetFieldValues() {
578590
programState = programState.resetFieldValues(constraintManager);
579591
}

java-squid/src/main/java/org/sonar/java/se/checks/NullDereferenceCheck.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.sonar.check.Rule;
2727
import org.sonar.java.model.DefaultJavaFileScannerContext;
2828
import org.sonar.java.se.CheckerContext;
29+
import org.sonar.java.se.ExplodedGraphWalker;
2930
import org.sonar.java.se.ObjectConstraint;
3031
import org.sonar.java.se.ProgramState;
3132
import org.sonar.java.se.symbolicvalues.SymbolicValue;
@@ -51,6 +52,8 @@
5152
@ActivatedByDefault
5253
public class NullDereferenceCheck extends SECheck implements JavaFileScanner {
5354

55+
private static final String REQUIRE_NON_NULL_METHOD_NAME = "requireNonNull";
56+
5457
@Override
5558
public void scanFile(JavaFileScannerContext context) {
5659
Multimap<Tree, String> issues = ((DefaultJavaFileScannerContext) context).getSEIssues(NullDereferenceCheck.class);
@@ -70,6 +73,14 @@ public ProgramState checkPreStatement(CheckerContext context, Tree syntaxNode) {
7073
if (syntaxNode.is(Tree.Kind.METHOD_INVOCATION)) {
7174
MethodInvocationTree methodInvocation = (MethodInvocationTree) syntaxNode;
7275
toCheck = methodInvocation.methodSelect();
76+
if (ExplodedGraphWalker.isObjectsMethod(methodInvocation, REQUIRE_NON_NULL_METHOD_NAME, 1)) {
77+
final List<SymbolicValue> values = context.getState().peekValues(2);
78+
return context.getState().addConstraint(values.get(1), ObjectConstraint.NOT_NULL);
79+
} else if (ExplodedGraphWalker.isObjectsMethod(methodInvocation, REQUIRE_NON_NULL_METHOD_NAME, 2)
80+
|| ExplodedGraphWalker.isObjectsMethod(methodInvocation, REQUIRE_NON_NULL_METHOD_NAME, 2)) {
81+
final List<SymbolicValue> values = context.getState().peekValues(3);
82+
return context.getState().addConstraint(values.get(2), ObjectConstraint.NOT_NULL);
83+
}
7384
}
7485
if (toCheck.is(Tree.Kind.MEMBER_SELECT)) {
7586
return checkMemberSelect(context, (MemberSelectExpressionTree) toCheck, currentVal);
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2016 SonarSource SA
4+
* mailto:contact AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.java.se.symbolicvalues;
21+
22+
import com.google.common.collect.ImmutableList;
23+
import org.sonar.java.se.ConstraintManager.BooleanConstraint;
24+
import org.sonar.java.se.ObjectConstraint;
25+
import org.sonar.java.se.ProgramState;
26+
import org.sonar.java.se.symbolicvalues.SymbolicValue.UnarySymbolicValue;
27+
28+
import java.util.List;
29+
30+
public class NullCheckSymbolicValue extends UnarySymbolicValue {
31+
32+
private boolean isNull;
33+
34+
public NullCheckSymbolicValue(int id, boolean isNull) {
35+
super(id);
36+
this.isNull = isNull;
37+
}
38+
39+
@Override
40+
public List<ProgramState> setConstraint(ProgramState programState, BooleanConstraint booleanConstraint) {
41+
final ObjectConstraint constraint;
42+
if (BooleanConstraint.TRUE.equals(booleanConstraint)) {
43+
constraint = isNull ? ObjectConstraint.NULL : ObjectConstraint.NOT_NULL;
44+
} else {
45+
constraint = isNull ? ObjectConstraint.NOT_NULL : ObjectConstraint.NULL;
46+
}
47+
return ImmutableList.of(programState.addConstraint(operand, constraint));
48+
}
49+
}

java-squid/src/test/files/se/NullDereferenceCheck.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
@SomeAnnotation(name = value)
22
package javax.annotation;
33

4+
import java.util.function.Supplier;
5+
46
import java.util.List;
7+
import java.util.Objects;
58

69
@interface CheckForNull {}
710

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package java.util;
2+
3+
import java.util.Objects;
4+
5+
public class ObjectsNullCheck {
6+
7+
public void parameterMaybeNullable(Object a) {
8+
Object x = checkForNullMethod();
9+
if (a.equals(x)) {
10+
x.toString(); // Compliant: x cannot be null hereafter because of equality
11+
}
12+
}
13+
14+
public void parameterNoLongerNullable(Object a) {
15+
Object x = checkForNullMethod();
16+
if (Objects.nonNull(x)) {
17+
x.toString(); // Compliant: x was checked for non null
18+
} else {
19+
x.logNull(); // Noncompliant {{NullPointerException might be thrown as 'x' is nullable here}}
20+
}
21+
}
22+
23+
public void parameterStillNullable(Object a) {
24+
Object x = checkForNullMethod();
25+
if (Objects.isNull(x)) {
26+
log("was null");
27+
} else {
28+
x.toString(); // Compliant: x was checked for non null
29+
}
30+
}
31+
32+
public void testRequireNull(Supplier<String> supplier) {
33+
Object x = checkForNullMethod();
34+
Objects.requireNonNull(x);
35+
x.toString(); // Compliant: x was checked for non null
36+
Object y = checkForNullMethod();
37+
Objects.requireNonNull(y, "Should not be null!");
38+
y.toString(); // Compliant: y was checked for non null
39+
Object z = checkForNullMethod();
40+
Objects.requireNonNull(z, supplier);
41+
z.toString(); // Compliant: z was checked for non null
42+
}
43+
}
44+
45+
// Needed to ensure that all methods of Objects are declared (for tests with Java version prior to 1.8
46+
47+
class Objects {
48+
public static boolean isNull(Object obj) {
49+
return obj == null;
50+
}
51+
52+
public static boolean nonNull(Object obj) {
53+
return obj != null;
54+
}
55+
56+
public static <T> T requireNonNull(T obj) {
57+
if (obj == null) {
58+
throw new NullPointerException();
59+
}
60+
}
61+
62+
public static <T> T requireNonNull(T obj, String message) {
63+
if (obj == null) {
64+
throw new NullPointerException(message);
65+
}
66+
}
67+
68+
public static <T> T requireNonNull(T obj, Supplier<String> supplier) {
69+
if (obj == null) {
70+
throw new NullPointerException(supplier.get());
71+
}
72+
}
73+
}

java-squid/src/test/java/org/sonar/java/se/checks/NullDereferenceCheckTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,9 @@ public void test() {
2929
JavaCheckVerifier.verify("src/test/files/se/NullDereferenceCheck.java", new NullDereferenceCheck());
3030
}
3131

32+
@Test
33+
public void objectsMethodsTest() {
34+
JavaCheckVerifier.verify("src/test/files/se/ObjectsMethodsTest.java", new NullDereferenceCheck());
35+
}
36+
3237
}

0 commit comments

Comments
 (0)