[ty] Diagnostic for generic classes that reference typevars in enclosing scope

crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md

@@ -71,6 +71,26 @@ reveal_type(generic_context(InheritedGenericPartiallySpecialized))
 reveal_type(generic_context(InheritedGenericFullySpecialized))
 ```
 
+In a nested class, references to typevars in an enclosing class are not allowed, but if they are
+present, they are not included in the class's generic context.
+
+```py
+class OuterClass(Generic[T]):
+    # error: [invalid-generic-class] "Generic class `InnerClass` must not reference type variables bound in an enclosing scope"
+    class InnerClass(list[T]): ...
+    # revealed: None
+    reveal_type(generic_context(InnerClass))
+
+    def method(self):
+        # error: [invalid-generic-class] "Generic class `InnerClassInMethod` must not reference type variables bound in an enclosing scope"
+        class InnerClassInMethod(list[T]): ...
+        # revealed: None
+        reveal_type(generic_context(InnerClassInMethod))
+
+# revealed: tuple[T@OuterClass]
+reveal_type(generic_context(OuterClass))
+```
+
 If you don't specialize a generic base class, we use the default specialization, which maps each
 typevar to its default value or `Any`. Since that base class is fully specialized, it does not make
 the inheriting class generic.

crates/ty_python_semantic/resources/mdtest/generics/scoping.md

@@ -260,27 +260,31 @@ class C[T]:
 
 ### Generic class within generic function
 
+<!-- snapshot-diagnostics -->
+
 ```py
 from typing import Iterable
 
 def f[T](x: T, y: T) -> None:
     class Ok[S]: ...
-    # TODO: error for reuse of typevar
+    # error: [invalid-generic-class] "Generic class `Bad1` must not reference type variables bound in an enclosing scope"
     class Bad1[T]: ...
-    # TODO: error for reuse of typevar
+    # error: [invalid-generic-class] "Generic class `Bad2` must not reference type variables bound in an enclosing scope"
     class Bad2(Iterable[T]): ...
 ```
 
 ### Generic class within generic class
 
+<!-- snapshot-diagnostics -->
+
 ```py
 from typing import Iterable
 
 class C[T]:
     class Ok1[S]: ...
-    # TODO: error for reuse of typevar
+    # error: [invalid-generic-class] "Generic class `Bad1` must not reference type variables bound in an enclosing scope"
     class Bad1[T]: ...
