Fix false positive bad-override when overriding methods using bounded TypeVars

When checking override compatibility between generic methods, type variables in the child method signature are instantiated with fresh inference variables and matched contravariantly against the parent parameters. When a parameter is a union like `T | None`, `is_subset_eq` previously attempted to check whether the `TypeVar` bound was a subtype of the union before splitting the RHS union. When the RHS union contained an unsolved inference variable, the speculative check succeeded under a snapshot and rolled back all variable assignments, leaving the child type variable unconstrained and causing override validation to fail.

To fix this, check `Type::Quantified` against individual members of a RHS union first before falling back to testing whether the `TypeVar` bound or constraints as a whole satisfy the union. This allows inference variables in the RHS union to be properly constrained to the parent type variable, correctly validating method overrides.

Author: kavix

pyrefly/lib/solver/subset.rs

@@ -1867,52 +1867,6 @@ impl<'solver, 'subset, Ans: LookupAnswer> Subset<'solver, 'subset, Ans> {
             {
                 Ok(())
             }
-            // Given `A | B <: C | D` we must always split the LHS first, but a quantified might be hiding a LHS union in its bounds.
-            // Given (Quantified(bounds = A | B), A | B), we need to examine the bound _before_ splitting up the RHS union.
-            // But given (T@Quantified(bounds = ...), T | Something), we need to split the union.
-            // Therefore try these quantified cases, but only pick them if they work.
-            (Type::Quantified(q), u)
-                if let Restriction::Bound(bound) = q.restriction()
-                    // A bare inference variable can preserve the quantified type itself. Expanding
-                    // it to its bound here would make inference depend on which argument is checked
-                    // first (https://github.com/facebook/pyrefly/issues/4187).
-                    && !matches!(u, Type::Union(union) if union.members.iter().any(|t| matches!(t, Type::Var(_))))
-                    && self
-                        .solver
-                        .with_snapshot(&u.collect_maybe_placeholder_vars(), || {
-                            self.is_subset_eq(bound, u)
-                        })
-                        .is_ok() =>
-            {
-                Ok(())
-            }
-            (Type::Quantified(q), u)
-                if let Restriction::ShapeExtension(extension) = q.restriction()
-                    && self
-                        .solver
-                        .with_snapshot(&u.collect_maybe_placeholder_vars(), || {
-                            self.is_subset_eq(
-                                &extension.upper_bound(self.type_order.stdlib(), &self.solver.heap),
-                                u,
-                            )
-                        })
-                        .is_ok() =>
-            {
-                Ok(())
-            }
-            (Type::Quantified(q), u)
-                if let Restriction::Constraints(constraints) = q.restriction()
-                    && self
-                        .solver
-                        .with_snapshot(&u.collect_maybe_placeholder_vars(), || {
-                            all(constraints.iter(), |constraint| {
-                                self.is_subset_eq(constraint, u)
-                            })
-                        })
-                        .is_ok() =>
-            {
-                Ok(())
-            }
             (Type::Quantified(q), u @ Type::Tuple(_)) if q.is_type_var_tuple() => self
                 .is_subset_eq(
                     &self.solver.heap.mk_unbounded_tuple(
@@ -2084,6 +2038,25 @@ impl<'solver, 'subset, Ans: LookupAnswer> Subset<'solver, 'subset, Ans> {
                     all(members.iter(), |m| {
                         self.is_subset_eq(&Type::type_of(m.clone()), want)
                     })
+                } else if let Type::Quantified(q) = l {
+                    // A quantified type parameter may hide a union in its bound or constraints
+                    // (e.g. `T: (A, B)` or `T: A | B`). If per-member matching against the RHS
+                    // union failed, check whether the bound or all constraints as a whole satisfy
+                    // the RHS union.
+                    match q.restriction() {
+                        Restriction::Bound(bound) => self.is_subset_eq(bound, want),
+                        Restriction::Constraints(constraints) => {
+                            all(constraints.iter(), |constraint| {
+                                self.is_subset_eq(constraint, want)
+                            })
+                        }
+                        Restriction::ShapeExtension(extension) => {
+                            let upper =
+                                extension.upper_bound(self.type_order.stdlib(), &self.solver.heap);
+                            self.is_subset_eq(&upper, want)
+                        }
+                        Restriction::Unrestricted => Err(error.unwrap_or(SubsetError::Other)),
+                    }
                 } else {
                     Err(error.unwrap_or(SubsetError::Other))
                 }
@@ -2098,13 +2071,21 @@ impl<'solver, 'subset, Ans: LookupAnswer> Subset<'solver, 'subset, Ans> {
                     _ => result,
                 }
             }
-            (Type::Quantified(q), u) if !q.restriction().is_restricted() => self.is_subset_eq(
-                &self
-                    .solver
-                    .heap
-                    .mk_class_type(self.type_order.stdlib().object().clone()),
-                u,
-            ),
+            (Type::Quantified(q), u) => match q.restriction() {
+                Restriction::Bound(bound) => self.is_subset_eq(bound, u),
+                Restriction::Constraints(constraints) => all(constraints.iter(), |constraint| {
+                    self.is_subset_eq(constraint, u)
+                }),
+                Restriction::ShapeExtension(extension) => {
+                    let upper = extension.upper_bound(self.type_order.stdlib(), &self.solver.heap);
+                    self.is_subset_eq(&upper, u)
+                }
+                Restriction::Unrestricted => {
+                    let upper = q.upper_bound(self.type_order.stdlib(), &self.solver.heap);
+                    self.is_subset_eq(&upper, u)
+                }
+            },
+
             (Type::Module(_), Type::ClassType(cls)) if cls.has_qname("types", "ModuleType") => {
                 Ok(())
             }
@@ -2652,12 +2633,9 @@ impl<'solver, 'subset, Ans: LookupAnswer> Subset<'solver, 'subset, Ans> {
             {
                 self.is_subset_literal_int_size(n, got, false)
             }
-            (Type::Int(_) | Type::Quantified(_), Type::ClassType(cls))
-                if is_int_class_type(cls) =>
-            {
-                Ok(())
-            }
+            (Type::Int(_), Type::ClassType(cls)) if is_int_class_type(cls) => Ok(()),
             (Type::QuantifiedValue(_), Type::ClassType(cls)) if is_int_class_type(cls) => Ok(()),
+
             (Type::Literal(l_lit), Type::Literal(u_lit)) => {
                 ok_or(l_lit.value == u_lit.value, SubsetError::Other)
             }

pyrefly/lib/test/class_overrides.rs

@@ -2242,3 +2242,38 @@ class D(C):
 from stub import C, D
     "#,
 );
+
+testcase!(
+    test_override_module_level_typevar,
+    r#"
+from typing import TypeVar
+from typing_extensions import override
+
+T = TypeVar("T", bound=int)
+TConstrained = TypeVar("TConstrained", int, str)
+
+class Base:
+    def method(self, x: T | None = None) -> T: ...
+    def method_no_opt(self, x: T) -> T: ...
+    def method_constrained(self, x: TConstrained | None = None) -> TConstrained: ...
+
+class Derived(Base):
+    @override
+    def method(self, x: T | None = None) -> T:
+        raise NotImplementedError
+    @override
+    def method_no_opt(self, x: T) -> T:
+        raise NotImplementedError
+    @override
+    def method_constrained(self, x: TConstrained | None = None) -> TConstrained:
+        raise NotImplementedError
+
+class BasePep:
+    def method[T: int](self, x: T | None = None) -> T: ...
+
+class DerivedPep(BasePep):
+    @override
+    def method[T: int](self, x: T | None = None) -> T:
+        raise NotImplementedError
+    "#,
+);