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

Fix #278 - make ConfusingTernary treat != null as a positive condition

Browse Source 
update release notes
This commit is contained in:
Clément Fournier
2022-02-08 19:41:01 +01:00
parent 1290999ec2
commit c85154adfc
3 changed files with 55 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
docs/pages/release_notes.md
View File

@ -16,6 +16,9 @@ This is a {{ site.pmd.release_type }} release.
### Fixed Issues
* java-codestyle
* [#278](https://github.com/pmd/pmd/issues/278): \[java] ConfusingTernary should treat `!= null` as positive condition
### API Changes
### External Contributions

29
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java
View File

@ -6,13 +6,14 @@ package net.sourceforge.pmd.lang.java.rule.codestyle;
import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression;
import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression;
import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression;
import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpressionNotPlusMinus;
@ -55,6 +56,7 @@ import net.sourceforge.pmd.properties.PropertyDescriptor;
* </pre>
*/
public class ConfusingTernaryRule extends AbstractJavaRule {
private static PropertyDescriptor<Boolean> ignoreElseIfProperty = booleanProperty("ignoreElseIf").desc("Ignore conditions with an else-if case").defaultValue(false).build();
public ConfusingTernaryRule() {
@ -100,14 +102,33 @@ public class ConfusingTernaryRule extends AbstractJavaRule {
return isUnaryNot(node) || isNotEquals(node) || isConditionalWithAllMatches(node);
}
private static boolean isUnaryNot(Node node) {
private static boolean isUnaryNot(JavaNode node) {
// look for "!x"
return node instanceof ASTUnaryExpressionNotPlusMinus && "!".equals(node.getImage());
}
private static boolean isNotEquals(Node node) {
private static boolean isNotEquals(JavaNode node) {
// look for "x != y"
return node instanceof ASTEqualityExpression && "!=".equals(node.getImage());
return node instanceof ASTEqualityExpression && "!=".equals(node.getImage())
&& !isNullLiteral(node.getChild(0))
&& !isNullLiteral(node.getChild(1));
}
private static boolean isNullLiteral(JavaNode node) {
node = unwrapParentheses(node);
if (node instanceof ASTExpression && node.getNumChildren() == 1) {
node = node.getChild(0);
}
if (node instanceof ASTPrimaryExpression && node.getNumChildren() == 1) {
node = node.getChild(0);
if (node instanceof ASTPrimaryPrefix && node.getNumChildren() == 1) {
node = node.getChild(0);
if (node instanceof ASTLiteral && node.getNumChildren() == 1) {
return node.getChild(0) instanceof ASTNullLiteral;
}
}
}
return false;
}
private static boolean isConditionalWithAllMatches(JavaNode node) {

27
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/ConfusingTernary.xml
View File

@ -28,6 +28,33 @@ public class Foo {
]]></code>
</test-code>
<test-code>
<description>null check, ok (if) - #278</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
String bar2(Object a) {
if (a != null)
return a.toString();
else
return null;
}
}
]]></code>
</test-code>
<test-code>
<description>null check, ok (ternary) - #278</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
String bar(Object a) {
return a != null ? a.toString() : null;
}
}
]]></code>
</test-code>
<test-code>
<description>!= inside if, bad</description>
<expected-problems>2</expected-problems>