[ty] Break mandatory salsa cycle when inferring function definitions

crates/ty_python_semantic/resources/mdtest/cycle.md

@@ -87,38 +87,28 @@ class C:
         def inner_a(positional=self.a):
             return
         self.a = inner_a
-        # revealed: def inner_a(positional=Unknown | (def inner_a(positional=Unknown) -> Unknown)) -> Unknown
+        # revealed: Divergent
         reveal_type(inner_a)
 
         def inner_b(*, kw_only=self.b):
             return
         self.b = inner_b
-        # revealed: def inner_b(*, kw_only=Unknown | (def inner_b(*, kw_only=Unknown) -> Unknown)) -> Unknown
+        # revealed: Divergent
         reveal_type(inner_b)
 
         def inner_c(positional_only=self.c, /):
             return
         self.c = inner_c
-        # revealed: def inner_c(positional_only=Unknown | (def inner_c(positional_only=Unknown, /) -> Unknown), /) -> Unknown
+        # revealed: Divergent
         reveal_type(inner_c)
 
         def inner_d(*, kw_only=self.d):
             return
         self.d = inner_d
-        # revealed: def inner_d(*, kw_only=Unknown | (def inner_d(*, kw_only=Unknown) -> Unknown)) -> Unknown
+        # revealed: Divergent
         reveal_type(inner_d)
 ```
 
-We do, however, still check assignability of the default value to the parameter type:
-
-```py
-class D:
-    def f(self: "D"):
-        # error: [invalid-parameter-default] "Default value of type `Unknown | (def inner_a(a: int = Unknown | (def inner_a(a: int = Unknown) -> Unknown)) -> Unknown)` is not assignable to annotated parameter type `int`"
-        def inner_a(a: int = self.a): ...
-        self.a = inner_a
-```
-
 ### Lambdas
 
 ```py
@@ -129,15 +119,15 @@ class C:
         self.c = lambda positional_only=self.c, /: positional_only
         self.d = lambda *, kw_only=self.d: kw_only
 
-        # revealed: (positional=Unknown | ((positional=Unknown) -> Unknown)) -> Unknown
+        # revealed: (positional=Unknown | Divergent) -> Unknown
         reveal_type(self.a)
 
-        # revealed: (*, kw_only=Unknown | ((*, kw_only=Unknown) -> Unknown)) -> Unknown
+        # revealed: (*, kw_only=Unknown | Divergent) -> Unknown
         reveal_type(self.b)
 
-        # revealed: (positional_only=Unknown | ((positional_only=Unknown, /) -> Unknown), /) -> Unknown
+        # revealed: (positional_only=Unknown | Divergent, /) -> Unknown
         reveal_type(self.c)
 
