diff --git a/.ci/files/all-java.xml b/.ci/files/all-java.xml
index e93a6040c0..6bf302b1f1 100644
--- a/.ci/files/all-java.xml
+++ b/.ci/files/all-java.xml
@@ -258,7 +258,7 @@
-
+
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnnecessaryCaseChangeRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnnecessaryCaseChangeRule.java
index 62512bedc8..510a0a1142 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnnecessaryCaseChangeRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnnecessaryCaseChangeRule.java
@@ -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 CASE_CHANGING_METHODS = asList("toLowerCase", "toUpperCase");
private static final List 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 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;
- }
}
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnnecessaryCaseChangeTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnnecessaryCaseChangeTest.java
index 5bd4304f20..4cbb6ea7e3 100644
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnnecessaryCaseChangeTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/UnnecessaryCaseChangeTest.java
@@ -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
}
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UnnecessaryCaseChange.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UnnecessaryCaseChange.xml
index d3daf483ca..8bbb435a24 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UnnecessaryCaseChange.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UnnecessaryCaseChange.xml
@@ -10,7 +10,7 @@
@@ -22,7 +22,7 @@ public class Foo {
@@ -34,7 +34,7 @@ public class Foo {
@@ -46,7 +46,7 @@ public class Foo {
@@ -57,13 +57,17 @@ public class Foo {
1
clazz) { return null; }
+ public String getVariableName() { return null; }
+ public String getImage() { return null; }
}
]]>
@@ -86,8 +90,11 @@ public class Foo {
NullPointerException if in constructor test
0