Skip to content

Commit 69ea1ba

Browse files
cushonError Prone Team
authored andcommitted
Require types to be the same in PatternMatchingInstanceOf
Accepting subtypes can result in e.g. different overloads being selected. Fixes #4921 PiperOrigin-RevId: 741195433
1 parent 831645f commit 69ea1ba

File tree

2 files changed

+77
-16
lines changed

2 files changed

+77
-16
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceof.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,11 @@ public Description matchInstanceOf(InstanceOfTree instanceOfTree, VisitorState s
142142
if (!variableTree.getInitializer().equals(treePath.getLeaf())) {
143143
return null;
144144
}
145+
// Check that the type is exactly the same (not a subtypes), since refactoring cases where the
146+
// instanceof type is a supertype of the cast type could affect overload resolution.
145147
if (!state
146148
.getTypes()
147-
.isSubtype(getType(instanceOfTree.getType()), getType(variableTree.getType()))) {
149+
.isSameType(getType(instanceOfTree.getType()), getType(variableTree.getType()))) {
148150
return null;
149151
}
150152
return variableTree;
@@ -243,7 +245,7 @@ private static ImmutableSet<TreePath> findAllCasts(
243245
public Void visitTypeCast(TypeCastTree node, Void unused) {
244246
if (getSymbol(node.getExpression()) instanceof VarSymbol v) {
245247
if (v.equals(symbol)
246-
&& state.getTypes().isSubtype(targetType, getType(node.getType()))) {
248+
&& state.getTypes().isSameType(getType(node.getType()), targetType)) {
247249
usages.add(getUsage(getCurrentPath()));
248250
}
249251
}

core/src/test/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceofTest.java

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -719,20 +719,7 @@ public Class stringify(Object o) {
719719
}
720720
}
721721
""")
722-
.addOutputLines(
723-
"Test.java",
724-
"""
725-
class Test<T> {
726-
private String val;
727-
728-
public Class stringify(Object o) {
729-
if (o instanceof Class<?> c) {
730-
return c;
731-
}
732-
return null;
733-
}
734-
}
735-
""")
722+
.expectUnchanged()
736723
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
737724
}
738725

@@ -830,4 +817,76 @@ boolean test_if() {
830817
""")
831818
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
832819
}
820+
821+
// https://github.com/google/error-prone/issues/4921
822+
@Test
823+
public void castToSupertypeOfInstanceofCheck_noFinding() {
824+
assume().that(Runtime.version().feature()).isAtLeast(21);
825+
helper
826+
.addInputLines(
827+
"Test.java",
828+
"""
829+
import java.nio.file.Path;
830+
import java.util.ArrayList;
831+
832+
public class Test {
833+
void superinterface() {
834+
Object o = Path.of(".");
835+
if (o instanceof Path) {
836+
f((Iterable<?>) o);
837+
}
838+
}
839+
840+
void f(Comparable<?> c) {}
841+
842+
void f(Iterable<?> c) {}
843+
844+
void f(Path p) {}
845+
846+
void rawtypes() {
847+
Object o = new ArrayList<Integer>();
848+
if (o instanceof ArrayList<?>) {
849+
@SuppressWarnings("rawtypes")
850+
ArrayList list = (ArrayList) o;
851+
rawTypeNecessary(list);
852+
}
853+
}
854+
855+
void rawTypeNecessary(ArrayList<Integer> l) {}
856+
}
857+
""")
858+
.addOutputLines(
859+
"Test.java",
860+
"""
861+
import java.nio.file.Path;
862+
import java.util.ArrayList;
863+
864+
public class Test {
865+
void superinterface() {
866+
Object o = Path.of(".");
867+
if (o instanceof Path) {
868+
f((Iterable<?>) o);
869+
}
870+
}
871+
872+
void f(Comparable<?> c) {}
873+
874+
void f(Iterable<?> c) {}
875+
876+
void f(Path p) {}
877+
878+
void rawtypes() {
879+
Object o = new ArrayList<Integer>();
880+
if (o instanceof ArrayList<?>) {
881+
@SuppressWarnings("rawtypes")
882+
ArrayList list = (ArrayList) o;
883+
rawTypeNecessary(list);
884+
}
885+
}
886+
887+
void rawTypeNecessary(ArrayList<Integer> l) {}
888+
}
889+
""")
890+
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
891+
}
833892
}

0 commit comments

Comments
 (0)