Merge branch 'pr/3545' into 7.0.x
This commit is contained in:
4 files changed
+63
-31
No files matched your search
@@ -234,7 +234,7 @@
|
||||
<rule ref="category/java/errorprone.xml/JUnitStaticSuite"/>
|
||||
<!-- <rule ref="category/java/errorprone.xml/JumbledIncrementer"/> -->
|
||||
<!-- <rule ref="category/java/errorprone.xml/MethodWithSameNameAsEnclosingClass"/> -->
|
||||
<!-- <rule ref="category/java/errorprone.xml/MisplacedNullCheck"/> -->
|
||||
<rule ref="category/java/errorprone.xml/MisplacedNullCheck"/>
|
||||
<rule ref="category/java/errorprone.xml/MissingSerialVersionUID"/>
|
||||
<!-- <rule ref="category/java/errorprone.xml/MissingStaticMethodInNonInstantiatableClass"/> -->
|
||||
<!-- <rule ref="category/java/errorprone.xml/MoreThanOneLogger"/> -->
|
||||
|
||||
@@ -2211,37 +2211,41 @@ public class MyClass {
|
||||
class="net.sourceforge.pmd.lang.rule.XPathRule"
|
||||
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#misplacednullcheck">
|
||||
<description>
|
||||
The null check here is misplaced. If the variable is null a NullPointerException will be thrown.
|
||||
Either the check is useless (the variable will never be "null") or it is incorrect.
|
||||
The null check here is misplaced. If the variable is null a `NullPointerException` will be thrown.
|
||||
Either the check is useless (the variable will never be `null`) or it is incorrect.
|
||||
</description>
|
||||
<priority>3</priority>
|
||||
<properties>
|
||||
<property name="xpath">
|
||||
<value>
|
||||
<![CDATA[
|
||||
//ConditionalAndExpression
|
||||
/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 preceding-sibling::PrimaryExpression//Name
|
||||
[not(ancestor::ConditionalOrExpression/EqualityExpression[@Image = '=='])]
|
||||
/@Image
|
||||
satisfies starts-with($var, concat(PrimaryExpression/PrimaryPrefix/Name/@Image, '.'))]
|
||||
/PrimaryExpression/PrimaryPrefix/Name
|
||||
//InfixExpression[@Operator = '&&']
|
||||
/InfixExpression[@Operator = '!=']
|
||||
(: one side is null :)
|
||||
[NullLiteral]
|
||||
(: other side checks for the variable used somewhere in the first child of conditional and expression :)
|
||||
[VariableAccess]
|
||||
[some $var in preceding-sibling::*//VariableAccess
|
||||
[parent::MethodCall or parent::FieldAccess]
|
||||
[not(ancestor::InfixExpression[@Operator = '||'])]
|
||||
/@Name
|
||||
satisfies $var = VariableAccess/@Name
|
||||
]
|
||||
/VariableAccess
|
||||
|
|
||||
//ConditionalOrExpression
|
||||
/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 preceding-sibling::PrimaryExpression//Name
|
||||
[not(ancestor::ConditionalAndExpression/EqualityExpression[@Image = '!='])]
|
||||
/@Image
|
||||
satisfies starts-with($var, concat(PrimaryExpression/PrimaryPrefix/Name/@Image, '.'))]
|
||||
/PrimaryExpression/PrimaryPrefix/Name
|
||||
//InfixExpression[@Operator = '||']
|
||||
/InfixExpression[@Operator = '==']
|
||||
(: one side is null :)
|
||||
[NullLiteral]
|
||||
(: other side checks for the variable used somewhere in the first child of conditional or expression :)
|
||||
[VariableAccess]
|
||||
[some $var in preceding-sibling::*//VariableAccess
|
||||
[parent::MethodCall or parent::FieldAccess]
|
||||
[not(ancestor::InfixExpression[@Operator = '&&'])]
|
||||
/@Name
|
||||
satisfies $var = VariableAccess/@Name
|
||||
]
|
||||
/VariableAccess
|
||||
]]>
|
||||
</value>
|
||||
</property>
|
||||
|
||||
-1
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
|
||||
|
||||
import net.sourceforge.pmd.testframework.PmdRuleTst;
|
||||
|
||||
@org.junit.Ignore("Rule has not been updated yet")
|
||||
public class MisplacedNullCheckTest extends PmdRuleTst {
|
||||
// no additional unit tests
|
||||
}
|
||||
+34
-5
@@ -16,7 +16,7 @@
|
||||
</expected-messages>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
void bar() {
|
||||
void bar(Object a, Object baz) {
|
||||
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
|
||||
|
||||
@@ -42,7 +42,7 @@ public class Foo {
|
||||
</expected-messages>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
void bar() {
|
||||
void bar(Object a, Foo baz) {
|
||||
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
|
||||
|
||||
@@ -52,6 +52,8 @@ public class Foo {
|
||||
if (baz != null && a.equals(baz.foo())) {} // correct null check
|
||||
if (baz == null || a.equals(baz.foo())) {} // correct null check
|
||||
}
|
||||
|
||||
public Object foo() { return null; }
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
@@ -61,9 +63,11 @@ public class Foo {
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
void bar() {
|
||||
void bar(Object a) {
|
||||
if (a != null && a.equals(foo())) {}
|
||||
}
|
||||
|
||||
public Object foo() { return null; }
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
@@ -77,7 +81,7 @@ public class Foo {
|
||||
</expected-messages>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
void bar() {
|
||||
void bar(Object a, Object baz) {
|
||||
if (a.equals(baz) || a == null) {}
|
||||
}
|
||||
}
|
||||
@@ -88,7 +92,10 @@ public class Foo {
|
||||
<description>3372128: False positive: ArrayIsStoredDirectly</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
public class Foo {
|
||||
private List<String> excludeStatus;
|
||||
public final void setExcludeStatus(String[] excludeStatus) {
|
||||
if (excludeStatus != null) {
|
||||
this.excludeStatus = Arrays.asList(excludeStatus.clone());
|
||||
@@ -155,12 +162,13 @@ public class Test {
|
||||
<test-code>
|
||||
<description>False-positive/negative with multiple conditions</description>
|
||||
<expected-problems>2</expected-problems>
|
||||
<expected-linenumbers>7,8</expected-linenumbers>
|
||||
<expected-linenumbers>8,9</expected-linenumbers>
|
||||
<expected-messages>
|
||||
<message>The null check here is misplaced; if the variable 'attributes' is null there will be a NullPointerException</message>
|
||||
<message>The null check here is misplaced; if the variable 'attributes' is null there will be a NullPointerException</message>
|
||||
</expected-messages>
|
||||
<code><![CDATA[
|
||||
import java.util.Map;
|
||||
public class Test {
|
||||
public void method(String annotationType, Map<String, Object> attributes) {
|
||||
boolean isStereotype = annotationType.equals("javax.inject.Named");
|
||||
@@ -170,6 +178,27 @@ public class Test {
|
||||
if (isStereotype && attributes.containsKey("value") && attributes != null) {}
|
||||
if (isStereotype || attributes.containsKey("value") || attributes == null) {}
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>False positive with no dereferencing variable access</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
void bar(Object a, Object b, boolean aMustBePresent) {
|
||||
if (a == b || a == null) { } // there is no NPE happening here
|
||||
int aUndefined = (a == b || a == null) ? 1 : 0;
|
||||
if (a != b && a != null) { }
|
||||
if (a == this || a == null) { }
|
||||
for (Object p = a; p != b && p != null; p = p.prev) {}
|
||||
if (a != this &&
|
||||
a != null && b.equals(a.toString())) { } // potential NPE for b.equals, but that's not the point here
|
||||
if (a == b || a == null || (b == null && !aMustBePresent)) { }
|
||||
if ((a instanceof Foo) || (a == null)) { }
|
||||
while (Object.class != a && a != null) { }
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
Reference in new issue
Block a user