Merge pull request #3524 from adangel:pmd7-update-UnnecessaryCaseChange

[java] Update rule UnnecessaryCaseChange #3524
This commit is contained in:
Andreas Dangel
2021-10-08 09:17:58 +02:00
4 changed files with 39 additions and 108 deletions

View File

@ -258,7 +258,7 @@
<rule ref="category/java/errorprone.xml/TestClassWithoutTestCases"/>
<!-- <rule ref="category/java/errorprone.xml/UnconditionalIfStatement"/> -->
<!-- <rule ref="category/java/errorprone.xml/UnnecessaryBooleanAssertion"/> -->
<!-- <rule ref="category/java/errorprone.xml/UnnecessaryCaseChange"/> -->
<rule ref="category/java/errorprone.xml/UnnecessaryCaseChange"/>
<rule ref="category/java/errorprone.xml/UnnecessaryConversionTemporary"/>
<rule ref="category/java/errorprone.xml/UnusedNullCheckInEquals"/>
<rule ref="category/java/errorprone.xml/UseCorrectExceptionLogging"/>

View File

@ -8,115 +8,40 @@ import static java.util.Arrays.asList;
import java.util.List;
import net.sourceforge.pmd.lang.java.ast.ASTArguments;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
public class UnnecessaryCaseChangeRule extends AbstractJavaRule {
public class UnnecessaryCaseChangeRule extends AbstractJavaRulechainRule {
private static final List<String> CASE_CHANGING_METHODS = asList("toLowerCase", "toUpperCase");
private static final List<String> EQUALITY_METHODS = asList("equals", "equalsIgnoreCase");
public UnnecessaryCaseChangeRule() {
super(ASTMethodCall.class);
}
@Override
public Object visit(ASTPrimaryExpression expr, Object data) {
if (hasUnnecessaryCaseChange(expr)) {
addViolation(data, expr);
}
return super.visit(expr, data);
}
private boolean hasUnnecessaryCaseChange(ASTPrimaryExpression expr) {
int equalsMethodCallIndex = getEqualsMethodCallIndex(expr);
if (equalsMethodCallIndex != -1) {
int equalsMethodCallArgsIndex = equalsMethodCallIndex + 1;
ASTPrimaryExpression equalsCallArgs = getMethodCallArgsAtPosition(expr, equalsMethodCallArgsIndex);
return anyHasCaseChangingMethodCall(expr, equalsCallArgs);
}
return false;
}
private int getEqualsMethodCallIndex(ASTPrimaryExpression expr) {
for (int callIndex = 0; callIndex < expr.getNumChildren(); callIndex++) {
JavaNode methodCall = expr.getChild(callIndex);
if (isEqualsMethodCall(methodCall)) {
return callIndex;
public Object visit(ASTMethodCall node, Object data) {
if (EQUALITY_METHODS.contains(node.getMethodName()) && node.getArguments().size() == 1) {
if (isCaseChangingMethodCall(node.getQualifier())
|| isCaseChangingMethodCall(node.getArguments().get(0))) {
addViolation(data, node);
}
}
return -1;
return data;
}
private boolean isEqualsMethodCall(JavaNode methodCall) {
return calledMethodHasNameFromList(methodCall, EQUALITY_METHODS);
}
private ASTPrimaryExpression getMethodCallArgsAtPosition(ASTPrimaryExpression expr, int argsPos) {
if (hasChildAtPosition(expr, argsPos)) {
JavaNode methodCallArgs = expr.getChild(argsPos);
return methodCallArgs.getFirstDescendantOfType(ASTPrimaryExpression.class);
}
return null;
}
private boolean hasChildAtPosition(ASTPrimaryExpression expr, int pos) {
return expr.getNumChildren() > pos;
}
private boolean anyHasCaseChangingMethodCall(ASTPrimaryExpression ... exprs) {
for (ASTPrimaryExpression expr : exprs) {
if (expr != null && hasCaseChangingMethodCall(expr)) {
return true;
}
/**
* Checks for toLower/UpperCase method calls without arguments.
* These method take an optional Locale as an argument - in that case,
* these case conversions are considered deliberate.
*/
private boolean isCaseChangingMethodCall(ASTExpression expr) {
if (expr instanceof ASTMethodCall) {
ASTMethodCall call = (ASTMethodCall) expr;
return CASE_CHANGING_METHODS.contains(call.getMethodName()) && call.getArguments().size() == 0;
}
return false;
}
private boolean hasCaseChangingMethodCall(ASTPrimaryExpression expr) {
for (int callArgsIndex = 1; callArgsIndex < expr.getNumChildren(); callArgsIndex++) {
JavaNode methodCall = expr.getChild(callArgsIndex - 1);
JavaNode methodCallArgs = expr.getChild(callArgsIndex);
if (isCaseChangingMethodCall(methodCall, methodCallArgs)) {
return true;
}
}
return false;
}
private boolean isCaseChangingMethodCall(JavaNode methodCall, JavaNode methodCallArgs) {
if (calledMethodHasNameFromList(methodCall, CASE_CHANGING_METHODS)) {
ASTArguments args = methodCallArgs.getFirstDescendantOfType(ASTArguments.class);
return args != null && args.size() == 0;
}
return false;
}
private boolean calledMethodHasNameFromList(JavaNode methodCall, List<String> nameList) {
String methodName = getCalledMethodName(methodCall);
if (methodName != null) {
for (String nameFromList : nameList) {
if (methodName.endsWith(nameFromList)) {
return true;
}
}
}
return false;
}
private String getCalledMethodName(JavaNode methodCall) {
String methodName = methodCall.getImage();
if (methodName == null) {
ASTName name = methodCall.getFirstDescendantOfType(ASTName.class);
return name != null ? methodNameFromCallImage(name.getImage()) : null;
}
return methodName;
}
private String methodNameFromCallImage(String methodCallImage) {
if (methodCallImage.contains(".")) {
String[] callParts = methodCallImage.split("\\.");
return callParts[1];
}
return methodCallImage;
}
}

View File

@ -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 UnnecessaryCaseChangeTest extends PmdRuleTst {
// no additional unit tests
}

View File

@ -10,7 +10,7 @@
<code><![CDATA[
public class Foo {
private boolean baz(String buz) {
return foo.toUpperCase().equals("foo");
return buz.toUpperCase().equals("foo");
}
}
]]></code>
@ -22,7 +22,7 @@ public class Foo {
<code><![CDATA[
public class Foo {
private boolean baz(String buz) {
return foo.toLowerCase().equals("foo");
return buz.toLowerCase().equals("foo");
}
}
]]></code>
@ -34,7 +34,7 @@ public class Foo {
<code><![CDATA[
public class Foo {
private boolean baz(String buz) {
return foo.toUpperCase().equalsIgnoreCase("foo");
return buz.toUpperCase().equalsIgnoreCase("foo");
}
}
]]></code>
@ -46,7 +46,7 @@ public class Foo {
<code><![CDATA[
public class Foo {
private boolean baz(String buz) {
return foo.toUpperCase(locale).equals("foo");
return buz.toUpperCase(locale).equals("foo");
}
}
]]></code>
@ -57,13 +57,17 @@ public class Foo {
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
public Object visit(ASTFieldDeclaration node, Object data) {
ASTClassOrInterfaceDeclaration cl = node.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class);
public Object visit(Foo node, Object data) {
Foo cl = node.getFirstParentOfType(Foo.class);
if (cl != null && node.getVariableName().toLowerCase().equals(cl.getImage().toLowerCase())) {
addViolation(data, node);
}
return data;
}
public Foo getFirstParentOfType(Class<?> clazz) { return null; }
public String getVariableName() { return null; }
public String getImage() { return null; }
}
]]></code>
</test-code>
@ -86,8 +90,11 @@ public class Foo {
<description>NullPointerException if in constructor test</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void bar(String s) {
public class Person {
public Person(String name) { }
public void printData() { }
public static void bar(String s) {
Person p = new Person(s.toUpperCase());
p.printData();
}