Skip to content

Commit 9fa3a92

Browse files
authored
SimplifyBooleanExpression: don't simplify nullable boolean comparisons in Kotlin (#7103)
`shouldSimplifyEqualsOn` now checks whether the source file is Java before allowing `x == true` / `x != false` simplifications. For non-Java languages (Kotlin), simplification is restricted to primitive boolean types to avoid changing semantics of nullable Boolean comparisons like `Boolean? == true`. Fixes openrewrite/rewrite-static-analysis#303
1 parent 142c67a commit 9fa3a92

2 files changed

Lines changed: 124 additions & 8 deletions

File tree

rewrite-java/src/main/java/org/openrewrite/java/cleanup/SimplifyBooleanExpressionVisitor.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -445,19 +445,24 @@ private boolean canSimplifyDoubleNegation(Expression innerExpression) {
445445
}
446446

447447
/**
448-
* Override this method to disable simplification of equals expressions,
449-
* specifically for Kotlin while that is not yet part of the OpenRewrite/rewrite.
448+
* Determines whether an equals comparison with a boolean literal can be simplified.
450449
* <p>
451-
* Comparing Kotlin nullable type `?` with tree/false can not be simplified,
452-
* e.g. `X?.fun() == true` is not equivalent to `X?.fun()`
450+
* In Java, {@code x == true} can always be simplified to {@code x}.
451+
* In Kotlin and other languages, nullable types like {@code Boolean?} compared
452+
* with {@code == true} have different semantics than using the value directly,
453+
* e.g. {@code nullableBoolean == true} evaluates to {@code false} when null,
454+
* whereas using {@code nullableBoolean} directly would be a type error.
453455
* <p>
454-
* Subclasses will want to check if the `org.openrewrite.kotlin.marker.IsNullSafe`
455-
* marker is present.
456+
* For non-Java languages, simplification is only allowed when the expression
457+
* type is primitive boolean (non-nullable).
456458
*
457459
* @param j the expression to simplify
458-
* @return true by default, unless overridden
460+
* @return true if the equals comparison can be safely simplified
459461
*/
460462
protected boolean shouldSimplifyEqualsOn(J j) {
461-
return true;
463+
if (getCursor().firstEnclosing(SourceFile.class) instanceof J.CompilationUnit) {
464+
return true;
465+
}
466+
return j instanceof Expression && ((Expression) j).getType() == JavaType.Primitive.Boolean;
462467
}
463468
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* Copyright 2026 the original author or authors.
3+
* <p>
4+
* Licensed under the Apache License, Version 2.0 (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://www.apache.org/licenses/LICENSE-2.0
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.kotlin.cleanup;
17+
18+
import org.junit.jupiter.api.Test;
19+
import org.openrewrite.Issue;
20+
import org.openrewrite.java.cleanup.SimplifyBooleanExpressionVisitor;
21+
import org.openrewrite.test.RecipeSpec;
22+
import org.openrewrite.test.RewriteTest;
23+
24+
import static org.openrewrite.kotlin.Assertions.kotlin;
25+
import static org.openrewrite.test.RewriteTest.toRecipe;
26+
27+
class SimplifyBooleanExpressionVisitorTest implements RewriteTest {
28+
29+
@Override
30+
public void defaults(RecipeSpec spec) {
31+
spec.recipe(toRecipe(() -> new SimplifyBooleanExpressionVisitor()));
32+
}
33+
34+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/303")
35+
@Test
36+
void nullableReceiverEqualsTrue() {
37+
rewriteRun(
38+
kotlin(
39+
"""
40+
fun Boolean?.toLegacyFlag() = if (this == true) "1" else "0"
41+
"""
42+
)
43+
);
44+
}
45+
46+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/303")
47+
@Test
48+
void nullableVariableEqualsTrue() {
49+
rewriteRun(
50+
kotlin(
51+
"""
52+
data class Todo(val completed: Boolean?)
53+
fun main() {
54+
val todo = Todo(null)
55+
val isCompleted: Boolean = todo.completed == true
56+
}
57+
"""
58+
)
59+
);
60+
}
61+
62+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/303")
63+
@Test
64+
void nullableVariableEqualsFalse() {
65+
rewriteRun(
66+
kotlin(
67+
"""
68+
fun check(b: Boolean?) {
69+
if (b == false) println("false or null")
70+
}
71+
"""
72+
)
73+
);
74+
}
75+
76+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/303")
77+
@Test
78+
void nullableVariableNotEqualTrue() {
79+
rewriteRun(
80+
kotlin(
81+
"""
82+
fun check(b: Boolean?) {
83+
if (b != true) println("not true")
84+
}
85+
"""
86+
)
87+
);
88+
}
89+
90+
@Test
91+
void simplifyBooleanLiteralOperations() {
92+
rewriteRun(
93+
kotlin(
94+
"""
95+
fun check(b: Boolean) {
96+
val a = !false
97+
val c = b || true
98+
val d = b && false
99+
}
100+
""",
101+
"""
102+
fun check(b: Boolean) {
103+
val a = true
104+
val c = true
105+
val d = false
106+
}
107+
"""
108+
)
109+
);
110+
}
111+
}

0 commit comments

Comments
 (0)