Skip to content

Commit df3a60b

Browse files
authored
Add SillyEqualsCheck recipe (RSPEC-S2159) (#834)
* Add SillyEqualsCheck recipe (RSPEC-S2159) Implements SonarQube rule S2159: detects `.equals()` calls that always return false due to incompatible types. The recipe transforms: - `x.equals(null)` → `x == null` (with negation handling) - `arr1.equals(arr2)` → `Arrays.equals(arr1, arr2)` for arrays Unrelated type and array-vs-non-array comparisons are flagged as search results since they represent bugs that cannot be safely auto-fixed. * Fix dropParentUntil crash, use deepEquals for multidimensional arrays - Broaden dropParentUntil predicate to handle equals(null) in if-conditions, assignments, variable declarations, and other contexts - Use Arrays.deepEquals() instead of Arrays.equals() for multidimensional arrays - Add tests for multidimensional arrays, if-condition, and method select * Simplify dropParentUntil to parent cursor walk Replace fragile dropParentUntil predicate listing 12 specific AST types with a simple parent cursor walk through J.Parentheses nodes. This cannot throw on unlisted AST contexts. * Remove redundant @Getter annotations implied by @value * Use J.Literal.isLiteralValue(arg, null) for null check
1 parent e59af63 commit df3a60b

3 files changed

Lines changed: 501 additions & 0 deletions

File tree

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.openrewrite.staticanalysis;
17+
18+
import lombok.EqualsAndHashCode;
19+
import lombok.Value;
20+
import org.openrewrite.*;
21+
import org.openrewrite.java.JavaTemplate;
22+
import org.openrewrite.java.JavaVisitor;
23+
import org.openrewrite.java.MethodMatcher;
24+
import org.openrewrite.java.search.UsesMethod;
25+
import org.openrewrite.java.tree.Expression;
26+
import org.openrewrite.java.tree.J;
27+
import org.openrewrite.java.tree.JavaType;
28+
import org.openrewrite.java.tree.TypeUtils;
29+
import org.openrewrite.marker.SearchResult;
30+
31+
import java.time.Duration;
32+
import java.util.Set;
33+
34+
import static java.util.Collections.singleton;
35+
36+
@Value
37+
@EqualsAndHashCode(callSuper = false)
38+
public class SillyEqualsCheck extends Recipe {
39+
private static final MethodMatcher EQUALS_MATCHER = new MethodMatcher("java.lang.Object equals(java.lang.Object)", true);
40+
41+
final String displayName = "Silly equality checks should not be made";
42+
43+
final String description = "Detects `.equals()` calls that compare incompatible types and will always return `false`. " +
44+
"Replaces `.equals(null)` with `== null` and array `.equals()` with `Arrays.equals()`. " +
45+
"Flags comparisons between unrelated types or between arrays and non-arrays.";
46+
47+
final Set<String> tags = singleton("RSPEC-S2159");
48+
49+
final Duration estimatedEffortPerOccurrence = Duration.ofMinutes(5);
50+
51+
@Override
52+
public TreeVisitor<?, ExecutionContext> getVisitor() {
53+
return Preconditions.check(new UsesMethod<>(EQUALS_MATCHER), new JavaVisitor<ExecutionContext>() {
54+
@Override
55+
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
56+
J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
57+
if (!EQUALS_MATCHER.matches(mi) || mi.getSelect() == null) {
58+
return mi;
59+
}
60+
61+
Expression arg = mi.getArguments().get(0);
62+
63+
// Case: x.equals(null) -> x == null
64+
if (J.Literal.isLiteralValue(arg, null)) {
65+
return replaceWithEqualityCheck(mi);
66+
}
67+
68+
JavaType selectType = mi.getSelect().getType();
69+
JavaType argType = arg.getType();
70+
if (selectType == null || argType == null) {
71+
return mi;
72+
}
73+
74+
JavaType.Array selectArray = TypeUtils.asArray(selectType);
75+
JavaType.Array argArray = TypeUtils.asArray(argType);
76+
77+
// Case: both arrays -> Arrays.equals() or Arrays.deepEquals() for multidimensional
78+
if (selectArray != null && argArray != null) {
79+
maybeAddImport("java.util.Arrays");
80+
boolean multidimensional = selectArray.getElemType() instanceof JavaType.Array;
81+
String arrayMethod = multidimensional ? "deepEquals" : "equals";
82+
return JavaTemplate.builder("Arrays." + arrayMethod + "(#{any()}, #{any()})")
83+
.imports("java.util.Arrays")
84+
.build()
85+
.apply(getCursor(), mi.getCoordinates().replace(), mi.getSelect(), arg);
86+
}
87+
88+
// Case: array vs non-array -> always false
89+
if (selectArray != null || argArray != null) {
90+
return SearchResult.found(mi, "Comparing array with non-array always returns false");
91+
}
92+
93+
// Case: unrelated types -> always false
94+
JavaType.FullyQualified selectFq = TypeUtils.asFullyQualified(selectType);
95+
JavaType.FullyQualified argFq = TypeUtils.asFullyQualified(argType);
96+
if (selectFq == null || argFq == null) {
97+
return mi;
98+
}
99+
if (TypeUtils.isOfClassType(selectType, "java.lang.Object") ||
100+
TypeUtils.isOfClassType(argType, "java.lang.Object")) {
101+
return mi;
102+
}
103+
if (!TypeUtils.isAssignableTo(selectFq, argFq) &&
104+
!TypeUtils.isAssignableTo(argFq, selectFq)) {
105+
return SearchResult.found(mi, "Comparing unrelated types " +
106+
selectFq.getFullyQualifiedName() + " and " +
107+
argFq.getFullyQualifiedName() + " always returns false");
108+
}
109+
110+
return mi;
111+
}
112+
113+
private J replaceWithEqualityCheck(J.MethodInvocation mi) {
114+
Cursor parent = getCursor().getParentTreeCursor();
115+
while (parent.getValue() instanceof J.Parentheses) {
116+
parent = parent.getParentTreeCursor();
117+
}
118+
boolean isNot = parent.getValue() instanceof J.Unary &&
119+
((J.Unary) parent.getValue()).getOperator() == J.Unary.Type.Not;
120+
if (isNot) {
121+
parent.putMessage("REMOVE_UNARY_NOT", parent.getValue());
122+
}
123+
String operator = isNot ? "!=" : "==";
124+
return JavaTemplate.apply("#{any()} " + operator + " null",
125+
updateCursor(mi),
126+
mi.getCoordinates().replace(),
127+
mi.getSelect());
128+
}
129+
130+
@Override
131+
public J visitUnary(J.Unary unary, ExecutionContext ctx) {
132+
J j = super.visitUnary(unary, ctx);
133+
J.Unary asUnary = (J.Unary) j;
134+
if (asUnary.equals(getCursor().pollMessage("REMOVE_UNARY_NOT"))) {
135+
return asUnary.getExpression().unwrap().withPrefix(asUnary.getPrefix());
136+
}
137+
return j;
138+
}
139+
});
140+
}
141+
}

