[ty] typing.Self is bound by the method, not the class (#19784)

This fixes our logic for binding a legacy typevar with its binding
context. (To recap, a legacy typevar starts out "unbound" when it is
first created, and each time it's used in a generic class or function,
we "bind" it with the corresponding `Definition`.)

We treat `typing.Self` the same as a legacy typevar, and so we apply
this binding logic to it too. Before, we were using the enclosing class
as its binding context. But that's not correct — it's the method where
`typing.Self` is used that binds the typevar. (Each invocation of the
method will find a new specialization of `Self` based on the specific
instance type containing the invoked method.)

This required plumbing through some additional state to the
`in_type_expression` method.

This also revealed that we weren't handling `Self`-typed instance
attributes correctly (but were coincidentally not getting the expected
false positive diagnostics).

crates/ty_python_semantic/src/types.rs

@@ -47,8 +47,8 @@ use crate::types::function::{
     DataclassTransformerParams, FunctionSpans, FunctionType, KnownFunction,
 };
 use crate::types::generics::{
-    GenericContext, PartialSpecialization, Specialization, walk_generic_context,
-    walk_partial_specialization, walk_specialization,
+    GenericContext, PartialSpecialization, Specialization, bind_legacy_typevar,
+    walk_generic_context, walk_partial_specialization, walk_specialization,
 };
 pub use crate::types::ide_support::{
     CallSignatureDetails, Member, all_members, call_signature_details, definition_kind_for_name,
@@ -5268,12 +5268,13 @@ impl<'db> Type<'db> {
     /// expression, it names the type `Type::NominalInstance(builtins.int)`, that is, all objects whose
     /// `__class__` is `int`.
     ///
-    /// The `scope_id` argument must always be a scope from the file we are currently inferring, so
+    /// The `scope_id` and `legacy_typevar_binding_context` arguments must always come from the file we are currently inferring, so
     /// as to avoid cross-module AST dependency.
     pub(crate) fn in_type_expression(
         &self,
         db: &'db dyn Db,
         scope_id: ScopeId<'db>,
+        legacy_typevar_binding_context: Option<Definition<'db>>,
     ) -> Result<Type<'db>, InvalidTypeExpressionError<'db>> {
         match self {
             // Special cases for `float` and `complex`
@@ -5402,16 +5403,26 @@ impl<'db> Type<'db> {
                         "nearest_enclosing_class must return type that can be instantiated",
                     );
                     let class_definition = class.definition(db);
-                    Ok(Type::TypeVar(TypeVarInstance::new(
+                    let typevar = TypeVarInstance::new(
                         db,
                         ast::name::Name::new_static("Self"),
                         Some(class_definition),
-                        Some(class_definition),
+                        None,
                         Some(TypeVarBoundOrConstraints::UpperBound(instance)),
                         TypeVarVariance::Invariant,
                         None,
                         TypeVarKind::Implicit,
-                    )))
+                    );
+                    let typevar = bind_legacy_typevar(
+                        db,
+                        &module,
+                        index,
+                        scope_id.file_scope_id(db),
+                        legacy_typevar_binding_context,
+                        typevar,
+                    )
+                    .unwrap_or(typevar);
+                    Ok(Type::TypeVar(typevar))
                 }
                 SpecialFormType::TypeAlias => Ok(Type::Dynamic(DynamicType::TodoTypeAlias)),
                 SpecialFormType::TypedDict => Err(InvalidTypeExpressionError {
@@ -5486,7 +5497,7 @@ impl<'db> Type<'db> {
                 let mut builder = UnionBuilder::new(db);
                 let mut invalid_expressions = smallvec::SmallVec::default();
                 for element in union.elements(db) {
-                    match element.in_type_expression(db, scope_id) {
+                    match element.in_type_expression(db, scope_id, legacy_typevar_binding_context) {
                         Ok(type_expr) => builder = builder.add(type_expr),
                         Err(InvalidTypeExpressionError {
                             fallback_type,
@@ -6660,7 +6671,7 @@ impl<'db> InvalidTypeExpression<'db> {
             return;
         };
         if module_member_with_same_name
-            .in_type_expression(db, scope)
+            .in_type_expression(db, scope, None)
             .is_err()
         {
             return;

crates/ty_python_semantic/src/types/generics.rs

@@ -9,6 +9,7 @@ use crate::semantic_index::scope::{FileScopeId, NodeWithScopeKind};
 use crate::semantic_index::{SemanticIndex, semantic_index};
 use crate::types::class::ClassType;
 use crate::types::class_base::ClassBase;
+use crate::types::infer::infer_definition_types;
 use crate::types::instance::{NominalInstanceType, Protocol, ProtocolInstanceType};
 use crate::types::signatures::{Parameter, Parameters, Signature};
 use crate::types::tuple::{TupleSpec, TupleType};
@@ -20,7 +21,7 @@ use crate::{Db, FxOrderSet};
 
 /// Returns an iterator of any generic context introduced by the given scope or any enclosing
 /// scope.
-pub(crate) fn enclosing_generic_contexts<'db>(
+fn enclosing_generic_contexts<'db>(
     db: &'db dyn Db,
     module: &ParsedModuleRef,
     index: &SemanticIndex<'db>,
@@ -35,7 +36,9 @@ pub(crate) fn enclosing_generic_contexts<'db>(
                     .generic_context(db)
             }
             NodeWithScopeKind::Function(function) => {
-                binding_type(db, index.expect_single_definition(function.node(module)))
+                infer_definition_types(db, index.expect_single_definition(function.node(module)))
+                    .undecorated_type()
+                    .expect("function should have undecorated type")
                     .into_function_literal()?
                     .signature(db)
                     .iter()
@@ -59,6 +62,37 @@ fn bound_legacy_typevars<'db>(
         .filter(|typevar| typevar.is_legacy(db))
 }
 
+/// Binds an unbound legacy typevar.
+///
+/// When a legacy typevar is first created, we will have a [`TypeVarInstance`] which does not have
+/// an associated binding context. When the typevar is used in a generic class or function, we
+/// "bind" it, adding the [`Definition`] of the generic class or function as its "binding context".
+///
+/// When an expression resolves to a legacy typevar, our inferred type will refer to the unbound
+/// [`TypeVarInstance`] from when the typevar was first created. This function walks the scopes
+/// that enclosing the expression, looking for the innermost binding context that binds the
+/// typevar.
+///
+/// If no enclosing scope has already bound the typevar, we might be in a syntactic position that
+/// is about to bind it (indicated by a non-`None` `legacy_typevar_binding_context`), in which case
+/// we bind the typevar with that new binding context.
+pub(crate) fn bind_legacy_typevar<'db>(
+    db: &'db dyn Db,
+    module: &ParsedModuleRef,
+    index: &SemanticIndex<'db>,
+    containing_scope: FileScopeId,
+    legacy_typevar_binding_context: Option<Definition<'db>>,
+    typevar: TypeVarInstance<'db>,
+) -> Option<TypeVarInstance<'db>> {
+    enclosing_generic_contexts(db, module, index, containing_scope)
+        .find_map(|enclosing_context| enclosing_context.binds_legacy_typevar(db, typevar))
+        .or_else(|| {
+            legacy_typevar_binding_context.map(|legacy_typevar_binding_context| {
+                typevar.with_binding_context(db, legacy_typevar_binding_context)
+            })
+        })
+}
+
 /// A list of formal type variables for a generic function, class, or type alias.
 ///
 /// TODO: Handle nested generic contexts better, with actual parent links to the lexically
@@ -259,12 +293,13 @@ impl<'db> GenericContext<'db> {
         db: &'db dyn Db,
         typevar: TypeVarInstance<'db>,
     ) -> Option<TypeVarInstance<'db>> {
-        assert!(typevar.is_legacy(db));
+        assert!(typevar.is_legacy(db) || typevar.is_implicit(db));
         let typevar_def = typevar.definition(db);
         self.variables(db)
             .iter()
             .find(|self_typevar| {
-                self_typevar.is_legacy(db) && self_typevar.definition(db) == typevar_def
+                (self_typevar.is_legacy(db) || self_typevar.is_implicit(db))
+                    && self_typevar.definition(db) == typevar_def
             })
             .copied()
     }

crates/ty_python_semantic/src/types/infer.rs

@@ -110,7 +110,7 @@ use crate::types::enums::is_enum_class;
 use crate::types::function::{
     FunctionDecorators, FunctionLiteral, FunctionType, KnownFunction, OverloadLiteral,
 };
-use crate::types::generics::{GenericContext, enclosing_generic_contexts};
+use crate::types::generics::{GenericContext, bind_legacy_typevar};
 use crate::types::mro::MroErrorKind;
 use crate::types::signatures::{CallableSignature, Signature};
 use crate::types::tuple::{TupleSpec, TupleType};
@@ -519,6 +519,9 @@ struct DefinitionInferenceExtra<'db> {
 
     /// The diagnostics for this region.
     diagnostics: TypeCheckDiagnostics,
+
+    /// For function definitions, the undecorated type of the function.
+    undecorated_type: Option<Type<'db>>,
 }
 
 impl<'db> DefinitionInference<'db> {
@@ -610,6 +613,10 @@ impl<'db> DefinitionInference<'db> {
     fn fallback_type(&self) -> Option<Type<'db>> {
         self.is_cycle_callback().then_some(Type::Never)
     }
+
+    pub(crate) fn undecorated_type(&self) -> Option<Type<'db>> {
+        self.extra.as_ref().and_then(|extra| extra.undecorated_type)
+    }
 }
 
 /// The inferred types for an expression region.
@@ -823,6 +830,9 @@ pub(super) struct TypeInferenceBuilder<'db, 'ast> {
     /// is a stub file but we're still in a non-deferred region.
     deferred_state: DeferredExpressionState,
 
+    /// For function definitions, the undecorated type of the function.
+    undecorated_type: Option<Type<'db>>,
+
     /// The fallback type for missing expressions/bindings/declarations.
     ///
     /// This is used only when constructing a cycle-recovery `TypeInference`.
@@ -858,6 +868,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             declarations: VecMap::default(),
             legacy_typevar_binding_context: None,
             deferred: VecSet::default(),
+            undecorated_type: None,
             cycle_fallback: false,
         }
     }
@@ -2662,6 +2673,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             function_literal,
             type_mappings,
         ));
+        self.undecorated_type = Some(inferred_ty);
 
         for (decorator_ty, decorator_node) in decorator_types_and_nodes.iter().rev() {
             inferred_ty = match decorator_ty
@@ -6427,7 +6439,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             self.infer_place_load(PlaceExprRef::from(&expr), ast::ExprRef::Name(name_node));
 
         resolved
-            .map_type(|ty| {
+            .map_type(|ty| match ty {
                 // If the expression resolves to a legacy typevar, we will have the TypeVarInstance
                 // that was created when the typevar was created, which will not have an associated
                 // binding context. If this expression appears inside of a generic context that
@@ -6438,40 +6450,31 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                 // If the legacy typevar is still unbound after that search, and we are in a
                 // context that binds unbound legacy typevars (i.e., the signature of a generic
                 // function), bind it with that context.
-                let find_legacy_typevar_binding = |typevar: TypeVarInstance<'db>| {
-                    enclosing_generic_contexts(
+                Type::TypeVar(typevar) if typevar.is_legacy(self.db()) => bind_legacy_typevar(
+                    self.db(),
+                    self.context.module(),
+                    self.index,
+                    self.scope().file_scope_id(self.db()),
+                    self.legacy_typevar_binding_context,
+                    typevar,
+                )
+                .map(Type::TypeVar)
+                .unwrap_or(ty),
+                Type::KnownInstance(KnownInstanceType::TypeVar(typevar))
+                    if typevar.is_legacy(self.db()) =>
+                {
+                    bind_legacy_typevar(
                         self.db(),
                         self.context.module(),
                         self.index,
                         self.scope().file_scope_id(self.db()),
+                        self.legacy_typevar_binding_context,
+                        typevar,
                     )
-                    .find_map(|enclosing_context| {
-                        enclosing_context.binds_legacy_typevar(self.db(), typevar)
-                    })
-                    .or_else(|| {
-                        self.legacy_typevar_binding_context
-                            .map(|legacy_typevar_binding_context| {
-                                typevar
-                                    .with_binding_context(self.db(), legacy_typevar_binding_context)
-                            })
-                    })
-                };
-
-                match ty {
-                    Type::TypeVar(typevar) if typevar.is_legacy(self.db()) => {
-                        find_legacy_typevar_binding(typevar)
-                            .map(Type::TypeVar)
-                            .unwrap_or(ty)
-                    }
-                    Type::KnownInstance(KnownInstanceType::TypeVar(typevar))
-                        if typevar.is_legacy(self.db()) =>
-                    {
-                        find_legacy_typevar_binding(typevar)
-                            .map(|typevar| Type::KnownInstance(KnownInstanceType::TypeVar(typevar)))
-                            .unwrap_or(ty)
-                    }
-                    _ => ty,
+                    .map(|typevar| Type::KnownInstance(KnownInstanceType::TypeVar(typevar)))
+                    .unwrap_or(ty)
                 }
+                _ => ty,
             })
             // Not found in the module's explicitly declared global symbols?
             // Check the "implicit globals" such as `__doc__`, `__file__`, `__name__`, etc.
@@ -9130,6 +9133,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             deferred,
             cycle_fallback,
 
+            // Ignored; only relevant to definition regions
+            undecorated_type: _,
+
             // builder only state
             legacy_typevar_binding_context: _,
             deferred_state: _,
@@ -9189,6 +9195,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             declarations,
             deferred,
             cycle_fallback,
+            undecorated_type,
 
             // builder only state
             legacy_typevar_binding_context: _,
@@ -9202,14 +9209,18 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         let _ = scope;
         let diagnostics = context.finish();
 
-        let extra =
-            (!diagnostics.is_empty() || cycle_fallback || !deferred.is_empty()).then(|| {
-                Box::new(DefinitionInferenceExtra {
-                    cycle_fallback,
-                    deferred: deferred.into_boxed_slice(),
-                    diagnostics,
-                })
-            });
+        let extra = (!diagnostics.is_empty()
+            || cycle_fallback
+            || undecorated_type.is_some()
+            || !deferred.is_empty())
+        .then(|| {
+            Box::new(DefinitionInferenceExtra {
+                cycle_fallback,
+                deferred: deferred.into_boxed_slice(),
+                diagnostics,
+                undecorated_type,
+            })
+        });
 
         if bindings.len() > 20 {
             tracing::debug!(
@@ -9253,6 +9264,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             bindings: _,
             declarations: _,
 
+            // Ignored; only relevant to definition regions
+            undecorated_type: _,
+
             // Builder only state
             legacy_typevar_binding_context: _,
             deferred_state: _,
@@ -9360,7 +9374,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
                             TypeAndQualifiers::new(Type::unknown(), TypeQualifiers::INIT_VAR)
                         }
                         _ => name_expr_ty
-                            .in_type_expression(self.db(), self.scope())
+                            .in_type_expression(
+                                self.db(),
+                                self.scope(),
+                                self.legacy_typevar_binding_context,
+                            )
                             .unwrap_or_else(|error| {
                                 error.into_fallback_type(
                                     &self.context,
@@ -9579,7 +9597,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
             ast::Expr::Name(name) => match name.ctx {
                 ast::ExprContext::Load => self
                     .infer_name_expression(name)
-                    .in_type_expression(self.db(), self.scope())
+                    .in_type_expression(
+                        self.db(),
+                        self.scope(),
+                        self.legacy_typevar_binding_context,
+                    )
                     .unwrap_or_else(|error| {
                         error.into_fallback_type(
                             &self.context,
@@ -9596,7 +9618,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
             ast::Expr::Attribute(attribute_expression) => match attribute_expression.ctx {
                 ast::ExprContext::Load => self
                     .infer_attribute_expression(attribute_expression)
-                    .in_type_expression(self.db(), self.scope())
+                    .in_type_expression(
+                        self.db(),
+                        self.scope(),
+                        self.legacy_typevar_binding_context,
+                    )
                     .unwrap_or_else(|error| {
                         error.into_fallback_type(
                             &self.context,
@@ -10282,7 +10308,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
                             generic_context,
                         );
                         specialized_class
-                            .in_type_expression(self.db(), self.scope())
+                            .in_type_expression(
+                                self.db(),
+                                self.scope(),
+                                self.legacy_typevar_binding_context,
+                            )
                             .unwrap_or(Type::unknown())
                     }
                     None => {