phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[java] MisplacedNullCheck - fix false positive

Browse Source 
* Upgrade to XPath 2.0
* Extend test cases
* Report on variable that is being null check and
  improve violation message

Fixes #2242
This commit is contained in:
Andreas Dangel
2020-02-13 15:04:27 +01:00
parent 86ef30f26a
commit 00c0d3a713
2 changed files with 79 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
pmd-java/src/main/resources/category/java/errorprone.xml
View File

@ -2288,7 +2288,7 @@ public class MyClass {
<rule name="MisplacedNullCheck"
language="java"
since="3.5"
message="The null check here is misplaced; if the variable is null there will be a NullPointerException"
message="The null check here is misplaced; if the variable ''{0}'' is null there will be a NullPointerException"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#misplacednullcheck">
<description>
@ -2297,26 +2297,39 @@ Either the check is useless (the variable will never be "null") or it is incorre
</description>
<priority>3</priority>
<properties>
<property name="version" value="2.0"/>
<property name="xpath">
<value>
<![CDATA[
//Expression
/*[self::ConditionalOrExpression or self::ConditionalAndExpression]
/descendant::PrimaryExpression/PrimaryPrefix
/Name[starts-with(@Image,
concat(ancestor::PrimaryExpression/following-sibling::EqualityExpression
[./PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
/PrimaryExpression/PrimaryPrefix
/Name[count(../../PrimarySuffix)=0]/@Image,".")
)
]
[count(ancestor::ConditionalAndExpression/EqualityExpression
[@Image='!=']
[./PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
[starts-with(following-sibling::*/PrimaryExpression/PrimaryPrefix/Name/@Image,
concat(./PrimaryExpression/PrimaryPrefix/Name/@Image, '.'))]
) = 0
]
//ConditionalAndExpression
(: first child is PrimaryExpression :)
[*[1][self::PrimaryExpression]]
(: second child is EqualityExpression != null :)
[*[2][self::EqualityExpression][@Image = '!=']
(: one side is null :)
[PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
(: other side checks for the variable used somewhere in the first child of conditional and expression :)
[some $var in ../PrimaryExpression//Name
[not(ancestor::ConditionalOrExpression/EqualityExpression[@Image = '=='])]
/@Image
satisfies starts-with($var, concat(PrimaryExpression/PrimaryPrefix/Name/@Image, '.'))]
]
/EqualityExpression/PrimaryExpression/PrimaryPrefix/Name
|
//ConditionalOrExpression
(: first child is PrimaryExpression :)
[*[1][self::PrimaryExpression]]
(: second child is EqualityExpression == null :)
[*[2][self::EqualityExpression][@Image = '==']
(: one side is null :)
[PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
(: other side checks for the variable used somewhere in the first child of conditional or expression :)
[some $var in ../PrimaryExpression//Name
[not(ancestor::ConditionalAndExpression/EqualityExpression[@Image = '!='])]
/@Image
satisfies starts-with($var, concat(PrimaryExpression/PrimaryPrefix/Name/@Image, '.'))]
]
/EqualityExpression/PrimaryExpression/PrimaryPrefix/Name
]]>
</value>
</property>
@ -2325,8 +2338,10 @@ Either the check is useless (the variable will never be "null") or it is incorre
<![CDATA[
public class Foo {
void bar() {
if (a.equals(baz) && a != null) {}
}
if (a.equals(baz) && a != null) {} // a could be null, misplaced null check
if (a != null && a.equals(baz)) {} // correct null check
}
}
]]>
</example>
@ -2334,7 +2349,9 @@ public class Foo {
<![CDATA[
public class Foo {
void bar() {
if (a.equals(baz) || a == null) {}
if (a.equals(baz) || a == null) {} // a could be null, misplaced null check
if (a == null || a.equals(baz)) {} // correct null check
}
}
]]>

51
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/MisplacedNullCheck.xml
View File

@ -6,26 +6,52 @@
<test-code>
<description>null check after method invocation with conditional AND and !=</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<expected-problems>4</expected-problems>
<expected-linenumbers>3,4,6,7</expected-linenumbers>
<expected-messages>
<message>The null check here is misplaced; if the variable 'a' is null there will be a NullPointerException</message>
<message>The null check here is misplaced; if the variable 'a' is null there will be a NullPointerException</message>
<message>The null check here is misplaced; if the variable 'a' is null there will be a NullPointerException</message>
<message>The null check here is misplaced; if the variable 'a' is null there will be a NullPointerException</message>
</expected-messages>
<code><![CDATA[
public class Foo {
void bar() {
if (a.equals(baz) && a!=null) {}
}
void bar() {
if (a.equals(baz) && a != null) {} // a could be null, misplaced null check
if (a.equals(baz) || a == null) {} // a could be null, misplaced null check
if (a.equals(baz) && null != a) {} // a could be null, misplaced null check
if (a.equals(baz) || null == a) {} // a could be null, misplaced null check
if (a != null && a.equals(baz)) {} // correct null check
if (a == null || a.equals(baz)) {} // correct null check
}
}
]]></code>
</test-code>
<test-code>
<description>null check after nested method invocation</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<expected-problems>4</expected-problems>
<expected-linenumbers>3,4,6,7</expected-linenumbers>
<expected-messages>
<message>The null check here is misplaced; if the variable 'baz' is null there will be a NullPointerException</message>
<message>The null check here is misplaced; if the variable 'baz' is null there will be a NullPointerException</message>
<message>The null check here is misplaced; if the variable 'baz' is null there will be a NullPointerException</message>
<message>The null check here is misplaced; if the variable 'baz' is null there will be a NullPointerException</message>
</expected-messages>
<code><![CDATA[
public class Foo {
void bar() {
if (a.equals(baz.foo()) && baz != null) {}
}
void bar() {
if (a.equals(baz.foo()) && baz != null) {} // baz could be null, misplaced null check
if (a.equals(baz.foo()) || baz == null) {} // baz could be null, misplaced null check
if (a.equals(baz.foo()) && null != baz) {} // baz could be null, misplaced null check
if (a.equals(baz.foo()) || null == baz) {} // baz could be null, misplaced null check
if (baz != null && a.equals(baz.foo())) {} // correct null check
if (baz == null || a.equals(baz.foo())) {} // correct null check
}
}
]]></code>
</test-code>
@ -46,6 +72,9 @@ public class Foo {
<description>1610730: null check after method invocation with conditional OR and ==</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<expected-messages>
<message>The null check here is misplaced; if the variable 'a' is null there will be a NullPointerException</message>
</expected-messages>
<code><![CDATA[
public class Foo {
void bar() {
@ -80,6 +109,8 @@ public class Test {
if ((value != null && !value.equals(oldValue)) || value == null) {
// Do something
}
if ((value == null || !value.equals(oldValue)) && value != null) {}
}
}
]]></code>