Skip to content

Commit c26c8a3

Browse files
motlinmotlin
committed
Skip wrapper-primitive in UseAssertSame; rewrite via AssertTrueComparisonToAssertEquals
`UseAssertSame` was rewriting `assertTrue(wrapper == primitive)` to `assertSame(wrapper, primitive)`, which fails: Java unboxes for `==`, but `assertSame` autoboxes the primitive into a new wrapper and compares by reference. Closes the gap left by #992 (primitive-primitive only) by also handling the mixed wrapper-primitive form, with `AssertTrueComparisonToAssertEquals` extended to accept it.
1 parent 22b9122 commit c26c8a3

4 files changed

Lines changed: 143 additions & 11 deletions

File tree

‎src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueComparisonToAssertEquals.java‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.openrewrite.java.tree.Expression;
2929
import org.openrewrite.java.tree.J;
3030
import org.openrewrite.java.tree.JavaType;
31+
import org.openrewrite.java.tree.TypeUtils;
3132

3233
public class AssertTrueComparisonToAssertEquals extends Recipe {
3334
private static final MethodMatcher ASSERT_TRUE = new MethodMatcher(
@@ -99,14 +100,36 @@ private boolean isEqualBinary(J.MethodInvocation method) {
99100
return false;
100101
}
101102

102-
// Prevent breaking identity comparison.
103-
// Objects that are compared with == should not be compared with `.equals()` instead.
104-
// Out of the primitives == is not allowed when both are of type String
105-
return binary.getLeft().getType() instanceof JavaType.Primitive &&
106-
binary.getRight().getType() instanceof JavaType.Primitive &&
107-
!(binary.getLeft().getType() == JavaType.Primitive.String &&
108-
binary.getRight().getType() == JavaType.Primitive.String);
103+
// Prevent breaking identity comparison: wrapper-wrapper and reference-reference == are
104+
// reference equality and belong to UseAssertSame, not assertEquals. We rewrite when:
105+
// - both operands are primitive (excluding String == String, which is reference identity), OR
106+
// - one operand is primitive and the other a numeric wrapper (Java unboxes; == is value).
107+
JavaType leftType = binary.getLeft().getType();
108+
JavaType rightType = binary.getRight().getType();
109+
if (leftType instanceof JavaType.Primitive && rightType instanceof JavaType.Primitive) {
110+
return !(leftType == JavaType.Primitive.String && rightType == JavaType.Primitive.String);
111+
}
112+
return (isPrimitiveValueType(leftType) && isNumericWrapper(rightType)) ||
113+
(isNumericWrapper(leftType) && isPrimitiveValueType(rightType));
109114
}
110115
});
111116
}
117+
118+
private static boolean isPrimitiveValueType(JavaType type) {
119+
return type instanceof JavaType.Primitive &&
120+
type != JavaType.Primitive.Null &&
121+
type != JavaType.Primitive.String &&
122+
type != JavaType.Primitive.None;
123+
}
124+
125+
private static boolean isNumericWrapper(JavaType type) {
126+
return TypeUtils.isOfClassType(type, "java.lang.Boolean") ||
127+
TypeUtils.isOfClassType(type, "java.lang.Byte") ||
128+
TypeUtils.isOfClassType(type, "java.lang.Character") ||
129+
TypeUtils.isOfClassType(type, "java.lang.Short") ||
130+
TypeUtils.isOfClassType(type, "java.lang.Integer") ||
131+
TypeUtils.isOfClassType(type, "java.lang.Long") ||
132+
TypeUtils.isOfClassType(type, "java.lang.Float") ||
133+
TypeUtils.isOfClassType(type, "java.lang.Double");
134+
}
112135
}

‎src/main/java/org/openrewrite/java/testing/junit5/UseAssertSame.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ public class UseAssertSame extends Recipe {
3636

3737
@Getter
3838
final String description = "Prefers the usage of `assertSame` or `assertNotSame` methods instead of using of vanilla `assertTrue` " +
39-
"or `assertFalse` with a boolean comparison.";
39+
"or `assertFalse` with a boolean comparison. Only applies when both operands are reference types — " +
40+
"primitive operands are handled by `AssertTrueComparisonToAssertEquals`.";
4041

4142
private static final MethodMatcher ASSERT_TRUE_MATCHER = new MethodMatcher("org.junit.jupiter.api.Assertions assertTrue(..)");
4243
private static final MethodMatcher ASSERT_FALSE_MATCHER = new MethodMatcher("org.junit.jupiter.api.Assertions assertFalse(..)");
@@ -67,9 +68,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat
6768
binary.getRight().getType() == JavaType.Primitive.Null) {
6869
return mi;
6970
}
70-
// Skip primitive comparisons — `==` is value equality, not reference equality
71-
if (binary.getLeft().getType() instanceof JavaType.Primitive ||
72-
binary.getRight().getType() instanceof JavaType.Primitive) {
71+
// Skip when either operand has value-equality semantics under == (primitives, plus
72+
// the wrapper-primitive case where Java unboxes). Defer to AssertTrueComparisonToAssertEquals.
73+
if (isPrimitiveValueType(binary.getLeft().getType()) ||
74+
isPrimitiveValueType(binary.getRight().getType())) {
7375
return mi;
7476
}
7577
List<Expression> newArguments = new ArrayList<>();
@@ -111,4 +113,12 @@ private JavaType.Method assertSameMethodType(J.MethodInvocation mi, String newMe
111113
visitor);
112114
}
113115

