diff --git a/src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java b/src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java index eb528ed5..ce994926 100644 --- a/src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java @@ -19,7 +19,6 @@ import lombok.EqualsAndHashCode; import lombok.Value; import org.openrewrite.*; -import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.VariableNameUtils; @@ -265,7 +264,7 @@ private String patternVariableName(J.InstanceOf instanceOf, Cursor cursor) { } else { strategy = VariableNameStrategy.short_(); } - String baseName = variableBaseName((TypeTree) instanceOf.getClazz(), strategy); + String baseName = strategy.variableName(((TypeTree) instanceOf.getClazz()).getType()); return VariableNameUtils.generateVariableName(baseName, cursor, INCREMENT_NUMBER); } @@ -295,10 +294,6 @@ public J processVariableDeclarations(J.VariableDeclarations multiVariable) { } } - private static String variableBaseName(TypeTree typeTree, VariableNameStrategy nameStrategy) { - return nameStrategy.variableName(typeTree.getType()); - } - private static class UseInstanceOfPatternMatching extends JavaVisitor { private final InstanceOfPatternReplacements replacements; @@ -312,6 +307,26 @@ static J refactor(@Nullable J tree, InstanceOfPatternReplacements replacements, return new UseInstanceOfPatternMatching(replacements).visit(tree, 0, cursor); } + @Override + public J visitBinary(J.Binary original, Integer integer) { + Expression newLeft = (Expression) super.visitNonNull(original.getLeft(), integer); + if (newLeft != original.getLeft()) { + // The left side changed, so the right side should see any introduced variable names + J.Binary replacement = original.withLeft(newLeft); + Cursor widenedCursor = updateCursor(replacement); + + Expression newRight; + if (original.getRight() instanceof J.InstanceOf) { + newRight = replacements.processInstanceOf((J.InstanceOf) original.getRight(), widenedCursor); + } else { + newRight = (Expression) super.visitNonNull(original.getRight(), integer, widenedCursor); + } + return replacement.withRight(newRight); + } + // The left side didn't change, so the right side doesn't need to see any introduced variable names + return super.visitBinary(original, integer); + } + @Override public J.InstanceOf visitInstanceOf(J.InstanceOf instanceOf, Integer executionContext) { instanceOf = (J.InstanceOf) super.visitInstanceOf(instanceOf, executionContext); diff --git a/src/test/java/org/openrewrite/staticanalysis/InstanceOfPatternMatchTest.java b/src/test/java/org/openrewrite/staticanalysis/InstanceOfPatternMatchTest.java index 9ea52485..bcc35611 100644 --- a/src/test/java/org/openrewrite/staticanalysis/InstanceOfPatternMatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/InstanceOfPatternMatchTest.java @@ -37,7 +37,6 @@ public void defaults(RecipeSpec spec) { @SuppressWarnings({"ImplicitArrayToString", "PatternVariableCanBeUsed", "UnnecessaryLocalVariable"}) @Nested class If { - @Test void ifConditionWithoutPattern() { rewriteRun( @@ -468,6 +467,71 @@ void test(Object o) { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/174") + void ifTwoDifferentInstanceOf() { + rewriteRun( + version( + //language=java + java( + """ + class A { + int combinedLength(Object o, Object o2) { + if (o instanceof String && o2 instanceof String) { + return ((String) o).length() + ((String) o2).length(); + } + return -1; + } + } + """, + """ + class A { + int combinedLength(Object o, Object o2) { + if (o instanceof String string && o2 instanceof String string1) { + return string.length() + string1.length(); + } + return -1; + } + } + """ + ), 17 + ) + ); + } + + @Disabled("Not handled correctly yet") + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/174") + void ifTwoDifferentInstanceOfWithParentheses() { + rewriteRun( + version( + //language=java + java( + """ + class A { + int combinedLength(Object o, Object o2) { + if (o instanceof String && (o2 instanceof String)) { + return ((String) o).length() + ((String) o2).length(); + } + return -1; + } + } + """, + """ + class A { + int combinedLength(Object o, Object o2) { + if (o instanceof String string && (o2 instanceof String string1)) { + return string.length() + string1.length(); + } + return -1; + } + } + """ + ), 17 + ) + ); + } } @SuppressWarnings({"CastCanBeRemovedNarrowingVariableType", "ClassInitializerMayBeStatic"}) @@ -570,6 +634,34 @@ String test(Object o) { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/pull/265") + void multipleCastsInDifferentOperands() { + rewriteRun( + //language=java + java( + """ + import java.util.Comparator; + public class A { + Comparator comparator() { + return (a, b) -> + a instanceof String && b instanceof String ? ((String) a).compareTo((String) b) : 0; + } + } + """, + """ + import java.util.Comparator; + public class A { + Comparator comparator() { + return (a, b) -> + a instanceof String s && b instanceof String s1 ? s.compareTo(s1) : 0; + } + } + """ + ) + ); + } } @Nested