break the cycle

crates/ty_python_semantic/src/types/function.rs

@@ -73,7 +73,8 @@ use crate::types::diagnostic::{
     report_runtime_check_against_non_runtime_checkable_protocol,
 };
 use crate::types::display::DisplaySettings;
-use crate::types::generics::{GenericContext, InferableTypeVars};
+use crate::types::generics::{GenericContext, InferableTypeVars, typing_self};
+use crate::types::infer::nearest_enclosing_class;
 use crate::types::list_members::all_members;
 use crate::types::narrow::ClassInfoConstraintFunction;
 use crate::types::signatures::{CallableSignature, Signature};
@@ -83,7 +84,7 @@ use crate::types::{
     ClassBase, ClassLiteral, ClassType, DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor,
     HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor, KnownClass, KnownInstanceType,
     NormalizedVisitor, SpecialFormType, Truthiness, Type, TypeContext, TypeMapping, TypeRelation,
-    UnionBuilder, binding_type, definition_expression_type, walk_signature,
+    UnionBuilder, binding_type, definition_expression_type, infer_definition_types, walk_signature,
 };
 use crate::{Db, FxOrderSet, ModuleName, resolve_module};
 
@@ -499,13 +500,66 @@ impl<'db> OverloadLiteral<'db> {
             index,
         );
 
