Skip to content

InstanceOfPatternMatch to avoid providing invalid variable definitions pointing to itself #849

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
greg-at-moderne merged 3 commits into from
Apr 8, 2026
Merged

InstanceOfPatternMatch to avoid providing invalid variable definitions pointing to itself #849

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
c64cbc5
Avoid invalid x=x variable declarations
greg-at-moderne Apr 8, 2026
e67e03f
A more involved case
greg-at-moderne Apr 8, 2026
ac02a53
Order of clauses shouldn't decide
greg-at-moderne Apr 8, 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
116 changes: 96 additions & 20 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 @@ -162,7 +162,7 @@ private static class InstanceOfPatternReplacements {
private final Map<J.InstanceOf, Set<J>> contexts = new HashMap<>();
private final Map<J.InstanceOf, Set<Cursor>> contextScopes = new HashMap<>();
private final Map<J.TypeCast, J.InstanceOf> replacements = new HashMap<>();
private final Map<J.InstanceOf, VariableAndTypeTree> variablesToDelete = new HashMap<>();
private final Map<J.InstanceOf, List<VariableAndTypeTree>> variablesToDelete = new HashMap<>();

public void registerInstanceOf(J.InstanceOf instanceOf, Set<J> contexts) {
Expression expression = instanceOf.getExpression();
Expand Down Expand Up @@ -201,9 +201,11 @@ public void registerTypeCast(J.TypeCast typeCast, Cursor cursor) {
if (isAcceptableTypeCast(typeCast.getType()) && isTheSameAsOtherTypeCasts(typeCast, instanceOf) && isAcceptableParentTypeCast(parent) &&
cursor.firstEnclosing(J.Try.Resource.class) == null) {
if (parent.getValue() instanceof J.VariableDeclarations.NamedVariable &&
!variablesToDelete.containsKey(instanceOf)) {
variablesToDelete.put(instanceOf, new VariableAndTypeTree(parent.getValue(),
requireNonNull(requireNonNull(parent.firstEnclosing(J.VariableDeclarations.class)).getTypeExpression())));
shouldDeleteVariableDeclaration(parent, instanceOf)) {
variablesToDelete
.computeIfAbsent(instanceOf, i -> new ArrayList<>())
.add(new VariableAndTypeTree(parent.getValue(),
requireNonNull(requireNonNull(parent.firstEnclosing(J.VariableDeclarations.class)).getTypeExpression())));
} else {
replacements.put(typeCast, instanceOf);
}
Expand Down Expand Up @@ -284,6 +286,21 @@ public boolean isEmpty() {
return replacements.isEmpty() && variablesToDelete.isEmpty();
}

private boolean shouldDeleteVariableDeclaration(Cursor parent, J.InstanceOf instanceOf) {
J.VariableDeclarations.NamedVariable namedVariable = parent.getValue();
TypeTree typeExpression = requireNonNull(requireNonNull(parent.firstEnclosing(J.VariableDeclarations.class)).getTypeExpression());
List<VariableAndTypeTree> existingVariables = variablesToDelete.get(instanceOf);
if (existingVariables == null || existingVariables.isEmpty()) {
return true;
}
VariableAndTypeTree primaryVariable = existingVariables.get(0);
JavaType currentType = typeExpression.getType();
JavaType primaryType = primaryVariable.getType().getType();
return namedVariable.getSimpleName().equals(primaryVariable.getVariable().getSimpleName()) &&
((currentType != null && primaryType != null && hasSameRawType(currentType, primaryType)) ||
SemanticallyEqual.areEqual(typeExpression, primaryVariable.getType()));
}

public J.InstanceOf processInstanceOf(J.InstanceOf instanceOf, Cursor cursor, Set<String> usedNames) {
if (!contextScopes.containsKey(instanceOf)) {
return instanceOf;
Expand Down Expand Up @@ -320,27 +337,22 @@ public J.InstanceOf processInstanceOf(J.InstanceOf instanceOf, Cursor cursor, Se
}

private TypeTree computeTypeTreeFromTypeCasts(J.InstanceOf instanceOf) {
TypeTree typeCastTypeTree = replacements
.entrySet()
.stream()
.filter(e -> e.getValue() == instanceOf)
.findFirst()
.map(e -> e.getKey().getClazz().getTree())
.orElse(null);
if (typeCastTypeTree == null) {
VariableAndTypeTree variable = variablesToDelete.get(instanceOf);
if (variable != null) {
typeCastTypeTree = variable.getType();
}
}
return typeCastTypeTree;
List<TypeTree> candidateTypes = Stream.concat(
replacements.entrySet().stream()
.filter(e -> e.getValue() == instanceOf)
.map(e -> e.getKey().getClazz().getTree()),
variablesToDelete.getOrDefault(instanceOf, emptyList()).stream()
.map(VariableAndTypeTree::getType)
)
.collect(toList());
return chooseTypeTree(candidateTypes, (TypeTree) instanceOf.getClazz());
}

private String patternVariableName(J.InstanceOf instanceOf, Cursor cursor, Set<String> usedNames) {
VariableNameStrategy strategy;
JavaType type = ((TypeTree) instanceOf.getClazz()).getType();
if (root instanceof J.If) {
VariableAndTypeTree variableData = variablesToDelete.get(instanceOf);
VariableAndTypeTree variableData = getPrimaryVariableToDelete(instanceOf);
if (variableData != null) {
// under the assumption that the code compiled previously we don't need to check for duplicates
return VariableNameStrategy.exact(variableData.getVariable().getSimpleName()).variableName(type);
Expand All @@ -366,6 +378,68 @@ private String patternVariableName(J.InstanceOf instanceOf, Cursor cursor, Set<S
return VariableNameUtils.generateVariableName(baseName, cursor, INCREMENT_NUMBER);
}

private @Nullable VariableAndTypeTree getPrimaryVariableToDelete(J.InstanceOf instanceOf) {
List<VariableAndTypeTree> variables = variablesToDelete.get(instanceOf);
if (variables == null || variables.isEmpty()) {
return null;
}
return variables.get(0);
}

private TypeTree chooseTypeTree(List<TypeTree> candidates, TypeTree originalTypeTree) {
if (candidates.isEmpty()) {
return originalTypeTree;
}

JavaType originalType = originalTypeTree.getType();
TypeTree firstCandidate = candidates.get(0);
JavaType firstType = firstCandidate.getType();
if (originalType == null || firstType == null) {
return originalTypeTree;
}

boolean allSameRawType = candidates.stream()
.map(TypeTree::getType)
.allMatch(type -> type != null &&
hasSameRawType(type, originalType) &&
hasSameRawType(type, firstType));
if (!allSameRawType) {
return originalTypeTree;
}

if (candidates.size() == 1) {
return candidates.get(0);
}

List<TypeTree> wildcardCandidates = candidates.stream()
.filter(this::isUnboundedWildcardParameterized)
.collect(toList());

if (!wildcardCandidates.isEmpty()) {
if (wildcardCandidates.stream().allMatch(candidate -> SemanticallyEqual.areEqual(candidate, wildcardCandidates.get(0)))) {
return wildcardCandidates.get(0);
}
return originalTypeTree;
}

if (candidates.stream().allMatch(candidate -> SemanticallyEqual.areEqual(candidate, firstCandidate))) {
return firstCandidate;
}

return originalTypeTree;
}

private boolean isUnboundedWildcardParameterized(TypeTree typeTree) {
JavaType type = typeTree.getType();
if (!(type instanceof JavaType.Parameterized)) {
return false;
}
return requireNonNull(((JavaType.Parameterized) type).getTypeParameters()).stream()
.allMatch(typeParameter -> typeParameter instanceof JavaType.GenericTypeVariable &&
((JavaType.GenericTypeVariable) typeParameter).getBounds().isEmpty() &&
"?".equals(((JavaType.GenericTypeVariable) typeParameter).getName()));
}

public @Nullable J processTypeCast(J.TypeCast typeCast, Cursor cursor) {
J.InstanceOf instanceOf = replacements.get(typeCast);
if (instanceOf != null && instanceOf.getPattern() != null) {
Expand All @@ -386,7 +460,9 @@ private String patternVariableName(J.InstanceOf instanceOf, Cursor cursor, Set<S
}

public @Nullable J processVariableDeclarations(J.VariableDeclarations multiVariable) {
return multiVariable.getVariables().stream().anyMatch(v -> variablesToDelete.values().stream().anyMatch(vd -> vd.getVariable() == v)) ? null : multiVariable;
return multiVariable.getVariables().stream().anyMatch(v -> variablesToDelete.values().stream()
.flatMap(Collection::stream)
.anyMatch(vd -> vd.getVariable() == v)) ? null : multiVariable;
}
}

Expand Down
199 changes: 199 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 @@ -521,6 +521,205 @@ void test(Object o) {
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/305")
@Test
void doesNotIntroduceSelfAssignmentInElseIfBranches() {
rewriteRun(
//language=java
java(
"""
class A {
String test(Object obj, String fieldName) {
if (obj instanceof Contact) {
if (fieldName.equals("FIRST_NAME")) {
Contact cont = (Contact) obj;
return getContactLink(cont, null, cont.getFirstName());
} else if (fieldName.equals("LAST_NAME")) {
Contact cont = (Contact) obj;
return getContactLink(cont, null, cont.getLastName());
} else if (fieldName.equals("FULL_NAME")) {
Contact cont = (Contact) obj;
return getContactLink(cont, null, cont.getDisplayName());
}
}
return "";
}

String getContactLink(Contact contact, Object unused, String value) {
return value;
}

static class Contact {
String getFirstName() {
return "";
}

String getLastName() {
return "";
}

String getDisplayName() {
return "";
}
}
}
""",
"""
class A {
String test(Object obj, String fieldName) {
if (obj instanceof Contact cont) {
if (fieldName.equals("FIRST_NAME")) {
return getContactLink(cont, null, cont.getFirstName());
} else if (fieldName.equals("LAST_NAME")) {
return getContactLink(cont, null, cont.getLastName());
} else if (fieldName.equals("FULL_NAME")) {
return getContactLink(cont, null, cont.getDisplayName());
}
}
return "";
}

String getContactLink(Contact contact, Object unused, String value) {
return value;
}

static class Contact {
String getFirstName() {
return "";
}

String getLastName() {
return "";
}

String getDisplayName() {
return "";
}
}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/305")
@Test
void doesNotIntroduceSelfAssignmentWhenAliasTypeSpellingsDiffer() {
rewriteRun(
//language=java
java(
"""
import java.util.List;

class A {
void test(Object obj, boolean b) {
if (obj instanceof List) {
if (b) {
List<?> list = (List<?>) obj;
System.out.println(list.size());
} else {
List list = (List) obj;
System.out.println(list.size());
}
}
}
}
""",
"""
import java.util.List;

class A {
void test(Object obj, boolean b) {
if (obj instanceof List<?> list) {
if (b) {
System.out.println(list.size());
} else {
System.out.println(list.size());
}
}
}
}
"""
)
);
}

@Test
void prefersStableTypeSpellingRegardlessOfBranchOrder() {
rewriteRun(
//language=java
java(
"""
import java.util.List;

class A {
void test(Object obj, boolean b) {
if (obj instanceof List) {
if (b) {
List list = (List) obj;
System.out.println(list.size());
} else {
List<?> list = (List<?>) obj;
System.out.println(list.size());
}
}
}
}
""",
"""
import java.util.List;

class A {
void test(Object obj, boolean b) {
if (obj instanceof List<?> list) {
if (b) {
System.out.println(list.size());
} else {
System.out.println(list.size());
}
}
}
}
"""
),
//language=java
java(
"""
import java.util.List;

class B {
void test(Object obj, boolean b) {
if (obj instanceof List) {
if (b) {
List<?> list = (List<?>) obj;
System.out.println(list.size());
} else {
List list = (List) obj;
System.out.println(list.size());
}
}
}
}
""",
"""
import java.util.List;

class B {
void test(Object obj, boolean b) {
if (obj instanceof List<?> list) {
if (b) {
System.out.println(list.size());
} else {
System.out.println(list.size());
}
}
}
}
"""
)
);
}

@Test
void conflictingVariableInBody() {
rewriteRun(
Expand Down
Loading
⚡