-        # revealed: (*, kw_only=Unknown | ((*, kw_only=Unknown) -> Unknown)) -> Unknown
+        # revealed: (*, kw_only=Unknown | Divergent) -> Unknown
         reveal_type(self.d)
 ```

crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md

@@ -417,16 +417,13 @@ The `converter` function act as a decorator here:
 def f3(x: int, y: str) -> int:
     return 1
 
-# TODO: This should reveal `(x: int, y: str) -> bool` but there's a cycle: https://github.com/astral-sh/ty/issues/1729
-reveal_type(f3)  # revealed: ((x: int, y: str) -> bool) | ((x: Divergent, y: Divergent) -> bool)
+reveal_type(f3)  # revealed: (x: int, y: str) -> bool
 
 reveal_type(f3(1, "a"))  # revealed: bool
 reveal_type(f3(x=1, y="a"))  # revealed: bool
 reveal_type(f3(1, y="a"))  # revealed: bool
 reveal_type(f3(y="a", x=1))  # revealed: bool
 
-# TODO: There should only be one error but the type of `f3` is a union: https://github.com/astral-sh/ty/issues/1729
-# error: [missing-argument] "No argument provided for required parameter `y`"
 # error: [missing-argument] "No argument provided for required parameter `y`"
 f3(1)
 # error: [invalid-argument-type] "Argument is incorrect: Expected `int`, found `Literal["a"]`"

crates/ty_python_semantic/src/semantic_index.rs

@@ -525,6 +525,11 @@ impl<'db> SemanticIndex<'db> {
         self.scopes_by_node[&node.node_key()]
     }
 
+    #[track_caller]
+    pub(crate) fn try_node_scope(&self, node: NodeWithScopeRef) -> Option<FileScopeId> {
+        self.scopes_by_node.get(&node.node_key()).copied()
+    }
+
     /// Checks if there is an import of `__future__.annotations` in the global scope, which affects
     /// the logic for type inference.
     pub(super) fn has_future_annotations(&self) -> bool {

crates/ty_python_semantic/src/types/function.rs

@@ -73,17 +73,18 @@ 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};
+use crate::types::signatures::{CallableSignature, Parameter, Signature};
 use crate::types::visitor::any_over_type;
 use crate::types::{
     ApplyTypeMappingVisitor, BoundMethodType, BoundTypeVarInstance, CallableType, CallableTypeKind,
     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};
 
@@ -195,6 +196,12 @@ pub(crate) fn is_implicit_classmethod(function_name: &str) -> bool {
     matches!(function_name, "__init_subclass__" | "__class_getitem__")
 }
 
+#[derive(Clone, Debug, Eq, Hash, PartialEq, get_size2::GetSize, salsa::Update)]
+pub struct InferredFunctionSignature<'db> {
+    pub(crate) parameters: Vec<Parameter<'db>>,
+    pub(crate) return_type: Option<Type<'db>>,
+}
+
 /// Representation of a function definition in the AST: either a non-generic function, or a generic
 /// function that has not been specialized.
 ///
@@ -227,6 +234,11 @@ pub struct OverloadLiteral<'db> {
     /// The arguments to `dataclass_transformer`, if this function was annotated
     /// with `@dataclass_transformer(...)`.
     pub(crate) dataclass_transformer_params: Option<DataclassTransformerParams<'db>>,
+
+    inferred_signature: Option<InferredFunctionSignature<'db>>,
+
+    #[returns(ref)]
+    inferred_defaults: Vec<Option<Type<'db>>>,
 }
 
 // The Salsa heap is tracked separately.
@@ -247,6 +259,8 @@ impl<'db> OverloadLiteral<'db> {
             self.decorators(db),
             self.deprecated(db),
             Some(params),
+            self.inferred_signature(db),
+            self.inferred_defaults(db),
         )
     }
 
@@ -499,13 +513,68 @@ impl<'db> OverloadLiteral<'db> {
             index,
         );
 
-        Signature::from_function(
+        let mut raw_signature = Signature::from_function(
             db,
             pep695_ctx,
             definition,
             function_stmt_node,
+            self.inferred_signature(db),
+            self.inferred_defaults(db),
             has_implicitly_positional_first_parameter,
-        )
+        );
+
+        let generic_context = raw_signature.generic_context;
+        raw_signature.add_implicit_self_annotation(db, || {
+            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(
@@ -1114,19 +1183,17 @@ impl<'db> FunctionType<'db> {
         nested: bool,
     ) -> Option<Self> {
         let literal = self.literal(db);
-        let updated_signature = match self.updated_signature(db) {
-            Some(signature) => Some(signature.recursive_type_normalized_impl(db, div, nested)?),
-            None => None,
-        };
-        let updated_last_definition_signature = match self.updated_last_definition_signature(db) {
-            Some(signature) => Some(signature.recursive_type_normalized_impl(db, div, nested)?),
-            None => None,
-        };
+        let updated_signature = self
+            .signature(db)
+            .recursive_type_normalized_impl(db, div, nested)?;
+        let updated_last_definition_signature = self
+            .last_definition_signature(db)
+            .recursive_type_normalized_impl(db, div, nested)?;
         Some(Self::new(
             db,
             literal,
-            updated_signature,
-            updated_last_definition_signature,
+            Some(updated_signature),
+            Some(updated_last_definition_signature),
         ))
     }
 }

crates/ty_python_semantic/src/types/infer.rs

@@ -49,7 +49,7 @@ use crate::semantic_index::expression::Expression;
 use crate::semantic_index::scope::ScopeId;
 use crate::semantic_index::{SemanticIndex, semantic_index};
 use crate::types::diagnostic::TypeCheckDiagnostics;
-use crate::types::function::FunctionType;
+use crate::types::function::{FunctionType, InferredFunctionSignature};
 use crate::types::generics::Specialization;
 use crate::types::unpacker::{UnpackResult, Unpacker};
 use crate::types::{
@@ -546,6 +546,9 @@ struct ScopeInferenceExtra<'db> {
 
     /// The diagnostics for this region.
     diagnostics: TypeCheckDiagnostics,
+
+    /// The inferred signature of a function definition (without any default values filled in).
+    signature: Option<InferredFunctionSignature<'db>>,
 }
 
 impl<'db> ScopeInference<'db> {
@@ -605,6 +608,12 @@ impl<'db> ScopeInference<'db> {
 
         extra.string_annotations.contains(&expression.into())
     }
+
+    pub(crate) fn inferred_function_signature(&self) -> Option<InferredFunctionSignature<'db>> {
+        self.extra
+            .as_ref()
+            .and_then(|extra| extra.signature.clone())
+    }
 }
 
 /// The inferred types for a definition region.
@@ -650,6 +659,9 @@ struct DefinitionInferenceExtra<'db> {
 
     /// For function definitions, the undecorated type of the function.
     undecorated_type: Option<Type<'db>>,
+
+    /// The inferred signature of a function definition (without any default values filled in).
+    signature: Option<InferredFunctionSignature<'db>>,
 }
 
 impl<'db> DefinitionInference<'db> {
@@ -778,6 +790,12 @@ impl<'db> DefinitionInference<'db> {
     pub(crate) fn undecorated_type(&self) -> Option<Type<'db>> {
         self.extra.as_ref().and_then(|extra| extra.undecorated_type)
     }
+
+    pub(crate) fn inferred_function_signature(&self) -> Option<InferredFunctionSignature<'db>> {
+        self.extra
+            .as_ref()
+            .and_then(|extra| extra.signature.clone())
+    }
 }
 
 /// The inferred types for an expression region.