-        Signature::from_function(
+        let mut raw_signature = Signature::from_function(
             db,
             pep695_ctx,
             definition,
             function_stmt_node,
             has_implicitly_positional_first_parameter,
+        );
+
+        let generic_context = raw_signature.generic_context;
+        raw_signature.add_implicit_self_annotation(|| {
+            if self.is_staticmethod(db) || self.is_classmethod(db) {
+                return None;
+            }
+
+            let method_may_be_generic = generic_context
+                .is_some_and(|context| context.variables(db).any(|v| v.typevar(db).is_self(db)));
+
+            let class_scope_id = definition.scope(db);
+            let class_scope = index.scope(class_scope_id.file_scope_id(db));
+            let class_node = class_scope.node().as_class()?;
+            let class_def = index.expect_single_definition(class_node);
+            let (class_literal, class_is_generic) = match infer_definition_types(db, class_def)
+                .declaration_type(class_def)
+                .inner_type()
+            {
+                Type::ClassLiteral(class_literal) => {
+                    (class_literal, class_literal.generic_context(db).is_some())
+                }
+                Type::GenericAlias(alias) => (alias.origin(db), true),
+                _ => return None,
+            };
+
+            if method_may_be_generic
+                || class_is_generic
+                || class_literal
+                    .known(db)
+                    .is_some_and(KnownClass::is_fallback_class)
+            {
+                let scope_id = definition.scope(db);
+                let typevar_binding_context = Some(definition);
+                let index = semantic_index(db, scope_id.file(db));
+                let class = nearest_enclosing_class(db, index, scope_id).unwrap();
+
+                Some(
+                    typing_self(db, scope_id, typevar_binding_context, class)
+                        .map(Type::TypeVar)
+                        .expect(
+                            "We should always find the surrounding class \
+                             for an implicit self: Self annotation",
+                        ),
                 )
+            } else {
+                // For methods of non-generic classes that are not otherwise generic (e.g. return `Self` or
+                // have additional type parameters), the implicit `Self` type of the `self` parameter would
+                // be the only type variable, so we can just use the class directly.
+                Some(class_literal.to_non_generic_instance(db))
+            }
+        });
+
+        raw_signature
     }
 
     pub(crate) fn parameter_span(

crates/ty_python_semantic/src/types/signatures.rs

@@ -13,23 +13,14 @@
 use std::{collections::HashMap, slice::Iter};
 
 use itertools::{EitherOrBoth, Itertools};
-use ruff_db::parsed::parsed_module;
-use ruff_python_ast::ParameterWithDefault;
 use smallvec::{SmallVec, smallvec_inline};
 
-use super::{
+use super::{DynamicType, Type, TypeVarVariance, definition_expression_type};
-    DynamicType, Type, TypeVarVariance, definition_expression_type, infer_definition_types,
+use crate::semantic_index::definition::Definition;
-    semantic_index,
-};
-use crate::semantic_index::definition::{Definition, DefinitionKind};
 use crate::types::constraints::{ConstraintSet, IteratorConstraintsExtension};
-use crate::types::function::{is_implicit_classmethod, is_implicit_staticmethod};
+use crate::types::generics::{GenericContext, InferableTypeVars, walk_generic_context};
-use crate::types::generics::{
-    GenericContext, InferableTypeVars, typing_self, walk_generic_context,
-};
-use crate::types::infer::nearest_enclosing_class;
 use crate::types::{
-    ApplyTypeMappingVisitor, BindingContext, BoundTypeVarInstance, CallableTypeKind, ClassLiteral,
+    ApplyTypeMappingVisitor, BindingContext, BoundTypeVarInstance, CallableTypeKind,
     FindLegacyTypeVarsVisitor, HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor,
     KnownClass, MaterializationKind, NormalizedVisitor, ParamSpecAttrKind, TypeContext,
     TypeMapping, TypeRelation, VarianceInferable, todo_type,
@@ -37,85 +28,6 @@ use crate::types::{
 use crate::{Db, FxOrderSet};
 use ruff_python_ast::{self as ast, name::Name};
 
-#[derive(Clone, Copy, Debug)]
-#[expect(clippy::struct_excessive_bools)]
-struct MethodInformation<'db> {
-    is_staticmethod: bool,
-    is_classmethod: bool,
-    method_may_be_generic: bool,
-    class_literal: ClassLiteral<'db>,
-    class_is_generic: bool,
-}
-
-fn infer_method_information<'db>(
-    db: &'db dyn Db,
-    definition: Definition<'db>,
-) -> Option<MethodInformation<'db>> {
-    let DefinitionKind::Function(function_definition) = definition.kind(db) else {
-        return None;
-    };
-
-    let class_scope_id = definition.scope(db);
-    let file = class_scope_id.file(db);
-    let module = parsed_module(db, file).load(db);
-    let index = semantic_index(db, file);
-
-    let class_scope = index.scope(class_scope_id.file_scope_id(db));
-    let class_node = class_scope.node().as_class()?;
-
-    let function_node = function_definition.node(&module);
-    let function_name = &function_node.name;
-
-    let mut is_staticmethod = is_implicit_classmethod(function_name);
-    let mut is_classmethod = is_implicit_staticmethod(function_name);
-
-    let inference = infer_definition_types(db, definition);
-    for decorator in &function_node.decorator_list {
-        let decorator_ty = inference.expression_type(&decorator.expression);
-
-        match decorator_ty
-            .as_class_literal()
-            .and_then(|class| class.known(db))
-        {
-            Some(KnownClass::Staticmethod) => {
-                is_staticmethod = true;
-            }
-            Some(KnownClass::Classmethod) => {
-                is_classmethod = true;
-            }
-            _ => {}
-        }
-    }
-
-    let method_may_be_generic = match inference.declaration_type(definition).inner_type() {
-        Type::FunctionLiteral(f) => f.signature(db).overloads.iter().any(|s| {
-            s.generic_context
-                .is_some_and(|context| context.variables(db).any(|v| v.typevar(db).is_self(db)))
-        }),
-        _ => true,
-    };
-
-    let class_def = index.expect_single_definition(class_node);
-    let (class_literal, class_is_generic) = match infer_definition_types(db, class_def)
-        .declaration_type(class_def)
-        .inner_type()
-    {
-        Type::ClassLiteral(class_literal) => {
-            (class_literal, class_literal.generic_context(db).is_some())
-        }
-        Type::GenericAlias(alias) => (alias.origin(db), true),
-        _ => return None,
-    };
-
-    Some(MethodInformation {
-        is_staticmethod,
-        is_classmethod,
-        method_may_be_generic,
-        class_literal,
-        class_is_generic,
-    })
-}
-
 /// The signature of a single callable. If the callable is overloaded, there is a separate
 /// [`Signature`] for each overload.
 #[derive(Clone, Debug, PartialEq, Eq, Hash, salsa::Update, get_size2::GetSize)]
@@ -771,6 +683,25 @@ impl<'db> Signature<'db> {
         &self.parameters
     }
 
