Skip to content

Commit 6a2edce

Browse files
aschackmullaschackmull
authored
Merge pull request #2205 from rneatherway/java/hamcrest-nullness
Java: Respect Hamcrest assertThat(X, notNullValue())
2 parents 2eaf91e + 7850d67 commit 6a2edce

File tree

11 files changed

+118
-30
lines changed

11 files changed

+118
-30
lines changed

‎java/ql/src/semmle/code/java/dataflow/Nullness.qll‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,21 @@ private ControlFlowNode varDereference(SsaVariable v, VarAccess va) {
143143
* subsequent use, either by dereferencing it or by an assertion.
144144
*/
145145
private ControlFlowNode ensureNotNull(SsaVariable v) {
146-
result = varDereference(v, _) or
147-
result.(AssertStmt).getExpr() = nullGuard(v, true, false) or
148-
exists(AssertTrueMethod m | result = m.getACheck(nullGuard(v, true, false))) or
149-
exists(AssertFalseMethod m | result = m.getACheck(nullGuard(v, false, false))) or
146+
result = varDereference(v, _)
147+
or
148+
result.(AssertStmt).getExpr() = nullGuard(v, true, false)
149+
or
150+
exists(AssertTrueMethod m | result = m.getACheck(nullGuard(v, true, false)))
151+
or
152+
exists(AssertFalseMethod m | result = m.getACheck(nullGuard(v, false, false)))
153+
or
150154
exists(AssertNotNullMethod m | result = m.getACheck(v.getAUse()))
155+
or
156+
exists(AssertThatMethod m, MethodAccess ma |
157+
result = m.getACheck(v.getAUse()) and ma.getControlFlowNode() = result
158+
|
159+
ma.getAnArgument().(MethodAccess).getMethod().getName() = "notNullValue"
160+
)
151161
}
152162

153163
/**

‎java/ql/src/semmle/code/java/frameworks/Assertions.qll‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ newtype AssertKind =
1111
AssertKindTrue() or
1212
AssertKindFalse() or
1313
AssertKindNotNull() or
14-
AssertKindFail()
14+
AssertKindFail() or
15+
AssertKindThat()
1516

1617
private predicate assertionMethod(Method m, AssertKind kind) {
1718
exists(RefType junit |
@@ -27,6 +28,13 @@ private predicate assertionMethod(Method m, AssertKind kind) {
2728
m.hasName("fail") and kind = AssertKindFail()
2829
)
2930
or
31+
exists(RefType hamcrest |
32+
m.getDeclaringType() = hamcrest and
33+
hamcrest.hasQualifiedName("org.hamcrest", "MatcherAssert")
34+
|
35+
m.hasName("assertThat") and kind = AssertKindThat()
36+
)
37+
or
3038
exists(RefType objects |
3139
m.getDeclaringType() = objects and
3240
objects.hasQualifiedName("java.util", "Objects")
@@ -82,6 +90,14 @@ class AssertFailMethod extends AssertionMethod {
8290
AssertFailMethod() { assertionMethod(this, AssertKindFail()) }
8391
}
8492

93+
/**
94+
* A method that asserts that its first argument has a property
95+
* given by its second argument.
96+
*/
97+
class AssertThatMethod extends AssertionMethod {
98+
AssertThatMethod() { assertionMethod(this, AssertKindThat()) }
99+
}
100+
85101
/** A trivially failing assertion. That is, `assert false` or its equivalents. */
86102
predicate assertFail(BasicBlock bb, ControlFlowNode n) {
87103
bb = n.getBasicBlock() and

‎java/ql/test/query-tests/Nullness/A.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import java.util.Iterator;
22
import java.util.List;
33
import static org.junit.Assert.*;
4+
import static org.hamcrest.core.IsNull.*;
5+
import static org.hamcrest.MatcherAssert.*;
46

57
public class A {
68
public void notTest() {
@@ -296,6 +298,12 @@ public void testForLoopCondition(Iterable iter) {
296298
for (it = iter.iterator(); !!it.hasNext(); ) {}
297299
}
298300

301+
public void assertThatTest() {
302+
Object assertThat_ok1 = maybe() ? null : new Object();
303+
assertThat(assertThat_ok1, notNullValue());
304+
assertThat_ok1.toString();
305+
}
306+
299307
private boolean m;
300308
A(boolean m) {
301309
this.m = m;

‎java/ql/test/query-tests/Nullness/B.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public void missedGuard(Object obj) {
120120
}
121121

122122
private Object mkMaybe() {
123-
if (maybe) throw new Exception();
123+
if (maybe) throw new RuntimeException();
124124
return new Object();
125125
}
126126

@@ -267,7 +267,7 @@ else if(obj.hashCode() > 7) { // OK
267267
}
268268
if(msg != null) {
269269
msg += "foobar";
270-
throw new Exception(msg);
270+
throw new RuntimeException(msg);
271271
}
272272
obj.hashCode(); // OK
273273
}
@@ -286,7 +286,7 @@ public void loopCorr(int iters) {
286286

287287
int[] b = maybe ? null : new int[iters];
288288
if (iters > 0 && (b == null || b.length < iters)) {
289-
throw new Exception();
289+
throw new RuntimeException();
290290
}
291291
for (int i = 0; i < iters; ++i) {
292292
b[i] = 0; // NPE - false positive

‎java/ql/test/query-tests/Nullness/C.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public void ex6(int[] vals, boolean b1, boolean b2) {
8787
vals[0] = 0; // OK
8888
break;
8989
default:
90-
throw new Exception();
90+
throw new RuntimeException();
9191
}
9292
}
9393

‎java/ql/test/query-tests/Nullness/NullAlways.expected‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
| A.java:13:7:13:9 | not | Variable $@ is always null here. | A.java:11:5:11:22 | Object not | not |
2-
| A.java:95:18:95:36 | synchronized_always | Variable $@ is always null here. | A.java:94:5:94:38 | Object synchronized_always | synchronized_always |
3-
| A.java:159:26:159:34 | do_always | Variable $@ is always null here. | A.java:157:5:157:28 | String do_always | do_always |
4-
| A.java:165:26:165:34 | do_maybe1 | Variable $@ is always null here. | A.java:163:5:163:28 | String do_maybe1 | do_maybe1 |
5-
| A.java:185:26:185:37 | while_always | Variable $@ is always null here. | A.java:183:5:183:31 | String while_always | while_always |
6-
| A.java:205:26:205:34 | if_always | Variable $@ is always null here. | A.java:203:5:203:28 | String if_always | if_always |
7-
| A.java:221:24:221:29 | for_ok | Variable $@ is always null here. | A.java:217:5:217:19 | String for_ok | for_ok |
8-
| A.java:224:26:224:35 | for_always | Variable $@ is always null here. | A.java:223:10:223:33 | String for_always | for_always |
9-
| A.java:234:5:234:14 | array_null | Variable $@ is always null here. | A.java:233:5:233:28 | int[] array_null | array_null |
10-
| A.java:246:24:246:34 | arrayaccess | Variable $@ is always null here. | A.java:242:5:242:29 | int[] arrayaccess | arrayaccess |
11-
| A.java:247:24:247:34 | fieldaccess | Variable $@ is always null here. | A.java:243:5:243:32 | String[] fieldaccess | fieldaccess |
12-
| A.java:248:24:248:35 | methodaccess | Variable $@ is always null here. | A.java:244:5:244:31 | Object methodaccess | methodaccess |
13-
| A.java:262:21:262:30 | for_always | Variable $@ is always null here. | A.java:261:5:261:35 | List<String> for_always | for_always |
14-
| A.java:264:24:264:33 | for_always | Variable $@ is always null here. | A.java:261:5:261:35 | List<String> for_always | for_always |
1+
| A.java:15:7:15:9 | not | Variable $@ is always null here. | A.java:13:5:13:22 | Object not | not |
2+
| A.java:97:18:97:36 | synchronized_always | Variable $@ is always null here. | A.java:96:5:96:38 | Object synchronized_always | synchronized_always |
3+
| A.java:161:26:161:34 | do_always | Variable $@ is always null here. | A.java:159:5:159:28 | String do_always | do_always |
4+
| A.java:167:26:167:34 | do_maybe1 | Variable $@ is always null here. | A.java:165:5:165:28 | String do_maybe1 | do_maybe1 |
5+
| A.java:187:26:187:37 | while_always | Variable $@ is always null here. | A.java:185:5:185:31 | String while_always | while_always |
6+
| A.java:207:26:207:34 | if_always | Variable $@ is always null here. | A.java:205:5:205:28 | String if_always | if_always |
7+
| A.java:223:24:223:29 | for_ok | Variable $@ is always null here. | A.java:219:5:219:19 | String for_ok | for_ok |
8+
| A.java:226:26:226:35 | for_always | Variable $@ is always null here. | A.java:225:10:225:33 | String for_always | for_always |
9+
| A.java:236:5:236:14 | array_null | Variable $@ is always null here. | A.java:235:5:235:28 | int[] array_null | array_null |
10+
| A.java:248:24:248:34 | arrayaccess | Variable $@ is always null here. | A.java:244:5:244:29 | int[] arrayaccess | arrayaccess |
11+
| A.java:249:24:249:34 | fieldaccess | Variable $@ is always null here. | A.java:245:5:245:32 | String[] fieldaccess | fieldaccess |
12+
| A.java:250:24:250:35 | methodaccess | Variable $@ is always null here. | A.java:246:5:246:31 | Object methodaccess | methodaccess |
13+
| A.java:264:21:264:30 | for_always | Variable $@ is always null here. | A.java:263:5:263:35 | List<String> for_always | for_always |
14+
| A.java:266:24:266:33 | for_always | Variable $@ is always null here. | A.java:263:5:263:35 | List<String> for_always | for_always |
1515
| B.java:304:7:304:9 | ioe | Variable $@ is always null here. | B.java:297:5:297:25 | Exception ioe | ioe |

‎java/ql/test/query-tests/Nullness/NullMaybe.expected‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
| A.java:46:5:46:21 | assertNotNull_ok3 | Variable $@ may be null here because of $@ assignment. | A.java:44:5:44:61 | Object assertNotNull_ok3 | assertNotNull_ok3 | A.java:44:12:44:60 | assertNotNull_ok3 | this |
2-
| A.java:170:26:170:33 | do_maybe | Variable $@ may be null here because of $@ assignment. | A.java:168:5:168:25 | String do_maybe | do_maybe | A.java:171:7:171:21 | ...=... | this |
3-
| A.java:191:26:191:36 | while_maybe | Variable $@ may be null here because of $@ assignment. | A.java:189:5:189:28 | String while_maybe | while_maybe | A.java:192:7:192:24 | ...=... | this |
4-
| A.java:213:24:213:31 | if_maybe | Variable $@ may be null here because of $@ assignment. | A.java:209:5:209:25 | String if_maybe | if_maybe | A.java:211:7:211:21 | ...=... | this |
5-
| A.java:228:26:228:34 | for_maybe | Variable $@ may be null here because of $@ assignment. | A.java:227:10:227:30 | String for_maybe | for_maybe | A.java:227:35:227:50 | ...=... | this |
6-
| A.java:271:24:271:32 | for_maybe | Variable $@ may be null here because of $@ assignment. | A.java:266:5:266:63 | List<String> for_maybe | for_maybe | A.java:269:7:269:22 | ...=... | this |
1+
| A.java:48:5:48:21 | assertNotNull_ok3 | Variable $@ may be null here because of $@ assignment. | A.java:46:5:46:61 | Object assertNotNull_ok3 | assertNotNull_ok3 | A.java:46:12:46:60 | assertNotNull_ok3 | this |
2+
| A.java:172:26:172:33 | do_maybe | Variable $@ may be null here because of $@ assignment. | A.java:170:5:170:25 | String do_maybe | do_maybe | A.java:173:7:173:21 | ...=... | this |
3+
| A.java:193:26:193:36 | while_maybe | Variable $@ may be null here because of $@ assignment. | A.java:191:5:191:28 | String while_maybe | while_maybe | A.java:194:7:194:24 | ...=... | this |
4+
| A.java:215:24:215:31 | if_maybe | Variable $@ may be null here because of $@ assignment. | A.java:211:5:211:25 | String if_maybe | if_maybe | A.java:213:7:213:21 | ...=... | this |
5+
| A.java:230:26:230:34 | for_maybe | Variable $@ may be null here because of $@ assignment. | A.java:229:10:229:30 | String for_maybe | for_maybe | A.java:229:35:229:50 | ...=... | this |
6+
| A.java:273:24:273:32 | for_maybe | Variable $@ may be null here because of $@ assignment. | A.java:268:5:268:63 | List<String> for_maybe | for_maybe | A.java:271:7:271:22 | ...=... | this |
77
| B.java:16:5:16:9 | param | Variable $@ may be null here because of $@ null argument. | B.java:15:23:15:34 | param | param | B.java:11:13:11:16 | null | this |
88
| B.java:23:5:23:9 | param | Variable $@ may be null here as suggested by $@ null guard. | B.java:19:23:19:34 | param | param | B.java:20:9:20:21 | ... != ... | this |
99
| B.java:57:7:57:8 | o7 | Variable $@ may be null here because of $@ assignment. | B.java:52:5:52:34 | Object o7 | o7 | B.java:52:12:52:33 | o7 | this |

‎java/ql/test/query-tests/Nullness/options‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/junit-4.11:${testdir}/../../stubs/junit-jupiter-api-5.2.0
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/junit-4.11:${testdir}/../../stubs/hamcrest-2.2:${testdir}/../../stubs/junit-jupiter-api-5.2.0

‎java/ql/test/stubs/hamcrest-2.2/LICENSE.txt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
BSD License
2+
3+
Copyright (c) 2000-2015 www.hamcrest.org
4+
All rights reserved.
5+
6+
Redistribution and use in source and binary forms, with or without
7+
modification, are permitted provided that the following conditions are met:
8+
9+
Redistributions of source code must retain the above copyright notice, this list of
10+
conditions and the following disclaimer. Redistributions in binary form must reproduce
11+
the above copyright notice, this list of conditions and the following disclaimer in
12+
the documentation and/or other materials provided with the distribution.
13+
14+
Neither the name of Hamcrest nor the names of its contributors may be used to endorse
15+
or promote products derived from this software without specific prior written
16+
permission.
17+
18+
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
19+
EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
20+
OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT
21+
SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
22+
INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
23+
TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
24+
BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
25+
CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
26+
WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
27+
DAMAGE.

‎java/ql/test/stubs/hamcrest-2.2/org/hamcrest/MatcherAssert.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package org.hamcrest;
2+
3+
4+
public class MatcherAssert {
5+
public static <T> void assertThat(T actual, Object matcher) {
6+
assertThat("", actual, matcher);
7+
}
8+
9+
public static <T> void assertThat(String reason, T actual, Object matcher) {
10+
}
11+
12+
public static void assertThat(String reason, boolean assertion) {
13+
}
14+
}

0 commit comments

Comments
 (0)