language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-29 11:07:54 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

[ty] Avoid ever-growing default types (#20991)

Browse source 
## 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
This commit is contained in:
David Peter 2025-10-21 19:13:36 +02:00 • committed by GitHub
parent 3dd78e711e
commit 2dbca6370b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 116 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

73
crates/ty_python_semantic/resources/mdtest/cycle.md
View file

@ -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 <https://github.com/astral-sh/ty/issues/1402>. 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)
```

25
crates/ty_python_semantic/src/types.rs
View file

@ -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

9
crates/ty_python_semantic/src/types/infer/builder.rs
View file

@ -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

28
crates/ty_python_semantic/src/types/signatures.rs
View file

@ -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<Type<'db>>| {
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(),