From 2dbca6370bb0940a8659b27e499bc6ad9fac7da6 Mon Sep 17 00:00:00 2001 From: David Peter Date: Tue, 21 Oct 2025 19:13:36 +0200 Subject: [PATCH] [ty] Avoid ever-growing default types (#20991) ## Summary We currently panic in the seemingly rare case where the type of a default value of a parameter depends on the callable itself: ```py class C: def f(self: C): self.x = lambda a=self.x: a ``` Types of default values are only used for display reasons, and it's unclear if we even want to track them (or if we should rather track the actual value). So it didn't seem to me that we should spend a lot of effort (and runtime) trying to achieve a theoretically correct type here (which would be infinite). Instead, we simply replace *nested* default types with `Unknown`, i.e. only if the type of the default value is a callable itself. closes https://github.com/astral-sh/ty/issues/1402 ## Test Plan Regression tests --- .../resources/mdtest/cycle.md | 73 +++++++++++++++++++ crates/ty_python_semantic/src/types.rs | 25 ++++++- .../src/types/infer/builder.rs | 9 ++- .../src/types/signatures.rs | 28 ++++--- 4 files changed, 116 insertions(+), 19 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/cycle.md b/crates/ty_python_semantic/resources/mdtest/cycle.md index 0bd3b5b2a6..e52641a05c 100644 --- a/crates/ty_python_semantic/resources/mdtest/cycle.md +++ b/crates/ty_python_semantic/resources/mdtest/cycle.md @@ -31,3 +31,76 @@ p = Point() reveal_type(p.x) # revealed: Unknown | int reveal_type(p.y) # revealed: Unknown | int ``` + +## Parameter default values + +This is a regression test for . When a parameter has a +default value that references the callable itself, we currently prevent infinite recursion by simply +falling back to `Unknown` for the type of the default value, which does not have any practical +impact except for the displayed type. We could also consider inferring `Divergent` when we encounter +too many layers of nesting (instead of just one), but that would require a type traversal which +could have performance implications. So for now, we mainly make sure not to panic or stack overflow +for these seeminly rare cases. + +### Functions + +```py +class C: + def f(self: "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 + 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 + 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 + 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 + 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 +class C: + def f(self: "C"): + self.a = lambda positional=self.a: positional + self.b = lambda *, kw_only=self.b: kw_only + 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 + reveal_type(self.a) + + # revealed: (*, kw_only=Unknown | ((*, kw_only=Unknown) -> Unknown)) -> Unknown + reveal_type(self.b) + + # revealed: (positional_only=Unknown | ((positional_only=Unknown, /) -> Unknown), /) -> Unknown + reveal_type(self.c) + + # revealed: (*, kw_only=Unknown | ((*, kw_only=Unknown) -> Unknown)) -> Unknown + reveal_type(self.d) +``` diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index d9a51ce5b9..aa13a13cd4 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -6678,6 +6678,7 @@ impl<'db> Type<'db> { } } TypeMapping::PromoteLiterals + | TypeMapping::ReplaceParameterDefaults | TypeMapping::BindLegacyTypevars(_) => self, TypeMapping::Materialize(materialization_kind) => { Type::TypeVar(bound_typevar.materialize_impl(db, *materialization_kind, visitor)) @@ -6693,7 +6694,8 @@ impl<'db> Type<'db> { TypeMapping::PromoteLiterals | TypeMapping::BindSelf(_) | TypeMapping::ReplaceSelf { .. } | - TypeMapping::Materialize(_) => self, + TypeMapping::Materialize(_) | + TypeMapping::ReplaceParameterDefaults => self, } Type::FunctionLiteral(function) => { @@ -6807,7 +6809,8 @@ impl<'db> Type<'db> { TypeMapping::BindLegacyTypevars(_) | TypeMapping::BindSelf(_) | TypeMapping::ReplaceSelf { .. } | - TypeMapping::Materialize(_) => self, + TypeMapping::Materialize(_) | + TypeMapping::ReplaceParameterDefaults => self, TypeMapping::PromoteLiterals => self.promote_literals_impl(db, tcx) } @@ -6817,7 +6820,8 @@ impl<'db> Type<'db> { TypeMapping::BindLegacyTypevars(_) | TypeMapping::BindSelf(_) | TypeMapping::ReplaceSelf { .. } | - TypeMapping::PromoteLiterals => self, + TypeMapping::PromoteLiterals | + TypeMapping::ReplaceParameterDefaults => self, TypeMapping::Materialize(materialization_kind) => match materialization_kind { MaterializationKind::Top => Type::object(), MaterializationKind::Bottom => Type::Never, @@ -6993,6 +6997,15 @@ impl<'db> Type<'db> { } } + /// Replace default types in parameters of callables with `Unknown`. + pub(crate) fn replace_parameter_defaults(self, db: &'db dyn Db) -> Type<'db> { + self.apply_type_mapping( + db, + &TypeMapping::ReplaceParameterDefaults, + TypeContext::default(), + ) + } + /// Return the string representation of this type when converted to string as it would be /// provided by the `__str__` method. /// @@ -7369,6 +7382,9 @@ pub enum TypeMapping<'a, 'db> { ReplaceSelf { new_upper_bound: Type<'db> }, /// Create the top or bottom materialization of a type. Materialize(MaterializationKind), + /// Replace default types in parameters of callables with `Unknown`. This is used to avoid infinite + /// recursion when the type of the default value of a parameter depends on the callable itself. + ReplaceParameterDefaults, } impl<'db> TypeMapping<'_, 'db> { @@ -7383,7 +7399,8 @@ impl<'db> TypeMapping<'_, 'db> { | TypeMapping::PartialSpecialization(_) | TypeMapping::PromoteLiterals | TypeMapping::BindLegacyTypevars(_) - | TypeMapping::Materialize(_) => context, + | TypeMapping::Materialize(_) + | TypeMapping::ReplaceParameterDefaults => context, TypeMapping::BindSelf(_) => GenericContext::from_typevar_instances( db, context diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 9c6826e71e..821e650886 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -6508,7 +6508,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let mut parameter = Parameter::positional_only(Some(param.name().id.clone())); if let Some(default) = param.default() { parameter = parameter.with_default_type( - self.infer_expression(default, TypeContext::default()), + self.infer_expression(default, TypeContext::default()) + .replace_parameter_defaults(self.db()), ); } parameter @@ -6521,7 +6522,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let mut parameter = Parameter::positional_or_keyword(param.name().id.clone()); if let Some(default) = param.default() { parameter = parameter.with_default_type( - self.infer_expression(default, TypeContext::default()), + self.infer_expression(default, TypeContext::default()) + .replace_parameter_defaults(self.db()), ); } parameter @@ -6538,7 +6540,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let mut parameter = Parameter::keyword_only(param.name().id.clone()); if let Some(default) = param.default() { parameter = parameter.with_default_type( - self.infer_expression(default, TypeContext::default()), + self.infer_expression(default, TypeContext::default()) + .replace_parameter_defaults(self.db()), ); } parameter diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index a34473dc45..26c3aca6a1 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -1267,9 +1267,9 @@ impl<'db> Parameters<'db> { } = parameters; let default_type = |param: &ast::ParameterWithDefault| { - param - .default() - .map(|default| definition_expression_type(db, definition, default)) + param.default().map(|default| { + definition_expression_type(db, definition, default).replace_parameter_defaults(db) + }) }; let method_info = infer_method_information(db, definition); @@ -1873,23 +1873,27 @@ impl<'db> ParameterKind<'db> { tcx: TypeContext<'db>, visitor: &ApplyTypeMappingVisitor<'db>, ) -> Self { + let apply_to_default_type = |default_type: &Option>| { + if type_mapping == &TypeMapping::ReplaceParameterDefaults && default_type.is_some() { + Some(Type::unknown()) + } else { + default_type + .as_ref() + .map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor)) + } + }; + match self { Self::PositionalOnly { default_type, name } => Self::PositionalOnly { - default_type: default_type - .as_ref() - .map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor)), + default_type: apply_to_default_type(default_type), name: name.clone(), }, Self::PositionalOrKeyword { default_type, name } => Self::PositionalOrKeyword { - default_type: default_type - .as_ref() - .map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor)), + default_type: apply_to_default_type(default_type), name: name.clone(), }, Self::KeywordOnly { default_type, name } => Self::KeywordOnly { - default_type: default_type - .as_ref() - .map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor)), + default_type: apply_to_default_type(default_type), name: name.clone(), }, Self::Variadic { .. } | Self::KeywordVariadic { .. } => self.clone(),