Skip to content

Fix InstanceOfPatternMatch duplicate pattern variable names with flow scoping #855

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.

Sign up for GitHub

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

Merged
timtebeek merged 3 commits into from
Apr 12, 2026
Merged

Fix InstanceOfPatternMatch duplicate pattern variable names with flow scoping #855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
20d2285
Fix InstanceOfPatternMatch duplicate pattern variable names with flow…
timtebeek Apr 11, 2026
591a3ef
Use cursor message instead of visitor field for introduced pattern names
timtebeek Apr 11, 2026
73cee8a
Add ROOT_VALUE guard and extract trackIntroducedName helper
timtebeek Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions src/main/java/org/openrewrite/staticanalysis/InstanceOfPatternMatch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
J result = super.postVisit(tree, ctx);
InstanceOfPatternReplacements original = getCursor().getMessage("flowTypeScope");
if (original != null && !original.isEmpty()) {
return UseInstanceOfPatternMatching.refactor(result, original, getCursor().getParentOrThrow());
Cursor methodCursor = getCursor().dropParentUntil(
v -> v instanceof J.MethodDeclaration || v instanceof J.ClassDeclaration || v == Cursor.ROOT_VALUE);
Set<String> introducedNames = methodCursor.computeMessageIfAbsent(
"introducedPatternVarNames", k -> new HashSet<>());
return UseInstanceOfPatternMatching.refactor(result, original, getCursor().getParentOrThrow(), introducedNames);
}
return result;
}
Expand Down Expand Up @@ -476,9 +480,10 @@ private boolean isUnboundedWildcardParameterized(TypeTree typeTree) {
private static class UseInstanceOfPatternMatching extends JavaVisitor<Integer> {

private final InstanceOfPatternReplacements replacements;
private final Set<String> introducedNames;

static @Nullable J refactor(@Nullable J tree, InstanceOfPatternReplacements replacements, Cursor cursor) {
return new UseInstanceOfPatternMatching(replacements).visit(tree, 0, cursor);
static @Nullable J refactor(@Nullable J tree, InstanceOfPatternReplacements replacements, Cursor cursor, Set<String> introducedNames) {
return new UseInstanceOfPatternMatching(replacements, introducedNames).visit(tree, 0, cursor);
}

@Override
Expand All @@ -489,7 +494,7 @@ public J visitBinary(J.Binary binary, Integer p) {
Cursor widenedCursor = updateCursor(b);

// Collect names from the left side if it's an instanceof with a pattern
Set<String> usedNames = new HashSet<>();
Set<String> usedNames = new HashSet<>(introducedNames);
if (b.getLeft() instanceof J.InstanceOf) {
J.InstanceOf leftInstanceOf = (J.InstanceOf) b.getLeft();
if (leftInstanceOf.getPattern() != null) {
Expand All @@ -499,11 +504,15 @@ public J visitBinary(J.Binary binary, Integer p) {

Expression newRight;
if (binary.getRight() instanceof J.InstanceOf) {
newRight = replacements.processInstanceOf((J.InstanceOf) binary.getRight(), widenedCursor, usedNames);
J.InstanceOf rightResult = replacements.processInstanceOf((J.InstanceOf) binary.getRight(), widenedCursor, usedNames);
trackIntroducedName((J.InstanceOf) binary.getRight(), rightResult);
newRight = rightResult;
} else if (binary.getRight() instanceof J.Parentheses &&
((J.Parentheses<?>) binary.getRight()).getTree() instanceof J.InstanceOf) {
@SuppressWarnings("unchecked") J.Parentheses<J.InstanceOf> originalRight = (J.Parentheses<J.InstanceOf>) binary.getRight();
newRight = originalRight.withTree(replacements.processInstanceOf(originalRight.getTree(), widenedCursor, usedNames));
J.InstanceOf rightResult = replacements.processInstanceOf(originalRight.getTree(), widenedCursor, usedNames);
trackIntroducedName(originalRight.getTree(), rightResult);
newRight = originalRight.withTree(rightResult);
} else {
newRight = (Expression) visitNonNull(binary.getRight(), p, widenedCursor);
}
Expand All @@ -516,7 +525,15 @@ public J visitBinary(J.Binary binary, Integer p) {
@Override
public J.InstanceOf visitInstanceOf(J.InstanceOf instanceOf, Integer p) {
instanceOf = (J.InstanceOf) super.visitInstanceOf(instanceOf, p);
return replacements.processInstanceOf(instanceOf, getCursor(), new HashSet<>());
J.InstanceOf result = replacements.processInstanceOf(instanceOf, getCursor(), new HashSet<>(introducedNames));
trackIntroducedName(instanceOf, result);
return result;
}

private void trackIntroducedName(J.InstanceOf original, J.InstanceOf result) {
if (result.getPattern() != null && original.getPattern() == null) {
introducedNames.add(((J.Identifier) result.getPattern()).getSimpleName());
}
}

@Override
Expand Down
44 changes: 44 additions & 0 deletions src/test/java/org/openrewrite/staticanalysis/InstanceOfPatternMatchTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1989,4 +1989,48 @@ void foo(Object object) {
)
);
}

@Issue("https://github.com/moderneinc/customer-requests/issues/2202")
@Test
void flowScopedPatternVariableConflictWithElseReturn() {
rewriteRun(
//language=java
java(
"""
class A {
void test(Object objNotOnOrAfter, Object objNotBefore) {
String strNotOnOrAfter = "";
if (objNotOnOrAfter != null && objNotOnOrAfter instanceof String) {
strNotOnOrAfter = (String) objNotOnOrAfter;
} else {
return;
}

String strNotBefore = "";
if (objNotBefore != null && objNotBefore instanceof String) {
strNotBefore = (String) objNotBefore;
}
}
}
""",
"""
class A {
void test(Object objNotOnOrAfter, Object objNotBefore) {
String strNotOnOrAfter = "";
if (objNotOnOrAfter != null && objNotOnOrAfter instanceof String string) {
strNotOnOrAfter = string;
} else {
return;
}

String strNotBefore = "";
if (objNotBefore != null && objNotBefore instanceof String string1) {
strNotBefore = string1;
}
}
}
"""
)
);
}
}
Loading