+    /// Adds an implicit annotation to the first parameter of this signature, if that parameter is
+    /// positional and does not already have an annotation. We do not check whether that's the
+    /// right thing to do! The caller must determine whether the first parameter is actually a
+    /// `self` or `cls` parameter, and must determine the correct type to use as the implicit
+    /// annotation.
+    pub(crate) fn add_implicit_self_annotation(
+        &mut self,
+        self_type: impl FnOnce() -> Option<Type<'db>>,
+    ) {
+        if let Some(first_parameter) = self.parameters.value.first_mut()
+            && first_parameter.is_positional()
+            && first_parameter.annotated_type.is_none()
+            && let Some(self_type) = self_type()
+        {
+            first_parameter.annotated_type = Some(self_type);
+            first_parameter.inferred_annotation = true;
+        }
+    }
+
     /// Return the definition associated with this signature, if any.
     pub(crate) fn definition(&self) -> Option<Definition<'db>> {
         self.definition
@@ -1674,60 +1605,7 @@ impl<'db> Parameters<'db> {
             })
         };
 
-        let method_info = infer_method_information(db, definition);
-        let is_static_or_classmethod =
-            method_info.is_some_and(|f| f.is_staticmethod || f.is_classmethod);
-
-        let inferred_annotation = |arg: &ParameterWithDefault| {
-            if let Some(MethodInformation {
-                method_may_be_generic,
-                class_literal,
-                class_is_generic,
-                ..
-            }) = method_info
-                && !is_static_or_classmethod
-                && arg.parameter.annotation().is_none()
-                && parameters.index(arg.name().id()) == Some(0)
-            {
-                if method_may_be_generic
-                    || class_is_generic
-                    || class_literal
-                        .known(db)
-                        .is_some_and(KnownClass::is_fallback_class)
-                {
-                    let scope_id = definition.scope(db);
-                    let typevar_binding_context = Some(definition);
-                    let index = semantic_index(db, scope_id.file(db));
-                    let class = nearest_enclosing_class(db, index, scope_id).unwrap();
-
-                    Some(
-                        typing_self(db, scope_id, typevar_binding_context, class)
-                            .map(Type::TypeVar)
-                            .expect("We should always find the surrounding class for an implicit self: Self annotation"),
-                    )
-                } else {
-                    // For methods of non-generic classes that are not otherwise generic (e.g. return `Self` or
-                    // have additional type parameters), the implicit `Self` type of the `self` parameter would
-                    // be the only type variable, so we can just use the class directly.
-                    Some(class_literal.to_non_generic_instance(db))
-                }
-            } else {
-                None
-            }
-        };
-
         let pos_only_param = |param: &ast::ParameterWithDefault| {
-            if let Some(inferred_annotation_type) = inferred_annotation(param) {
-                Parameter {
-                    annotated_type: Some(inferred_annotation_type),
-                    inferred_annotation: true,
-                    kind: ParameterKind::PositionalOnly {
-                        name: Some(param.parameter.name.id.clone()),
-                        default_type: default_type(param),
-                    },
-                    form: ParameterForm::Value,
-                }
-            } else {
             Parameter::from_node_and_kind(
                 db,
                 definition,
@@ -1737,7 +1615,6 @@ impl<'db> Parameters<'db> {
                     default_type: default_type(param),
                 },
             )
-            }
         };
 
         let mut positional_only: Vec<Parameter> = posonlyargs.iter().map(pos_only_param).collect();
@@ -1761,17 +1638,6 @@ impl<'db> Parameters<'db> {
         }
 
         let positional_or_keyword = pos_or_keyword_iter.map(|arg| {
-            if let Some(inferred_annotation_type) = inferred_annotation(arg) {
-                Parameter {
-                    annotated_type: Some(inferred_annotation_type),
-                    inferred_annotation: true,
-                    kind: ParameterKind::PositionalOrKeyword {
-                        name: arg.parameter.name.id.clone(),
-                        default_type: default_type(arg),
-                    },
-                    form: ParameterForm::Value,
-                }
-            } else {
             Parameter::from_node_and_kind(
                 db,
                 definition,
@@ -1781,7 +1647,6 @@ impl<'db> Parameters<'db> {
                     default_type: default_type(arg),
                 },
             )
-            }
         });
 
         let variadic = vararg.as_ref().map(|arg| {