From 6fa1b103cc4487f88e58245b59e1fb27b31de065 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 13:01:23 +0200 Subject: [PATCH 01/11] Basic impl of SimplifyBooleanExpressionWithDeMorgan --- ...SimplifyBooleanExpressionWithDeMorgan.java | 131 ++++++++++ ...lifyBooleanExpressionWithDeMorganTest.java | 239 ++++++++++++++++++ 2 files changed, 370 insertions(+) create mode 100644 src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java new file mode 100644 index 0000000000..33ac501434 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -0,0 +1,131 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JLeftPadded; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Space; +import org.openrewrite.marker.Markers; + +import java.time.Duration; +import java.util.Collections; +import java.util.Set; + +public class SimplifyBooleanExpressionWithDeMorgan extends Recipe { + + @Override + public String getDisplayName() { + return "Simplify boolean expressions using De Morgan's laws"; + } + + @Override + public String getDescription() { + return "Applies De Morgan's laws to simplify boolean expressions with negation. " + + "Transforms `!(a && b)` to `!a || !b` and `!(a || b)` to `!a && !b`."; + } + + @Override + public Set getTags() { + return Collections.singleton("RSPEC-1125"); + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(2); + } + + @Override + public TreeVisitor getVisitor() { + return new JavaVisitor() { + + @Override + public J visitUnary(J.Unary unary, ExecutionContext ctx) { + if (unary.getOperator() == J.Unary.Type.Not && unary.getExpression() instanceof J.Parentheses) { + J.Parentheses parentheses = (J.Parentheses) unary.getExpression(); + if (parentheses.getTree() instanceof J.Binary) { + J.Binary binary = (J.Binary) parentheses.getTree(); + + // Apply De Morgan's laws + if (binary.getOperator() == J.Binary.Type.And) { + // !(a && b) -> !a || !b + J.Unary leftNegated = new J.Unary( + Tree.randomId(), + binary.getLeft().getPrefix(), + Markers.EMPTY, + new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), + binary.getLeft().withPrefix(Space.EMPTY), + JavaType.Primitive.Boolean + ); + J.Unary rightNegated = new J.Unary( + Tree.randomId(), + Space.EMPTY, + Markers.EMPTY, + new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), + binary.getRight().withPrefix(Space.EMPTY), + JavaType.Primitive.Boolean + ); + + return new J.Binary( + binary.getId(), + unary.getPrefix(), + unary.getMarkers(), + leftNegated, + JLeftPadded.build(J.Binary.Type.Or).withBefore(Space.SINGLE_SPACE), + rightNegated.withPrefix(Space.SINGLE_SPACE), + JavaType.Primitive.Boolean + ); + } else if (binary.getOperator() == J.Binary.Type.Or) { + // !(a || b) -> !a && !b + J.Unary leftNegated = new J.Unary( + Tree.randomId(), + binary.getLeft().getPrefix(), + Markers.EMPTY, + new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), + binary.getLeft().withPrefix(Space.EMPTY), + JavaType.Primitive.Boolean + ); + J.Unary rightNegated = new J.Unary( + Tree.randomId(), + Space.EMPTY, + Markers.EMPTY, + new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), + binary.getRight().withPrefix(Space.EMPTY), + JavaType.Primitive.Boolean + ); + + return new J.Binary( + binary.getId(), + unary.getPrefix(), + unary.getMarkers(), + leftNegated, + JLeftPadded.build(J.Binary.Type.And).withBefore(Space.SINGLE_SPACE), + rightNegated.withPrefix(Space.SINGLE_SPACE), + JavaType.Primitive.Boolean + ); + } + } + } + return super.visitUnary(unary, ctx); + } + }; + } +} \ No newline at end of file diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java new file mode 100644 index 0000000000..b2ac63b831 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java @@ -0,0 +1,239 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class SimplifyBooleanExpressionWithDeMorganTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new SimplifyBooleanExpressionWithDeMorgan()); + } + + @DocumentExample + @Test + void transformNegatedAndToOr() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean a, boolean b) { + if (!(a && b)) { + System.out.println("Not both"); + } + } + } + """, + """ + class Test { + void test(boolean a, boolean b) { + if (!a || !b) { + System.out.println("Not both"); + } + } + } + """ + ) + ); + } + + @Test + void transformNegatedOrToAnd() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean a, boolean b) { + if (!(a || b)) { + System.out.println("Neither"); + } + } + } + """, + """ + class Test { + void test(boolean a, boolean b) { + if (!a && !b) { + System.out.println("Neither"); + } + } + } + """ + ) + ); + } + + @Test + void transformMethodCallExpressions() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(String s1, String s2) { + if (!(s1.isEmpty() && s2.isEmpty())) { + System.out.println("At least one not empty"); + } + } + } + """, + """ + class Test { + void test(String s1, String s2) { + if (!s1.isEmpty() || !s2.isEmpty()) { + System.out.println("At least one not empty"); + } + } + } + """ + ) + ); + } + + @Test + void transformVariableExpressions() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean isValid, boolean isEnabled) { + boolean result = !(isValid || isEnabled); + } + } + """, + """ + class Test { + void test(boolean isValid, boolean isEnabled) { + boolean result = !isValid && !isEnabled; + } + } + """ + ) + ); + } + + @Test + void noChangeWhenNoParentheses() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean a, boolean b) { + if (!a && b) { + System.out.println("Already simplified"); + } + } + } + """ + ) + ); + } + + @Test + void noChangeWhenNotNegatedBinaryExpression() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean a, boolean b) { + if (a && b) { + System.out.println("No negation"); + } + } + } + """ + ) + ); + } + + @Test + void noChangeWhenNegatingComparison() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(int x, int y) { + if (!(x > y)) { + System.out.println("Not greater"); + } + } + } + """ + ) + ); + } + + @Test + void transformParenthesizedOrExpression() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean a, boolean b, boolean c) { + if (!((a || b) || c)) { + System.out.println("None are true"); + } + } + } + """, + """ + class Test { + void test(boolean a, boolean b, boolean c) { + if (!a && !b && !c) { + System.out.println("None are true"); + } + } + } + """ + ) + ); + } + + @Test + void transformParenthesizedAndExpression() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean w, boolean x, boolean y) { + boolean result = !((w && x) && y); + } + } + """, + """ + class Test { + void test(boolean w, boolean x, boolean y) { + boolean result = !w || !x || !y; + } + } + """ + ) + ); + } +} From 6339422cb469e1d0fc3bdd822aa137c990b1c8d6 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 13:54:57 +0200 Subject: [PATCH 02/11] More thorough tests --- ...lifyBooleanExpressionWithDeMorganTest.java | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java index b2ac63b831..62c7133dc5 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java @@ -110,29 +110,6 @@ void test(String s1, String s2) { ); } - @Test - void transformVariableExpressions() { - rewriteRun( - //language=java - java( - """ - class Test { - void test(boolean isValid, boolean isEnabled) { - boolean result = !(isValid || isEnabled); - } - } - """, - """ - class Test { - void test(boolean isValid, boolean isEnabled) { - boolean result = !isValid && !isEnabled; - } - } - """ - ) - ); - } - @Test void noChangeWhenNoParentheses() { rewriteRun( @@ -159,7 +136,7 @@ void noChangeWhenNotNegatedBinaryExpression() { """ class Test { void test(boolean a, boolean b) { - if (a && b) { + if ((a && b)) { System.out.println("No negation"); } } @@ -188,14 +165,14 @@ void test(int x, int y) { } @Test - void transformParenthesizedOrExpression() { + void triple() { rewriteRun( //language=java java( """ class Test { void test(boolean a, boolean b, boolean c) { - if (!((a || b) || c)) { + if (!(a || !b || c)) { System.out.println("None are true"); } } @@ -204,7 +181,7 @@ void test(boolean a, boolean b, boolean c) { """ class Test { void test(boolean a, boolean b, boolean c) { - if (!a && !b && !c) { + if (!a && b && !c) { System.out.println("None are true"); } } @@ -215,21 +192,44 @@ void test(boolean a, boolean b, boolean c) { } @Test - void transformParenthesizedAndExpression() { + void quadruple() { + rewriteRun( + //language=java + java( + """ + class Test { + static void test(boolean a, boolean b, boolean c, boolean d) { + return !(a && !b && c && !d); + } + } + """, + """ + class Test { + static void test(boolean a, boolean b, boolean c, boolean d) { + return !a || b || !c || d; + } + } + """ + ) + ); + } + + @Test + void nested() { rewriteRun( //language=java java( """ class Test { - void test(boolean w, boolean x, boolean y) { - boolean result = !((w && x) && y); + void test(boolean w, boolean x, boolean y, boolean z) { + boolean result = !((w && x) && !(z || y)); } } """, """ class Test { - void test(boolean w, boolean x, boolean y) { - boolean result = !w || !x || !y; + void test(boolean w, boolean x, boolean y, boolean z) { + boolean result = (!w || !x) || (z || y); } } """ From a0c39453c59f8f8e380e592163c7c883759266c5 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 14:11:54 +0200 Subject: [PATCH 03/11] negate() method --- ...SimplifyBooleanExpressionWithDeMorgan.java | 99 ++++++++----------- ...lifyBooleanExpressionWithDeMorganTest.java | 8 +- 2 files changed, 45 insertions(+), 62 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index 33ac501434..dd4ff8a4e7 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -20,6 +20,7 @@ import org.openrewrite.Tree; import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JLeftPadded; import org.openrewrite.java.tree.JavaType; @@ -30,6 +31,8 @@ import java.util.Collections; import java.util.Set; +import static java.util.Objects.requireNonNull; + public class SimplifyBooleanExpressionWithDeMorgan extends Recipe { @Override @@ -57,75 +60,55 @@ public Duration getEstimatedEffortPerOccurrence() { public TreeVisitor getVisitor() { return new JavaVisitor() { + private Expression negate(Expression expression, Space prefix) { + if (expression instanceof J.Unary) { + J.Unary unaryExpr = (J.Unary) expression; + if (unaryExpr.getOperator() == J.Unary.Type.Not) { + return unaryExpr.getExpression().withPrefix(prefix); + } + } + return new J.Unary( + Tree.randomId(), + prefix, + Markers.EMPTY, + new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), + expression.withPrefix(Space.EMPTY), + JavaType.Primitive.Boolean + ); + } + @Override public J visitUnary(J.Unary unary, ExecutionContext ctx) { if (unary.getOperator() == J.Unary.Type.Not && unary.getExpression() instanceof J.Parentheses) { J.Parentheses parentheses = (J.Parentheses) unary.getExpression(); if (parentheses.getTree() instanceof J.Binary) { J.Binary binary = (J.Binary) parentheses.getTree(); - - // Apply De Morgan's laws + + J.Binary.Type newOperator = null; if (binary.getOperator() == J.Binary.Type.And) { - // !(a && b) -> !a || !b - J.Unary leftNegated = new J.Unary( - Tree.randomId(), - binary.getLeft().getPrefix(), - Markers.EMPTY, - new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), - binary.getLeft().withPrefix(Space.EMPTY), - JavaType.Primitive.Boolean - ); - J.Unary rightNegated = new J.Unary( - Tree.randomId(), - Space.EMPTY, - Markers.EMPTY, - new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), - binary.getRight().withPrefix(Space.EMPTY), - JavaType.Primitive.Boolean - ); - - return new J.Binary( - binary.getId(), - unary.getPrefix(), - unary.getMarkers(), - leftNegated, - JLeftPadded.build(J.Binary.Type.Or).withBefore(Space.SINGLE_SPACE), - rightNegated.withPrefix(Space.SINGLE_SPACE), - JavaType.Primitive.Boolean - ); + newOperator = J.Binary.Type.Or; } else if (binary.getOperator() == J.Binary.Type.Or) { - // !(a || b) -> !a && !b - J.Unary leftNegated = new J.Unary( - Tree.randomId(), - binary.getLeft().getPrefix(), - Markers.EMPTY, - new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), - binary.getLeft().withPrefix(Space.EMPTY), - JavaType.Primitive.Boolean - ); - J.Unary rightNegated = new J.Unary( - Tree.randomId(), - Space.EMPTY, - Markers.EMPTY, - new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), - binary.getRight().withPrefix(Space.EMPTY), - JavaType.Primitive.Boolean - ); - - return new J.Binary( - binary.getId(), - unary.getPrefix(), - unary.getMarkers(), - leftNegated, - JLeftPadded.build(J.Binary.Type.And).withBefore(Space.SINGLE_SPACE), - rightNegated.withPrefix(Space.SINGLE_SPACE), - JavaType.Primitive.Boolean - ); + newOperator = J.Binary.Type.And; + } else { + return binary; } + // TODO recurse to left and right + Expression leftNegated = negate(binary.getLeft(), binary.getLeft().getPrefix()); + Expression rightNegated = negate(binary.getRight(), Space.SINGLE_SPACE); + + return new J.Binary( + binary.getId(), + unary.getPrefix(), + unary.getMarkers(), + leftNegated, + binary.getPadding().getOperator().withElement(newOperator), + rightNegated, + JavaType.Primitive.Boolean + ); } } - return super.visitUnary(unary, ctx); + return requireNonNull(super.visitUnary(unary, ctx)); } }; } -} \ No newline at end of file +} diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java index 62c7133dc5..626472bca4 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java @@ -37,7 +37,7 @@ void transformNegatedAndToOr() { """ class Test { void test(boolean a, boolean b) { - if (!(a && b)) { + if (!(a && !b)) { System.out.println("Not both"); } } @@ -46,7 +46,7 @@ void test(boolean a, boolean b) { """ class Test { void test(boolean a, boolean b) { - if (!a || !b) { + if (!a || b) { System.out.println("Not both"); } } @@ -64,7 +64,7 @@ void transformNegatedOrToAnd() { """ class Test { void test(boolean a, boolean b) { - if (!(a || b)) { + if (!(!a || b)) { System.out.println("Neither"); } } @@ -73,7 +73,7 @@ void test(boolean a, boolean b) { """ class Test { void test(boolean a, boolean b) { - if (!a && !b) { + if (a && !b) { System.out.println("Neither"); } } From 73e96f4ac1aecd225c1d36f1e036a0c645fbb16e Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 14:14:37 +0200 Subject: [PATCH 04/11] Handling other binary operators better --- ...SimplifyBooleanExpressionWithDeMorgan.java | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index dd4ff8a4e7..d95428c3ea 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -89,22 +89,25 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { newOperator = J.Binary.Type.Or; } else if (binary.getOperator() == J.Binary.Type.Or) { newOperator = J.Binary.Type.And; - } else { - return binary; } + // TODO recurse to left and right Expression leftNegated = negate(binary.getLeft(), binary.getLeft().getPrefix()); Expression rightNegated = negate(binary.getRight(), Space.SINGLE_SPACE); - return new J.Binary( - binary.getId(), - unary.getPrefix(), - unary.getMarkers(), - leftNegated, - binary.getPadding().getOperator().withElement(newOperator), - rightNegated, - JavaType.Primitive.Boolean - ); + if (newOperator != null) { + return new J.Binary( + binary.getId(), + unary.getPrefix(), + unary.getMarkers(), + leftNegated, + binary.getPadding().getOperator().withElement(newOperator), + rightNegated, + JavaType.Primitive.Boolean + ); + } else { + return unary; // TODO recursion here as well + } } } return requireNonNull(super.visitUnary(unary, ctx)); From 2c010d872ce113b3654a41f8734252a77bc67569 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 14:20:31 +0200 Subject: [PATCH 05/11] Preserve prefix --- .../staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java | 2 +- .../SimplifyBooleanExpressionWithDeMorganTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index d95428c3ea..b44601c3b0 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -93,7 +93,7 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { // TODO recurse to left and right Expression leftNegated = negate(binary.getLeft(), binary.getLeft().getPrefix()); - Expression rightNegated = negate(binary.getRight(), Space.SINGLE_SPACE); + Expression rightNegated = negate(binary.getRight(), binary.getRight().getPrefix()); if (newOperator != null) { return new J.Binary( diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java index 626472bca4..456e393218 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java @@ -64,7 +64,7 @@ void transformNegatedOrToAnd() { """ class Test { void test(boolean a, boolean b) { - if (!(!a || b)) { + if (!(!a || /*Bee Bee Bee*/ b)) { System.out.println("Neither"); } } @@ -73,7 +73,7 @@ void test(boolean a, boolean b) { """ class Test { void test(boolean a, boolean b) { - if (a && !b) { + if (a && /*Bee Bee Bee*/ !b) { System.out.println("Neither"); } } From b2116fae25a20724c17e3cd73e687d641b3512ca Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 14:38:43 +0200 Subject: [PATCH 06/11] Recursing into left and right --- ...SimplifyBooleanExpressionWithDeMorgan.java | 77 +++++++++++++------ ...lifyBooleanExpressionWithDeMorganTest.java | 27 +++++++ 2 files changed, 82 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index b44601c3b0..7d17197d4c 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -15,6 +15,7 @@ */ package org.openrewrite.staticanalysis; +import org.jetbrains.annotations.NotNull; import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.Tree; @@ -60,29 +61,13 @@ public Duration getEstimatedEffortPerOccurrence() { public TreeVisitor getVisitor() { return new JavaVisitor() { - private Expression negate(Expression expression, Space prefix) { - if (expression instanceof J.Unary) { - J.Unary unaryExpr = (J.Unary) expression; - if (unaryExpr.getOperator() == J.Unary.Type.Not) { - return unaryExpr.getExpression().withPrefix(prefix); - } - } - return new J.Unary( - Tree.randomId(), - prefix, - Markers.EMPTY, - new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), - expression.withPrefix(Space.EMPTY), - JavaType.Primitive.Boolean - ); - } - @Override public J visitUnary(J.Unary unary, ExecutionContext ctx) { if (unary.getOperator() == J.Unary.Type.Not && unary.getExpression() instanceof J.Parentheses) { J.Parentheses parentheses = (J.Parentheses) unary.getExpression(); if (parentheses.getTree() instanceof J.Binary) { J.Binary binary = (J.Binary) parentheses.getTree(); + J.Parentheses parenthesesBinary = (J.Parentheses) unary.getExpression(); J.Binary.Type newOperator = null; if (binary.getOperator() == J.Binary.Type.And) { @@ -90,12 +75,13 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { } else if (binary.getOperator() == J.Binary.Type.Or) { newOperator = J.Binary.Type.And; } - - // TODO recurse to left and right - Expression leftNegated = negate(binary.getLeft(), binary.getLeft().getPrefix()); - Expression rightNegated = negate(binary.getRight(), binary.getRight().getPrefix()); + Expression leftAltered = requireNonNull((Expression) visit(binary.getLeft(), ctx)); + Expression rightAltered = requireNonNull((Expression) visit(binary.getRight(), ctx)); if (newOperator != null) { + Expression leftNegated = negate(leftAltered, binary.getLeft().getPrefix()); + Expression rightNegated = negate(rightAltered, binary.getRight().getPrefix()); + return new J.Binary( binary.getId(), unary.getPrefix(), @@ -106,12 +92,59 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { JavaType.Primitive.Boolean ); } else { - return unary; // TODO recursion here as well + J.Binary visitedBinary = binary.withLeft(leftAltered).withRight(rightAltered); + return unary.withExpression(parenthesesBinary.withTree(visitedBinary)); } } } return requireNonNull(super.visitUnary(unary, ctx)); } + + private Expression negate(Expression expression, Space prefix) { + if (expression instanceof J.Unary) { + J.Unary unaryExpr = (J.Unary) expression; + if (unaryExpr.getOperator() == J.Unary.Type.Not) { + return unaryExpr.getExpression().withPrefix(prefix); + } + } + return new J.Unary( + Tree.randomId(), + prefix, + Markers.EMPTY, + new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), + expression.withPrefix(Space.EMPTY), + JavaType.Primitive.Boolean + ); + } + + /** + * Check if all operators in a chained binary expression are the same. + * Handles nesting on both left and right sides. + */ + private boolean allOperatorsAreSame(J.Binary binary, J.Binary.Type expectedOperator) { + // Check current operator + if (binary.getOperator() != expectedOperator) { + return false; + } + + // Recursively check left side if it's a binary expression + if (binary.getLeft() instanceof J.Binary) { + J.Binary leftBinary = (J.Binary) binary.getLeft(); + if (!allOperatorsAreSame(leftBinary, expectedOperator)) { + return false; + } + } + + // Recursively check right side if it's a binary expression + if (binary.getRight() instanceof J.Binary) { + J.Binary rightBinary = (J.Binary) binary.getRight(); + if (!allOperatorsAreSame(rightBinary, expectedOperator)) { + return false; + } + } + + return true; + } }; } } diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java index 456e393218..8ba0e6bc29 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java @@ -236,4 +236,31 @@ void test(boolean w, boolean x, boolean y, boolean z) { ) ); } + + @Test + void deMorganWithinNonBoolean() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean a, boolean b, boolean c, boolean d) { + if ((!(a && !b)) == (!(!c || !d))) { + System.out.println("Complex boolean comparison"); + } + } + } + """, + """ + class Test { + void test(boolean a, boolean b, boolean c, boolean d) { + if ((!a || b) == (c && d)) { + System.out.println("Complex boolean comparison"); + } + } + } + """ + ) + ); + } } From 8c7c5b0d71f550d4057e89915eb42b122109f14d Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 15:24:53 +0200 Subject: [PATCH 07/11] Fix for the main logic --- ...SimplifyBooleanExpressionWithDeMorgan.java | 45 +++++++++---------- ...lifyBooleanExpressionWithDeMorganTest.java | 18 ++++++++ 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index 7d17197d4c..72f01aba79 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -21,6 +21,7 @@ import org.openrewrite.Tree; import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.ParenthesizeVisitor; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JLeftPadded; @@ -75,24 +76,23 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { } else if (binary.getOperator() == J.Binary.Type.Or) { newOperator = J.Binary.Type.And; } - Expression leftAltered = requireNonNull((Expression) visit(binary.getLeft(), ctx)); - Expression rightAltered = requireNonNull((Expression) visit(binary.getRight(), ctx)); + Expression left = binary.getLeft(); + Expression right = binary.getRight(); if (newOperator != null) { - Expression leftNegated = negate(leftAltered, binary.getLeft().getPrefix()); - Expression rightNegated = negate(rightAltered, binary.getRight().getPrefix()); - - return new J.Binary( - binary.getId(), - unary.getPrefix(), - unary.getMarkers(), - leftNegated, - binary.getPadding().getOperator().withElement(newOperator), - rightNegated, - JavaType.Primitive.Boolean - ); + left = negate(left); + left = (Expression) new ParenthesizeVisitor<>().visit(left, ctx); + right = negate(right); + right = (Expression) new ParenthesizeVisitor<>().visit(right, ctx); + } + + left = (Expression) this.visit(left, ctx); + right = (Expression) this.visit(right, ctx); + + if (newOperator != null) { + return binary.withLeft(left).withRight(right).withOperator(newOperator); } else { - J.Binary visitedBinary = binary.withLeft(leftAltered).withRight(rightAltered); + J.Binary visitedBinary = binary.withLeft(left).withRight(right); return unary.withExpression(parenthesesBinary.withTree(visitedBinary)); } } @@ -100,16 +100,16 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { return requireNonNull(super.visitUnary(unary, ctx)); } - private Expression negate(Expression expression, Space prefix) { + private Expression negate(Expression expression) { if (expression instanceof J.Unary) { J.Unary unaryExpr = (J.Unary) expression; if (unaryExpr.getOperator() == J.Unary.Type.Not) { - return unaryExpr.getExpression().withPrefix(prefix); + return unaryExpr.getExpression().withPrefix(expression.getPrefix()); } } return new J.Unary( Tree.randomId(), - prefix, + expression.getPrefix(), Markers.EMPTY, new JLeftPadded<>(Space.EMPTY, J.Unary.Type.Not, Markers.EMPTY), expression.withPrefix(Space.EMPTY), @@ -122,23 +122,18 @@ private Expression negate(Expression expression, Space prefix) { * Handles nesting on both left and right sides. */ private boolean allOperatorsAreSame(J.Binary binary, J.Binary.Type expectedOperator) { - // Check current operator if (binary.getOperator() != expectedOperator) { return false; } - // Recursively check left side if it's a binary expression if (binary.getLeft() instanceof J.Binary) { - J.Binary leftBinary = (J.Binary) binary.getLeft(); - if (!allOperatorsAreSame(leftBinary, expectedOperator)) { + if (!allOperatorsAreSame((J.Binary) binary.getLeft(), expectedOperator)) { return false; } } - // Recursively check right side if it's a binary expression if (binary.getRight() instanceof J.Binary) { - J.Binary rightBinary = (J.Binary) binary.getRight(); - if (!allOperatorsAreSame(rightBinary, expectedOperator)) { + if (!allOperatorsAreSame((J.Binary) binary.getRight(), expectedOperator)) { return false; } } diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java index 8ba0e6bc29..4b293359d9 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java @@ -237,6 +237,24 @@ void test(boolean w, boolean x, boolean y, boolean z) { ); } + @Test + void noChangeWhenMixedOperators() { + rewriteRun( + //language=java + java( + """ + class Test { + void test(boolean a, boolean b, boolean c) { + if (!(a && !b || c)) { + System.out.println("Mixed operators"); + } + } + } + """ + ) + ); + } + @Test void deMorganWithinNonBoolean() { rewriteRun( From 89ed0cad1ebcca415114f94e98dda344f156cb22 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 15:51:00 +0200 Subject: [PATCH 08/11] Merging comments --- ...SimplifyBooleanExpressionWithDeMorgan.java | 15 +++++++----- ...lifyBooleanExpressionWithDeMorganTest.java | 23 ++++++++++++------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index 72f01aba79..64671fb18e 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -22,15 +22,13 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.ParenthesizeVisitor; -import org.openrewrite.java.tree.Expression; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.JLeftPadded; -import org.openrewrite.java.tree.JavaType; -import org.openrewrite.java.tree.Space; +import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; import java.time.Duration; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Set; import static java.util.Objects.requireNonNull; @@ -90,7 +88,12 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { right = (Expression) this.visit(right, ctx); if (newOperator != null) { - return binary.withLeft(left).withRight(right).withOperator(newOperator); + Space prefix = unary.getPrefix(); + List comments = new ArrayList<>(prefix.getComments()); + comments.addAll(parenthesesBinary.getComments()); + comments.addAll(binary.getComments()); + prefix = prefix.withComments(comments); + return binary.withLeft(left).withRight(right).withOperator(newOperator).withPrefix(prefix); } else { J.Binary visitedBinary = binary.withLeft(left).withRight(right); return unary.withExpression(parenthesesBinary.withTree(visitedBinary)); diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java index 4b293359d9..a37c65cfe0 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java @@ -64,7 +64,7 @@ void transformNegatedOrToAnd() { """ class Test { void test(boolean a, boolean b) { - if (!(!a || /*Bee Bee Bee*/ b)) { + if /*0*/(/*1*/!/*2*/(/*3*/!a || /*Bee Bee Bee*/ b)) { System.out.println("Neither"); } } @@ -73,7 +73,7 @@ void test(boolean a, boolean b) { """ class Test { void test(boolean a, boolean b) { - if (a && /*Bee Bee Bee*/ !b) { + if /*0*/(/*1*//*2*//*3*/a && /*Bee Bee Bee*/ !b) { System.out.println("Neither"); } } @@ -217,6 +217,7 @@ static void test(boolean a, boolean b, boolean c, boolean d) { @Test void nested() { rewriteRun( + // NOTE: the parens around (z || y) in the output are not necessary, but they are not wrong. We might look into having them removed. //language=java java( """ @@ -229,7 +230,7 @@ void test(boolean w, boolean x, boolean y, boolean z) { """ class Test { void test(boolean w, boolean x, boolean y, boolean z) { - boolean result = (!w || !x) || (z || y); + boolean result = !w || !x || (z || y); } } """ @@ -238,16 +239,22 @@ void test(boolean w, boolean x, boolean y, boolean z) { } @Test - void noChangeWhenMixedOperators() { + void nested2() { rewriteRun( //language=java + // NOTE: the parens around (z || y) in the output are not necessary, but they are not wrong. We might look into having them removed. java( """ class Test { - void test(boolean a, boolean b, boolean c) { - if (!(a && !b || c)) { - System.out.println("Mixed operators"); - } + void test(boolean w, boolean x, boolean y, boolean z) { + boolean result = !((w || x) && !(z || y)); + } + } + """, + """ + class Test { + void test(boolean w, boolean x, boolean y, boolean z) { + boolean result = !w && !x || (z || y); } } """ From 02d40aef12f25dfa41b350da08cfea17f04ffd77 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 16:04:55 +0200 Subject: [PATCH 09/11] Handling mixed operators without allOperatorsAreSame() --- ...SimplifyBooleanExpressionWithDeMorgan.java | 27 ++----------------- ...lifyBooleanExpressionWithDeMorganTest.java | 23 ++++++++++++++++ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index 64671fb18e..a1caebd6e4 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -93,7 +93,8 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { comments.addAll(parenthesesBinary.getComments()); comments.addAll(binary.getComments()); prefix = prefix.withComments(comments); - return binary.withLeft(left).withRight(right).withOperator(newOperator).withPrefix(prefix); + binary = binary.withLeft(left).withRight(right).withOperator(newOperator).withPrefix(prefix); + return new ParenthesizeVisitor<>().visit(binary, ctx); } else { J.Binary visitedBinary = binary.withLeft(left).withRight(right); return unary.withExpression(parenthesesBinary.withTree(visitedBinary)); @@ -119,30 +120,6 @@ private Expression negate(Expression expression) { JavaType.Primitive.Boolean ); } - - /** - * Check if all operators in a chained binary expression are the same. - * Handles nesting on both left and right sides. - */ - private boolean allOperatorsAreSame(J.Binary binary, J.Binary.Type expectedOperator) { - if (binary.getOperator() != expectedOperator) { - return false; - } - - if (binary.getLeft() instanceof J.Binary) { - if (!allOperatorsAreSame((J.Binary) binary.getLeft(), expectedOperator)) { - return false; - } - } - - if (binary.getRight() instanceof J.Binary) { - if (!allOperatorsAreSame((J.Binary) binary.getRight(), expectedOperator)) { - return false; - } - } - - return true; - } }; } } diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java index a37c65cfe0..4d7dbabc72 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorganTest.java @@ -288,4 +288,27 @@ void test(boolean a, boolean b, boolean c, boolean d) { ) ); } + + + @Test + void mixedOperators() { + // As a human I probably wouldn't dare to change it probably, but it's not wrong, and it's not worse than the original, so be it. + rewriteRun( + //language=java + java( + """ + class Test { + boolean a, b, c; + boolean x = !(a && !b || c); + } + """, + """ + class Test { + boolean a, b, c; + boolean x = (!a || b) && !c; + } + """ + ) + ); + } } From e61e84caa49f49b2735fea868cdbe0304eeadd14 Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 16:15:38 +0200 Subject: [PATCH 10/11] Refactor to avoid else --- .../SimplifyBooleanExpressionWithDeMorgan.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index a1caebd6e4..a79280553d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -87,18 +87,17 @@ public J visitUnary(J.Unary unary, ExecutionContext ctx) { left = (Expression) this.visit(left, ctx); right = (Expression) this.visit(right, ctx); - if (newOperator != null) { - Space prefix = unary.getPrefix(); - List comments = new ArrayList<>(prefix.getComments()); - comments.addAll(parenthesesBinary.getComments()); - comments.addAll(binary.getComments()); - prefix = prefix.withComments(comments); - binary = binary.withLeft(left).withRight(right).withOperator(newOperator).withPrefix(prefix); - return new ParenthesizeVisitor<>().visit(binary, ctx); - } else { + if (newOperator == null) { J.Binary visitedBinary = binary.withLeft(left).withRight(right); return unary.withExpression(parenthesesBinary.withTree(visitedBinary)); } + Space prefix = unary.getPrefix(); + List comments = new ArrayList<>(prefix.getComments()); + comments.addAll(parenthesesBinary.getComments()); + comments.addAll(binary.getComments()); + prefix = prefix.withComments(comments); + binary = binary.withLeft(left).withRight(right).withOperator(newOperator).withPrefix(prefix); + return new ParenthesizeVisitor<>().visit(binary, ctx); } } return requireNonNull(super.visitUnary(unary, ctx)); From 6cba288e2ed159e1e9a30947016c1e7299f55c3c Mon Sep 17 00:00:00 2001 From: Greg Oledzki Date: Thu, 24 Jul 2025 16:16:32 +0200 Subject: [PATCH 11/11] Optimize imports --- .../staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java index a79280553d..901fcff3a4 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyBooleanExpressionWithDeMorgan.java @@ -15,7 +15,6 @@ */ package org.openrewrite.staticanalysis; -import org.jetbrains.annotations.NotNull; import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.Tree;