Skip to content

Commit e81e35d

Browse files
sharkdpsharkdp
committed
[ty] Avoid ever-growing default types
1 parent c2ae9c7 commit e81e35d

File tree

4 files changed

+106
-19
lines changed

4 files changed

+106
-19
lines changed

‎crates/ty_python_semantic/resources/mdtest/cycle.md‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,66 @@ p = Point()
3131
reveal_type(p.x) # revealed: Unknown | int
3232
reveal_type(p.y) # revealed: Unknown | int
3333
```
34+
35+
## Parameter default values
36+
37+
This is a regression test for <https://github.com/astral-sh/ty/issues/1402>. When a parameter has a
38+
default value that references the callable itself, we currently prevent infinite recursion by simply
39+
falling back to `Unknown` for the type of the default value, which does not have any practical
40+
impact except for the displayed type. We could also consider inferring `Divergent` when we encounter
41+
too many layers of nesting (instead of just one), but that would require a type traversal which
42+
could have performance implications. So for now, we mainly make sure not to panic or stack overflow
43+
for these seeminly rare cases.
44+
45+
### Functions
46+
47+
```py
48+
class C:
49+
def f(self: "C"):
50+
def inner_a(positional=self.a):
51+
return
52+
self.a = inner_a
53+
# revealed: def inner_a(positional=Unknown | (def inner_a(positional=Unknown) -> Unknown)) -> Unknown
54+
reveal_type(inner_a)
55+
56+
def inner_b(*, kw_only=self.b):
57+
return
58+
self.b = inner_b
59+
# revealed: def inner_b(*, kw_only=Unknown | (def inner_b(*, kw_only=Unknown) -> Unknown)) -> Unknown
60+
reveal_type(inner_b)
61+
62+
def inner_c(positional_only=self.c, /):
63+
return
64+
self.c = inner_c
65+
# revealed: def inner_c(positional_only=Unknown | (def inner_c(positional_only=Unknown, /) -> Unknown), /) -> Unknown
66+
reveal_type(inner_c)
67+
68+
def inner_d(*, kw_only=self.d):
69+
return
70+
self.d = inner_d
71+
# revealed: def inner_d(*, kw_only=Unknown | (def inner_d(*, kw_only=Unknown) -> Unknown)) -> Unknown
72+
reveal_type(inner_d)
73+
```
74+
75+
### Lambdas
76+
77+
```py
78+
class C:
79+
def f(self: "C"):
80+
self.a = lambda positional=self.a: positional
81+
self.b = lambda *, kw_only=self.b: kw_only
82+
self.c = lambda positional_only=self.c, /: positional_only
83+
self.d = lambda *, kw_only=self.d: kw_only
84+
85+
# revealed: (positional=Unknown | ((positional=Unknown) -> Unknown)) -> Unknown
86+
reveal_type(self.a)
87+
88+
# revealed: (*, kw_only=Unknown | ((*, kw_only=Unknown) -> Unknown)) -> Unknown
89+
reveal_type(self.b)
90+
91+
# revealed: (positional_only=Unknown | ((positional_only=Unknown, /) -> Unknown), /) -> Unknown
92+
reveal_type(self.c)
93+
94+
# revealed: (*, kw_only=Unknown | ((*, kw_only=Unknown) -> Unknown)) -> Unknown
95+
reveal_type(self.d)
96+
```

‎crates/ty_python_semantic/src/types.rs‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6677,6 +6677,7 @@ impl<'db> Type<'db> {
66776677
}
66786678
}
66796679
TypeMapping::PromoteLiterals
6680+
| TypeMapping::ReplaceParameterDefaults
66806681
| TypeMapping::BindLegacyTypevars(_) => self,
66816682
TypeMapping::Materialize(materialization_kind) => {
66826683
Type::TypeVar(bound_typevar.materialize_impl(db, *materialization_kind, visitor))
@@ -6692,7 +6693,8 @@ impl<'db> Type<'db> {
66926693
TypeMapping::PromoteLiterals |
66936694
TypeMapping::BindSelf(_) |
66946695
TypeMapping::ReplaceSelf { .. } |
6695-
TypeMapping::Materialize(_) => self,
6696+
TypeMapping::Materialize(_) |
6697+
TypeMapping::ReplaceParameterDefaults => self,
66966698
}
66976699

66986700
Type::FunctionLiteral(function) => {
@@ -6806,7 +6808,8 @@ impl<'db> Type<'db> {
68066808
TypeMapping::BindLegacyTypevars(_) |
68076809
TypeMapping::BindSelf(_) |
68086810
TypeMapping::ReplaceSelf { .. } |
6809-
TypeMapping::Materialize(_) => self,
6811+
TypeMapping::Materialize(_) |
6812+
TypeMapping::ReplaceParameterDefaults => self,
68106813
TypeMapping::PromoteLiterals => self.promote_literals_impl(db, tcx)
68116814
}
68126815

@@ -6816,7 +6819,8 @@ impl<'db> Type<'db> {
68166819
TypeMapping::BindLegacyTypevars(_) |
68176820
TypeMapping::BindSelf(_) |
68186821
TypeMapping::ReplaceSelf { .. } |
6819-
TypeMapping::PromoteLiterals => self,
6822+
TypeMapping::PromoteLiterals |
6823+
TypeMapping::ReplaceParameterDefaults => self,
68206824
TypeMapping::Materialize(materialization_kind) => match materialization_kind {
68216825
MaterializationKind::Top => Type::object(),
68226826
MaterializationKind::Bottom => Type::Never,
@@ -6992,6 +6996,15 @@ impl<'db> Type<'db> {
69926996
}
69936997
}
69946998

6999+
/// Replace default types in parameters of callables with `Unknown`.
7000+
pub(crate) fn replace_parameter_defaults(self, db: &'db dyn Db) -> Type<'db> {
7001+
self.apply_type_mapping(
7002+
db,
7003+
&TypeMapping::ReplaceParameterDefaults,
7004+
TypeContext::default(),
7005+
)
7006+
}
7007+
69957008
/// Return the string representation of this type when converted to string as it would be
69967009
/// provided by the `__str__` method.
69977010
///
@@ -7368,6 +7381,9 @@ pub enum TypeMapping<'a, 'db> {
73687381
ReplaceSelf { new_upper_bound: Type<'db> },
73697382
/// Create the top or bottom materialization of a type.
73707383
Materialize(MaterializationKind),
7384+
/// Replace default types in parameters of callables with `Unknown`. This is used to avoid infinite
7385+
/// recursion when the type of the default value of a parameter depends on the callable itself.
7386+
ReplaceParameterDefaults,
73717387
}
73727388

73737389
impl<'db> TypeMapping<'_, 'db> {
@@ -7382,7 +7398,8 @@ impl<'db> TypeMapping<'_, 'db> {
73827398
| TypeMapping::PartialSpecialization(_)
73837399
| TypeMapping::PromoteLiterals
73847400
| TypeMapping::BindLegacyTypevars(_)
7385-
| TypeMapping::Materialize(_) => context,
7401+
| TypeMapping::Materialize(_)
7402+
| TypeMapping::ReplaceParameterDefaults => context,
73867403
TypeMapping::BindSelf(_) => GenericContext::from_typevar_instances(
73877404
db,
73887405
context

‎crates/ty_python_semantic/src/types/infer/builder.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6508,7 +6508,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
65086508
let mut parameter = Parameter::positional_only(Some(param.name().id.clone()));
65096509
if let Some(default) = param.default() {
65106510
parameter = parameter.with_default_type(
6511-
self.infer_expression(default, TypeContext::default()),
6511+
self.infer_expression(default, TypeContext::default())
6512+
.replace_parameter_defaults(self.db()),
65126513
);
65136514
}
65146515
parameter
@@ -6521,7 +6522,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
65216522
let mut parameter = Parameter::positional_or_keyword(param.name().id.clone());
65226523
if let Some(default) = param.default() {
65236524
parameter = parameter.with_default_type(
6524-
self.infer_expression(default, TypeContext::default()),
6525+
self.infer_expression(default, TypeContext::default())
6526+
.replace_parameter_defaults(self.db()),
65256527
);
65266528
}
65276529
parameter
@@ -6538,7 +6540,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
65386540
let mut parameter = Parameter::keyword_only(param.name().id.clone());
65396541
if let Some(default) = param.default() {
65406542
parameter = parameter.with_default_type(
6541-
self.infer_expression(default, TypeContext::default()),
6543+
self.infer_expression(default, TypeContext::default())
6544+
.replace_parameter_defaults(self.db()),
65426545
);
65436546
}
65446547
parameter

‎crates/ty_python_semantic/src/types/signatures.rs‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,9 +1267,9 @@ impl<'db> Parameters<'db> {
12671267
} = parameters;
12681268

12691269
let default_type = |param: &ast::ParameterWithDefault| {
1270-
param
1271-
.default()
1272-
.map(|default| definition_expression_type(db, definition, default))
1270+
param.default().map(|default| {
1271+
definition_expression_type(db, definition, default).replace_parameter_defaults(db)
1272+
})
12731273
};
12741274

12751275
let method_info = infer_method_information(db, definition);
@@ -1873,23 +1873,27 @@ impl<'db> ParameterKind<'db> {
18731873
tcx: TypeContext<'db>,
18741874
visitor: &ApplyTypeMappingVisitor<'db>,
18751875
) -> Self {
1876+
let apply_to_default_type = |default_type: &Option<Type<'db>>| {
1877+
if type_mapping == &TypeMapping::ReplaceParameterDefaults && default_type.is_some() {
1878+
Some(Type::unknown())
1879+
} else {
1880+
default_type
1881+
.as_ref()
1882+
.map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor))
1883+
}
1884+
};
1885+
18761886
match self {
18771887
Self::PositionalOnly { default_type, name } => Self::PositionalOnly {
1878-
default_type: default_type
1879-
.as_ref()
1880-
.map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor)),
1888+
default_type: apply_to_default_type(default_type),
18811889
name: name.clone(),
18821890
},
18831891
Self::PositionalOrKeyword { default_type, name } => Self::PositionalOrKeyword {
1884-
default_type: default_type
1885-
.as_ref()
1886-
.map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor)),
1892+
default_type: apply_to_default_type(default_type),
18871893
name: name.clone(),
18881894
},
18891895
Self::KeywordOnly { default_type, name } => Self::KeywordOnly {
1890-
default_type: default_type
1891-
.as_ref()
1892-
.map(|ty| ty.apply_type_mapping_impl(db, type_mapping, tcx, visitor)),
1896+
default_type: apply_to_default_type(default_type),
18931897
name: name.clone(),
18941898
},
18951899
Self::Variadic { .. } | Self::KeywordVariadic { .. } => self.clone(),

0 commit comments

Comments
 (0)