diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 29a0de03a0..f4675ed704 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -125,6 +125,7 @@ Other languages are equivalent. * [#1003](https://github.com/pmd/pmd/issues/1003): \[java] UnnecessaryConstructor triggered on required empty constructor (Dagger @Inject) * [#1023](https://github.com/pmd/pmd/issues/1023): \[java] False positive for useless parenthesis * java-errorprone + * [#629](https://github.com/pmd/pmd/issues/629): \[java] NullAssignment false positive * [#816](https://github.com/pmd/pmd/issues/816): \[java] SingleMethodSingleton false positives with inner classes * java-performance * [#586](https://github.com/pmd/pmd/issues/586): \[java] AvoidUsingShortType erroneously triggered on overrides of 3rd party methods diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/NullAssignmentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/NullAssignmentRule.java index 9cbf691a06..668e98eca4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/NullAssignmentRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/NullAssignmentRule.java @@ -5,21 +5,26 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; +import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression; import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression; import net.sourceforge.pmd.lang.java.ast.ASTExpression; +import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral; +import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; +import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer; import net.sourceforge.pmd.lang.java.ast.AccessNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; -// TODO - should check that this is not the first assignment. e.g., this is OK: -// Object x; -// x = null; public class NullAssignmentRule extends AbstractJavaRule { + public NullAssignmentRule() { + addRuleChainVisit(ASTNullLiteral.class); + } + @Override public Object visit(ASTNullLiteral node, Object data) { @@ -55,7 +60,19 @@ public class NullAssignmentRule extends AbstractJavaRule { && ((AccessNode) ((VariableNameDeclaration) name.getNameDeclaration()).getAccessNodeParent()).isFinal(); } - private boolean isBadTernary(ASTConditionalExpression n) { - return n.isTernary() && !(n.jjtGetChild(0) instanceof ASTEqualityExpression); + private boolean isBadTernary(ASTConditionalExpression ternary) { + boolean isInitializer = false; + + ASTVariableInitializer variableInitializer = ternary.getFirstParentOfType(ASTVariableInitializer.class); + if (variableInitializer != null) { + ASTBlockStatement statement = ternary.getFirstParentOfType(ASTBlockStatement.class); + isInitializer = statement == variableInitializer.getFirstParentOfType(ASTBlockStatement.class); + } + + return ternary.isTernary() + && !(ternary.jjtGetChild(0) instanceof ASTEqualityExpression) + && !isInitializer + && !(ternary.getNthParent(2) instanceof ASTReturnStatement) + && !(ternary.getNthParent(2) instanceof ASTLambdaExpression); } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/NullAssignment.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/NullAssignment.xml index bc183811ac..ef656c0d50 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/NullAssignment.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/NullAssignment.xml @@ -64,10 +64,8 @@ public class Foo { ]]> - - 1 + null assignment in ternary - initialization + 0 - + null assignment in ternary 1 + + + null assignment in ternary, part deux - initialization + 0 + + + + null assignment in ternary, part deux + 1 + @@ -129,4 +149,41 @@ public class Foo { } ]]> + + + [java] NullAssignment false positive - initialization #629 + 0 + test ? truthy : null); + } +} + ]]> + + + + [java] NullAssignment false positive - no direct assignment, but lambda #629 + 0 + test ? truthy : null); + } +} + ]]> + + + + [java] NullAssignment false positive - return with ternary #629 + 0 + +