Skip to content

Commit c507f59

Browse files
authored
Add parentheses in CompareEnumsWithEqualityOperator as needed (#658)
* Add parentheses in `CompareEnumsWithEqualityOperator` as needed - Fixes #657 * Add test for #3 * Reduce code down to bare minimum to pass tests
1 parent f817979 commit c507f59

File tree

2 files changed

+76
-4
lines changed

2 files changed

+76
-4
lines changed

src/main/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperator.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,25 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
5858
if (enumEquals.matches(m) && m.getSelect() != null) {
5959
Cursor parent = getCursor().dropParentUntil(is -> is instanceof J.Unary || is instanceof J.Block || is instanceof J.Binary || is instanceof J.Ternary || is instanceof J.Lambda);
6060
boolean isNot = parent.getValue() instanceof J.Unary && ((J.Unary) parent.getValue()).getOperator() == J.Unary.Type.Not;
61+
boolean needsParentheses = false;
6162
if (isNot) {
6263
parent.putMessage("REMOVE_UNARY_NOT", parent.getValue());
64+
} else if (parent.getValue() instanceof J.Binary) {
65+
// Check if the method invocation is part of a binary expression with equality/inequality operators
66+
// In such cases, we need parentheses to avoid chained equality operators
67+
J.Binary binary = parent.getValue();
68+
J.Binary.Type op = binary.getOperator();
69+
// Only add parentheses when the parent binary uses equality/inequality operators
70+
needsParentheses = op == J.Binary.Type.Equal || op == J.Binary.Type.NotEqual;
6371
}
72+
6473
String code = "#{any()} " + (isNot ? "!=" : "==") + " #{any()}";
65-
return autoFormat(JavaTemplate
66-
.builder(code)
67-
.build()
68-
.apply(updateCursor(m), m.getCoordinates().replace(), m.getSelect(), m.getArguments().get(0)), ctx);
74+
return JavaTemplate.apply(
75+
needsParentheses ? "(" + code + ")" : code,
76+
updateCursor(m),
77+
m.getCoordinates().replace(),
78+
m.getSelect(),
79+
m.getArguments().get(0));
6980
}
7081
return m;
7182
}

src/test/java/org/openrewrite/staticanalysis/CompareEnumsWithEqualityOperatorTest.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,4 +400,65 @@ void method(A value1, A value2, A value3, A value4) {
400400
)
401401
);
402402
}
403+
404+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/657")
405+
@Test
406+
void parenthesesRequiredInBinaryExpression() {
407+
rewriteRun(
408+
//language=java
409+
java(
410+
"""
411+
import java.time.DayOfWeek;
412+
class Test {
413+
void method() {
414+
boolean foo = true == DayOfWeek.MONDAY.equals(DayOfWeek.TUESDAY);
415+
}
416+
}
417+
""",
418+
"""
419+
import java.time.DayOfWeek;
420+
class Test {
421+
void method() {
422+
boolean foo = true == (DayOfWeek.MONDAY == DayOfWeek.TUESDAY);
423+
}
424+
}
425+
"""
426+
)
427+
);
428+
}
429+
430+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/3")
431+
@Test
432+
void negatedEnumComparisonInComplexBooleanExpression() {
433+
rewriteRun(
434+
enumA,
435+
//language=java
436+
java(
437+
"""
438+
import a.A;
439+
class Test {
440+
boolean hasField(String field) {
441+
return true;
442+
}
443+
void method(A field, Object entry) {
444+
if ((!A.FOO.equals(field) && !A.BAR.equals(field)) || !hasField("test")) {
445+
}
446+
}
447+
}
448+
""",
449+
"""
450+
import a.A;
451+
class Test {
452+
boolean hasField(String field) {
453+
return true;
454+
}
455+
void method(A field, Object entry) {
456+
if ((A.FOO != field && A.BAR != field) || !hasField("test")) {
457+
}
458+
}
459+
}
460+
"""
461+
)
462+
);
463+
}
403464
}

0 commit comments

Comments
 (0)