116+
// String is a JavaType.Primitive enum value but its == is reference equality (assertSame is correct).
117+
private static boolean isPrimitiveValueType(JavaType type) {
118+
return type instanceof JavaType.Primitive &&
119+
type != JavaType.Primitive.Null &&
120+
type != JavaType.Primitive.String &&
121+
type != JavaType.Primitive.None;
122+
}
123+
114124
}

‎src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueComparisonToAssertEqualsTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,4 +177,57 @@ void test() {
177177
);
178178
}
179179

180+
@SuppressWarnings({"NumberEquality", "SimplifiableAssertion"})
181+
@Test
182+
void onlyRewritesWhenAtLeastOneOperandIsPrimitive() {
183+
// Mixed primitive-wrapper rewrites because Java unboxes (== is value equality).
184+
// Wrapper-wrapper is reference equality and stays for UseAssertSame to emit assertSame.
185+
//language=java
186+
rewriteRun(
187+
java(
188+
"""
189+
import org.junit.jupiter.api.Assertions;
190+
191+
import java.util.HashMap;
192+
import java.util.Map;
193+
194+
public class Test {
195+
void test() {
196+
Map<String, Double> map = new HashMap<>();
197+
map.put("k", 0.5);
198+
double d = 0.5;
199+
int i = 5;
200+
Integer iBoxed = 5;
201+
Double a = 0.5;
202+
Double b = a;
203+
Assertions.assertTrue(map.get("k") == d);
204+
Assertions.assertTrue(i == iBoxed);
205+
Assertions.assertTrue(a == b);
206+
}
207+
}
208+
""",
209+
"""
210+
import org.junit.jupiter.api.Assertions;
211+
212+
import java.util.HashMap;
213+
import java.util.Map;
214+
215+
public class Test {
216+
void test() {
217+
Map<String, Double> map = new HashMap<>();
218+
map.put("k", 0.5);
219+
double d = 0.5;
220+
int i = 5;
221+
Integer iBoxed = 5;
222+
Double a = 0.5;
223+
Double b = a;
224+
Assertions.assertEquals(map.get("k"), d);
225+
Assertions.assertEquals(i, iBoxed);
226+
Assertions.assertTrue(a == b);
227+
}
228+
}
229+
"""
230+
)
231+
);
232+
}
180233
}

‎src/test/java/org/openrewrite/java/testing/junit5/UseAssertSameTest.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,4 +255,50 @@ public void test(Object obj) {
255255
)
256256
);
257257
}
258+
259+
@Test
260+
void onlyConvertsReferenceComparisons() {
261+
// Wrappers are reference types so == is reference equality (assertSame).
262+
// With a primitive operand Java unboxes, so defer to AssertTrueComparisonToAssertEquals.
263+
//language=java
264+
rewriteRun(
265+
java(
266+
"""
267+
import org.junit.jupiter.api.Test;
268+
269+
import static org.junit.jupiter.api.Assertions.assertTrue;
270+
271+
class MyTest {
272+
273+
@Test
274+
public void test() {
275+
int primitive = 5;
276+
Integer a = 5;
277+
Integer b = a;
278+
assertTrue(primitive == a);
279+
assertTrue(a == b);
280+
}
281+
}
282+
""",
283+
"""
284+
import org.junit.jupiter.api.Test;
285+
286+
import static org.junit.jupiter.api.Assertions.assertSame;
287+
import static org.junit.jupiter.api.Assertions.assertTrue;
288+
289+
class MyTest {
290+
291+
@Test
292+
public void test() {
293+
int primitive = 5;
294+
Integer a = 5;
295+
Integer b = a;
296+
assertTrue(primitive == a);
297+
assertSame(a, b);
298+
}
299+
}
300+
"""
301+
)
302+
);
303+
}
258304
}

0 commit comments

Comments
 (0)