Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate variable name in if statement with two different instanceof #175

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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<Integer> {

private final InstanceOfPatternReplacements replacements;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public void defaults(RecipeSpec spec) {
@SuppressWarnings({"ImplicitArrayToString", "PatternVariableCanBeUsed", "UnnecessaryLocalVariable"})
@Nested
class If {

@Test
void ifConditionWithoutPattern() {
rewriteRun(
Expand Down Expand Up @@ -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"})
Expand Down Expand Up @@ -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<Object> comparator() {
return (a, b) ->
a instanceof String && b instanceof String ? ((String) a).compareTo((String) b) : 0;
}
}
""",
"""
import java.util.Comparator;
public class A {
Comparator<Object> comparator() {
return (a, b) ->
a instanceof String s && b instanceof String s1 ? s.compareTo(s1) : 0;
}
}
"""
)
);
}
}

@Nested
Expand Down