src/main/resources/META-INF/rewrite/recipes.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly
137137
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceTextBlockWithString,Replace text block with regular string,Replace text block with a regular multi-line string. Text blocks that fit on a single line without concatenation or escaped newlines gain no readability benefit from the triple-quote syntax and are clearer as plain string literals.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
138138
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceValidateNotNullHavingVarargsWithObjectsRequireNonNull,Replace `org.apache.commons.lang3.Validate#notNull` with `Objects#requireNonNull`,"Replace `org.apache.commons.lang3.Validate.notNull(Object, String, Object[])` with `Objects.requireNonNull(Object, String)`.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
139139
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceWeekYearWithYear,Week Year (YYYY) should not be used for date formatting,"For most dates Week Year (YYYY) and Year (yyyy) yield the same results. However, on the last week of December and the first week of January, Week Year could produce unexpected results. This is a common source of off-by-one-year bugs that typically only manifest around New Year's Eve, making them difficult to catch during development and testing.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
140+
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.SillyEqualsCheck,Silly equality checks should not be made,Detects `.equals()` calls that compare incompatible types and will always return `false`. Replaces `.equals(null)` with `== null` and array `.equals()` with `Arrays.equals()`. Flags comparisons between unrelated types or between arrays and non-arrays.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
140141
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.SimplifyArraysAsList,Simplify `Arrays.asList(..)` with varargs,"Simplifies `Arrays.asList()` method calls that use explicit array creation to use varargs instead. For example, `Arrays.asList(new String[]{""a"", ""b"", ""c""})` becomes `Arrays.asList(""a"", ""b"", ""c"")`. Explicitly constructing an array to pass to a varargs parameter adds visual clutter without changing behavior, since the compiler generates the array automatically.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
141142
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.SimplifyBooleanExpression,Simplify boolean expression,"Checks for overly complicated boolean expressions, such as `if (b == true)`, `b || true`, `!false`, etc. Needlessly complex boolean logic makes code harder to reason about and increases the chance of introducing errors during future modifications.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,
142143
maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.SimplifyBooleanExpressionWithDeMorgan,Simplify boolean expressions using De Morgan's laws,"Applies De Morgan's laws to simplify boolean expressions with negation. Transforms `!(a && b)` to `!a || !b` and `!(a || b)` to `!a && !b`. Distributing negations inward eliminates the outer `!` and makes each individual condition's polarity immediately visible, which aids comprehension.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,

0 commit comments

Comments
 (0)