Fix isinstance narrowing for unions with supertype members (#2169)

pyrefly/lib/alt/narrow.rs

@@ -227,19 +227,36 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
                     let (vs, right) = self
                         .solver()
                         .fresh_quantified(&tparams, right, self.uniques);
-                    let result = self.intersect_with_fallback(l, &right, &|| {
-                        // TODO: falling back to Never when the lhs is a union is a hack to get
-                        // reasonable behavior in cases like this:
-                        //     def f(x: int | list[int]):
-                        //         if isinstance(x, Iterable):
-                        //             reveal_type(x)
-                        // We want to narrow x to just `list[int]`, rather than `(int & Iterable[Unknown]) | list[int]`
-                        if left.is_union() {
-                            Type::never()
-                        } else {
-                            right.clone()
-                        }
-                    });
+                    // When the isinstance target has type parameters and we're in a union context,
+                    // check if the union member's class is a proper superclass of the isinstance
+                    // target's class. If so, return Never to avoid fabricating type arguments.
+                    // Example: Mapping[str, int] | Iterable[tuple[str, int]] with isinstance(x, Mapping)
+                    // - Mapping[str, int] is kept (its class equals the isinstance target class)
+                    // - Iterable[tuple[str, int]] becomes Never (Iterable is a superclass of Mapping,
+                    //   and narrowing it to Mapping[tuple[str, int], Unknown] would be incorrect)
+                    let is_supertype_in_union = !tparams.is_empty()
+                        && left.is_union()
+                        && match (&right, l) {
+                            (Type::ClassType(right_cls), Type::ClassType(l_cls)) => {
+                                let right_class = right_cls.class_object();
+                                let l_class = l_cls.class_object();
+                                // Check if isinstance target class is a PROPER subclass of union member class
+                                right_class != l_class
+                                    && self.type_order().has_superclass(right_class, l_class)
+                            }
+                            _ => false,
+                        };
+                    let result = if is_supertype_in_union {
+                        Type::never()
+                    } else {
+                        self.intersect_with_fallback(l, &right, &|| {
+                            if left.is_union() {
+                                Type::never()
+                            } else {
+                                right.clone()
+                            }
+                        })
+                    };
                     // These are safe to ignore, as the only possible specialization errors are handled elsewhere:
                     // * If `left` is an invalid specialization, the error has already been reported at its definition site.
                     // * Unsafe runtime protocol overlaps are separately checked for in special_calls.rs.

pyrefly/lib/test/narrow.rs

@@ -2533,3 +2533,86 @@ def f(o: C):
         o.x.foo
     "#,
 );
+
+// https://github.com/facebook/pyrefly/issues/2169
+// When narrowing a union with isinstance, if a union member is a supertype
+// of the isinstance target (e.g., Iterable is a supertype of Mapping), we should
+// not narrow that member to the isinstance target.
+testcase!(
+    test_isinstance_mapping_iterable_union,
+    r#"
+from collections.abc import Iterable, Mapping
+from typing import assert_type
+
+def test1(arg: Mapping[str, int] | Iterable[tuple[str, int]]) -> None:
+    if isinstance(arg, Mapping):
+        assert_type(arg, Mapping[str, int])
+    else:
+        assert_type(arg, Iterable[tuple[str, int]])
+
+def test2(arg: Mapping[str, int] | Iterable[tuple[str, int]]) -> None:
+    if not isinstance(arg, Mapping):
+        assert_type(arg, Iterable[tuple[str, int]])
+    else:
+        assert_type(arg, Mapping[str, int])
+    "#,
+);
+
+// Additional edge cases for isinstance narrowing with class hierarchies
+testcase!(
+    test_isinstance_subclass_in_union,
+    r#"
+from collections.abc import Mapping
+from typing import assert_type
+
+# When union member is a subclass of isinstance target, it should be kept
+def test_dict_mapping(arg: dict[str, int] | list[int]) -> None:
+    if isinstance(arg, Mapping):
+        # dict is a subclass of Mapping, so it passes isinstance check
+        assert_type(arg, dict[str, int])
+    else:
+        assert_type(arg, list[int])
+    "#,
+);
+
+testcase!(
+    test_isinstance_collection_mapping,
+    r#"
+from collections.abc import Collection, Mapping
+from typing import assert_type
+
+# Collection is a superclass of Mapping, so Collection[X] should become Never
+def test(arg: Mapping[str, int] | Collection[tuple[str, int]]) -> None:
+    if isinstance(arg, Mapping):
+        assert_type(arg, Mapping[str, int])
+    "#,
+);
+
+testcase!(
+    test_isinstance_unrelated_classes,
+    r#"
+from collections.abc import Sequence, Mapping
+from typing import assert_type
+
+# Sequence and Mapping are unrelated - neither is subclass of the other
+def test(arg: Sequence[int] | Mapping[str, int]) -> None:
+    if isinstance(arg, Mapping):
+        assert_type(arg, Mapping[str, int])
+    else:
+        assert_type(arg, Sequence[int])
+    "#,
+);
+
+testcase!(
+    test_isinstance_with_iterable_target,
+    r#"
+from collections.abc import Iterable, Mapping
+from typing import assert_type
+
+# When isinstance target is the superclass, both should be kept
+def test(arg: Mapping[str, int] | Iterable[tuple[str, int]]) -> None:
+    if isinstance(arg, Iterable):  # E: Runtime checkable protocol `Iterable` has an unsafe overlap
+        # Both Mapping and Iterable are Iterables, so both kept
+        assert_type(arg, Mapping[str, int] | Iterable[tuple[str, int]])
+    "#,
+);