-    # TODO: error for reuse of typevar
+    # error: [invalid-generic-class] "Generic class `Bad2` must not reference type variables bound in an enclosing scope"
     class Bad2(Iterable[T]): ...
 ```
 

crates/ty_python_semantic/resources/mdtest/snapshots/scoping.md_-_Scoping_rules_for_ty…_-_Nested_formal_typeva…_-_Generic_class_within…_(3259718bf20b45a2).snap

@@ -0,0 +1,62 @@
+---
+source: crates/ty_test/src/lib.rs
+expression: snapshot
+---
+---
+mdtest name: scoping.md - Scoping rules for type variables - Nested formal typevars must be distinct - Generic class within generic function
+mdtest path: crates/ty_python_semantic/resources/mdtest/generics/scoping.md
+---
+
+# Python source files
+
+## mdtest_snippet.py
+
+```
+1 | from typing import Iterable
+2 | 
+3 | def f[T](x: T, y: T) -> None:
+4 |     class Ok[S]: ...
+5 |     # error: [invalid-generic-class] "Generic class `Bad1` must not reference type variables bound in an enclosing scope"
+6 |     class Bad1[T]: ...
+7 |     # error: [invalid-generic-class] "Generic class `Bad2` must not reference type variables bound in an enclosing scope"
+8 |     class Bad2(Iterable[T]): ...
+```
+
+# Diagnostics
+
+```
+error[invalid-generic-class]: Generic class `Bad1` must not reference type variables bound in an enclosing scope
+ --> src/mdtest_snippet.py:3:1
+  |
+1 |   from typing import Iterable
+2 |
+3 | / def f[T](x: T, y: T) -> None:
+4 | |     class Ok[S]: ...
+5 | |     # error: [invalid-generic-class] "Generic class `Bad1` must not reference type variables bound in an enclosing scope"
+6 | |     class Bad1[T]: ...
+  | |           ^^^^
+7 | |     # error: [invalid-generic-class] "Generic class `Bad2` must not reference type variables bound in an enclosing scope"
+8 | |     class Bad2(Iterable[T]): ...
+  | |________________________________^ Type variable `T` is bound in this enclosing scope
+  |
+info: rule `invalid-generic-class` is enabled by default
+
+```
+
+```
+error[invalid-generic-class]: Generic class `Bad2` must not reference type variables bound in an enclosing scope
+ --> src/mdtest_snippet.py:3:1
+  |
+1 |   from typing import Iterable
+2 |
+3 | / def f[T](x: T, y: T) -> None:
+4 | |     class Ok[S]: ...
+5 | |     # error: [invalid-generic-class] "Generic class `Bad1` must not reference type variables bound in an enclosing scope"
+6 | |     class Bad1[T]: ...
+7 | |     # error: [invalid-generic-class] "Generic class `Bad2` must not reference type variables bound in an enclosing scope"
+8 | |     class Bad2(Iterable[T]): ...
+  | |___________^^^^_________________^ Type variable `T` is bound in this enclosing scope
+  |
+info: rule `invalid-generic-class` is enabled by default
+
+```

crates/ty_python_semantic/resources/mdtest/snapshots/scoping.md_-_Scoping_rules_for_ty…_-_Nested_formal_typeva…_-_Generic_class_within…_(711fb86287c4d87b).snap

@@ -0,0 +1,62 @@
+---
+source: crates/ty_test/src/lib.rs
+expression: snapshot
+---
+---
+mdtest name: scoping.md - Scoping rules for type variables - Nested formal typevars must be distinct - Generic class within generic class
+mdtest path: crates/ty_python_semantic/resources/mdtest/generics/scoping.md
+---
+
+# Python source files
+
+## mdtest_snippet.py
+
+```
+1 | from typing import Iterable
+2 | 
+3 | class C[T]:
+4 |     class Ok1[S]: ...
+5 |     # error: [invalid-generic-class] "Generic class `Bad1` must not reference type variables bound in an enclosing scope"
+6 |     class Bad1[T]: ...
+7 |     # error: [invalid-generic-class] "Generic class `Bad2` must not reference type variables bound in an enclosing scope"
+8 |     class Bad2(Iterable[T]): ...
+```
+
+# Diagnostics
+
+```
+error[invalid-generic-class]: Generic class `Bad1` must not reference type variables bound in an enclosing scope
+ --> src/mdtest_snippet.py:3:1
+  |
+1 |   from typing import Iterable
+2 |
+3 | / class C[T]:
+4 | |     class Ok1[S]: ...
+5 | |     # error: [invalid-generic-class] "Generic class `Bad1` must not reference type variables bound in an enclosing scope"
+6 | |     class Bad1[T]: ...
+  | |           ^^^^
+7 | |     # error: [invalid-generic-class] "Generic class `Bad2` must not reference type variables bound in an enclosing scope"
+8 | |     class Bad2(Iterable[T]): ...
+  | |________________________________^ Type variable `T` is bound in this enclosing scope
+  |
+info: rule `invalid-generic-class` is enabled by default
+
+```
+
+```
+error[invalid-generic-class]: Generic class `Bad2` must not reference type variables bound in an enclosing scope
+ --> src/mdtest_snippet.py:3:1
+  |
+1 |   from typing import Iterable
+2 |
+3 | / class C[T]:
+4 | |     class Ok1[S]: ...
+5 | |     # error: [invalid-generic-class] "Generic class `Bad1` must not reference type variables bound in an enclosing scope"
+6 | |     class Bad1[T]: ...
+7 | |     # error: [invalid-generic-class] "Generic class `Bad2` must not reference type variables bound in an enclosing scope"
+8 | |     class Bad2(Iterable[T]): ...
+  | |___________^^^^_________________^ Type variable `T` is bound in this enclosing scope
+  |
+info: rule `invalid-generic-class` is enabled by default
+
+```

crates/ty_python_semantic/src/types/class.rs

@@ -1,3 +1,4 @@
+use std::cell::RefCell;
 use std::sync::{LazyLock, Mutex};
 
 use super::TypeVarVariance;
@@ -24,6 +25,7 @@ use crate::types::infer::nearest_enclosing_class;
 use crate::types::signatures::{CallableSignature, Parameter, Parameters, Signature};
 use crate::types::tuple::{TupleSpec, TupleType};
 use crate::types::typed_dict::typed_dict_params_from_class_def;
+use crate::types::visitor::{NonAtomicType, TypeKind, TypeVisitor, walk_non_atomic_type};
 use crate::types::{
     ApplyTypeMappingVisitor, Binding, BoundSuperError, BoundSuperType, CallableType,
     DataclassParams, DeprecatedInstance, FindLegacyTypeVarsVisitor, HasRelationToVisitor,
@@ -33,7 +35,7 @@ use crate::types::{
     declaration_type, determine_upper_bound, infer_definition_types,
 };
 use crate::{
-    Db, FxIndexMap, FxOrderSet, Program,
+    Db, FxIndexMap, FxIndexSet, FxOrderSet, Program,
     module_resolver::file_to_module,
     place::{
         Boundness, LookupError, LookupResult, Place, PlaceAndQualifiers, class_symbol,
@@ -1458,13 +1460,69 @@ impl<'db> ClassLiteral<'db> {
     ) -> Option<GenericContext<'db>> {
         GenericContext::from_base_classes(
             db,
+            || self.definition(db),
             self.explicit_bases(db)
                 .iter()
                 .copied()
                 .filter(|ty| matches!(ty, Type::GenericAlias(_))),
         )
     }
 
+    /// Returns all of the typevars that are referenced in this class's definition. This includes
+    /// any typevars bound in its generic context, as well as any typevars mentioned in its base
+    /// class list. (This is used to ensure that classes do not bind or reference typevars from
+    /// enclosing generic contexts.)
+    pub(crate) fn typevars_referenced_in_definition(
+        self,
+        db: &'db dyn Db,
+    ) -> FxOrderSet<BoundTypeVarInstance<'db>> {
+        struct CollectTypeVars<'db> {
+            typevars: RefCell<FxOrderSet<BoundTypeVarInstance<'db>>>,
+            seen_types: RefCell<FxIndexSet<NonAtomicType<'db>>>,
+        }
+
+        impl<'db> TypeVisitor<'db> for CollectTypeVars<'db> {
+            fn should_visit_lazy_type_attributes(&self) -> bool {
+                false
+            }
+
+            fn visit_bound_type_var_type(
+                &self,
+                _db: &'db dyn Db,
+                bound_typevar: BoundTypeVarInstance<'db>,
+            ) {
+                self.typevars.borrow_mut().insert(bound_typevar);
+            }
+
+            fn visit_type(&self, db: &'db dyn Db, ty: Type<'db>) {
+                match TypeKind::from(ty) {
+                    TypeKind::Atomic => {}
+                    TypeKind::NonAtomic(non_atomic_type) => {
+                        if !self.seen_types.borrow_mut().insert(non_atomic_type) {
+                            // If we have already seen this type, we can skip it.
+                            return;
+                        }
+                        walk_non_atomic_type(db, non_atomic_type, self);
+                    }
+                }
+            }
+        }
+
+        let visitor = CollectTypeVars {
+            typevars: RefCell::new(FxOrderSet::default()),
+            seen_types: RefCell::new(FxIndexSet::default()),
+        };
+        if let Some(generic_context) = self.generic_context(db) {
+            for bound_typevar in generic_context.variables(db) {
+                visitor.visit_bound_type_var_type(db, bound_typevar);
+            }
+        }
+        for base in self.explicit_bases(db) {
+            visitor.visit_type(db, *base);
+        }
+        visitor.typevars.into_inner()
+    }
+
     /// Returns the generic context that should be inherited by any constructor methods of this
     /// class.
     ///

crates/ty_python_semantic/src/types/generics.rs

@@ -14,17 +14,17 @@ use crate::types::instance::{Protocol, ProtocolInstanceType};
 use crate::types::signatures::{Parameter, Parameters, Signature};
 use crate::types::tuple::{TupleSpec, TupleType, walk_tuple_type};
 use crate::types::{
-    ApplyTypeMappingVisitor, BoundTypeVarInstance, ClassLiteral, FindLegacyTypeVarsVisitor,
-    HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor, KnownClass, KnownInstanceType,
-    MaterializationKind, NormalizedVisitor, Type, TypeContext, TypeMapping, TypeRelation,
-    TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind, TypeVarVariance, UnionType,
-    binding_type, declaration_type,
+    ApplyTypeMappingVisitor, BindingContext, BoundTypeVarInstance, ClassLiteral,
+    FindLegacyTypeVarsVisitor, HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor,
+    KnownClass, KnownInstanceType, MaterializationKind, NormalizedVisitor, Type, TypeContext,
+    TypeMapping, TypeRelation, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind,
+    TypeVarVariance, UnionType, binding_type, declaration_type,
 };
 use crate::{Db, FxOrderMap, FxOrderSet};
 
 /// Returns an iterator of any generic context introduced by the given scope or any enclosing
 /// scope.
-fn enclosing_generic_contexts<'db>(
+pub(crate) fn enclosing_generic_contexts<'db>(
     db: &'db dyn Db,
     index: &SemanticIndex<'db>,
     scope: FileScopeId,
@@ -317,15 +317,30 @@ impl<'db> GenericContext<'db> {
     /// list.
     pub(crate) fn from_base_classes(
         db: &'db dyn Db,
+        definition: impl FnOnce() -> Definition<'db>,
         bases: impl Iterator<Item = Type<'db>>,
     ) -> Option<Self> {
         let mut variables = FxOrderSet::default();
         for base in bases {
             base.find_legacy_typevars(db, None, &mut variables);
         }
+
+        // If there are no legacy typevars mentioned in the base class list, we can return early.
+        if variables.is_empty() {
+            return None;
+        }
+
+        // If there are legacy typevars, filter out the ones that are not bound by this class. (We
+        // do this as a post-processing step, instead of by passing in a parameter to
+        // `find_legacy_typevars`, since there are very many classes that do not reference legacy
+        // typevars at all, and this avoids adding a salsa dependency on the class `Definition` in
+        // those cases.)
+        let binding_context = BindingContext::Definition(definition());
+        variables.retain(|bound_typevar| bound_typevar.binding_context(db) == binding_context);
         if variables.is_empty() {
             return None;
         }
+
         Some(Self::from_typevar_instances(db, variables))
     }
 
@@ -413,6 +428,15 @@ impl<'db> GenericContext<'db> {
             .all(|bound_typevar| other_variables.contains_key(&bound_typevar))
     }
 
+    pub(crate) fn binds_named_typevar(
+        self,
+        db: &'db dyn Db,
+        name: &'db ast::name::Name,
+    ) -> Option<BoundTypeVarInstance<'db>> {
+        self.variables(db)
+            .find(|self_bound_typevar| self_bound_typevar.typevar(db).name(db) == name)
+    }
+
     pub(crate) fn binds_typevar(
         self,
         db: &'db dyn Db,