Skip to content

Commit b8dec79

Browse files
sharkdpsharkdp
authored
[ty] Implicit instance attributes declared Final (astral-sh#19462)
## Summary Adds proper type inference for implicit instance attributes that are declared with a "bare" `Final` and adds `invalid-assignment` diagnostics for all implicit instance attributes that are declared `Final` or `Final[…]`. ## Test Plan New and updated MD tests. ## Ecosystem analysis ```diff pytest (https://github.com/pytest-dev/pytest) + error[invalid-return-type] src/_pytest/fixtures.py:1662:24: Return type does not match returned value: expected `Scope`, found `Scope | (Unknown & ~None & ~((...) -> object) & ~str) | (((str, Config, /) -> Unknown) & ~((...) -> object) & ~str) | (Unknown & ~str) ``` The definition of the `scope` attribute is [here]( https://github.com/pytest-dev/pytest/blob/5f993856350d1cff144e48810b1c0137c8ec45c5/src/_pytest/fixtures.py#L1020-L1028). Looks like this is a new false positive due to missing `TypeAlias` support that is surfaced here because we now infer a more precise type for `FixtureDef._scope`.
1 parent dc66019 commit b8dec79

File tree

4 files changed

+111
-66
lines changed

4 files changed

+111
-66
lines changed

‎crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ FINAL_A: Final[int] = 1
1919
FINAL_B: Annotated[Final[int], "the annotation for FINAL_B"] = 1
2020
FINAL_C: Final[Annotated[int, "the annotation for FINAL_C"]] = 1
2121
FINAL_D: "Final[int]" = 1
22+
# Note: Some type checkers do not support a separate declaration and
23+
# assignment for `Final` symbols, but it's possible to support this in
24+
# ty, and is useful for code that declares symbols `Final` inside
25+
# `if TYPE_CHECKING` blocks.
2226
FINAL_F: Final[int]
2327
FINAL_F = 1
2428

@@ -87,15 +91,17 @@ class C:
8791
def __init__(self):
8892
self.FINAL_C: Final[int] = 1
8993
self.FINAL_D: Final = 1
94+
self.FINAL_E: Final
95+
self.FINAL_E = 1
9096

9197
reveal_type(C.FINAL_A) # revealed: int
9298
reveal_type(C.FINAL_B) # revealed: Literal[1]
9399

94100
reveal_type(C().FINAL_A) # revealed: int
95101
reveal_type(C().FINAL_B) # revealed: Literal[1]
96102
reveal_type(C().FINAL_C) # revealed: int
97-
# TODO: this should be `Literal[1]`
98-
reveal_type(C().FINAL_D) # revealed: Unknown
103+
reveal_type(C().FINAL_D) # revealed: Literal[1]
104+
reveal_type(C().FINAL_E) # revealed: Literal[1]
99105
```
100106

101107
## Not modifiable
@@ -181,6 +187,8 @@ class C(metaclass=Meta):
181187
def __init__(self):
182188
self.INSTANCE_FINAL_A: Final[int] = 1
183189
self.INSTANCE_FINAL_B: Final = 1
190+
self.INSTANCE_FINAL_C: Final[int]
191+
self.INSTANCE_FINAL_C = 1
184192

185193
# error: [invalid-assignment] "Cannot assign to final attribute `META_FINAL_A` on type `<class 'C'>`"
186194
C.META_FINAL_A = 2
@@ -197,10 +205,12 @@ c = C()
197205
c.CLASS_FINAL_A = 2
198206
# error: [invalid-assignment] "Cannot assign to final attribute `CLASS_FINAL_B` on type `C`"
199207
c.CLASS_FINAL_B = 2
200-
# TODO: this should be an error
208+
# error: [invalid-assignment] "Cannot assign to final attribute `INSTANCE_FINAL_A` on type `C`"
201209
c.INSTANCE_FINAL_A = 2
202-
# TODO: this should be an error
210+
# error: [invalid-assignment] "Cannot assign to final attribute `INSTANCE_FINAL_B` on type `C`"
203211
c.INSTANCE_FINAL_B = 2
212+
# error: [invalid-assignment] "Cannot assign to final attribute `INSTANCE_FINAL_C` on type `C`"
213+
c.INSTANCE_FINAL_C = 2
204214
```
205215

206216
## Mutability

‎crates/ty_python_semantic/src/semantic_index/builder.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,6 +1421,13 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
14211421
self.visit_expr(&node.annotation);
14221422
if let Some(value) = &node.value {
14231423
self.visit_expr(value);
1424+
if self.is_method_of_class().is_some() {
1425+
// Record the right-hand side of the assignment as a standalone expression
1426+
// if we're inside a method. This allows type inference to infer the type
1427+
// of the value for annotated assignments like `self.CONSTANT: Final = 1`,
1428+
// where the type itself is not part of the annotation.
1429+
self.add_standalone_expression(value);
1430+
}
14241431
}
14251432

14261433
if let ast::Expr::Name(name) = &*node.target {

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

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::types::tuple::TupleType;
2323
use crate::types::{
2424
BareTypeAliasType, Binding, BoundSuperError, BoundSuperType, CallableType, DataclassParams,
2525
DeprecatedInstance, DynamicType, KnownInstanceType, TypeAliasType, TypeMapping, TypeRelation,
26-
TypeTransformer, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind,
26+
TypeTransformer, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind, declaration_type,
2727
infer_definition_types,
2828
};
2929
use crate::{
@@ -1477,8 +1477,7 @@ impl<'db> ClassLiteral<'db> {
14771477
return Place::bound(synthesized_member).into();
14781478
}
14791479
// The symbol was not found in the class scope. It might still be implicitly defined in `@classmethod`s.
1480-
return Self::implicit_attribute(db, body_scope, name, MethodDecorator::ClassMethod)
1481-
.into();
1480+
return Self::implicit_attribute(db, body_scope, name, MethodDecorator::ClassMethod);
14821481
}
14831482
symbol
14841483
}
@@ -1824,12 +1823,13 @@ impl<'db> ClassLiteral<'db> {
18241823
class_body_scope: ScopeId<'db>,
18251824
name: &str,
18261825
target_method_decorator: MethodDecorator,
1827-
) -> Place<'db> {
1826+
) -> PlaceAndQualifiers<'db> {
18281827
// If we do not see any declarations of an attribute, neither in the class body nor in
18291828
// any method, we build a union of `Unknown` with the inferred types of all bindings of
18301829
// that attribute. We include `Unknown` in that union to account for the fact that the
18311830
// attribute might be externally modified.
1832-
let mut union_of_inferred_types = UnionBuilder::new(db).add(Type::unknown());
1831+
let mut union_of_inferred_types = UnionBuilder::new(db);
1832+
let mut qualifiers = TypeQualifiers::empty();
18331833

18341834
let mut is_attribute_bound = false;
18351835

@@ -1864,28 +1864,55 @@ impl<'db> ClassLiteral<'db> {
18641864
}
18651865

18661866
for attribute_declaration in attribute_declarations {
1867-
let DefinitionState::Defined(decl) = attribute_declaration.declaration else {
1867+
let DefinitionState::Defined(declaration) = attribute_declaration.declaration
1868+
else {
18681869
continue;
18691870
};
18701871

1871-
let DefinitionKind::AnnotatedAssignment(annotated) = decl.kind(db) else {
1872+
let DefinitionKind::AnnotatedAssignment(assignment) = declaration.kind(db) else {
18721873
continue;
18731874
};
18741875

1876+
// We found an annotated assignment of one of the following forms (using 'self' in these
1877+
// examples, but we support arbitrary names for the first parameters of methods):
1878+
//
1879+
// self.name: <annotation>
1880+
// self.name: <annotation> = …
1881+
18751882
if use_def_map(db, method_scope)
18761883
.is_declaration_reachable(db, &attribute_declaration)
18771884
.is_always_false()
18781885
{
18791886
continue;
18801887
}
18811888

1882-
let annotation_ty =
1883-
infer_expression_type(db, index.expression(annotated.annotation(&module)));
1889+
let annotation = declaration_type(db, declaration);
1890+
let annotation =
1891+
Place::bound(annotation.inner).with_qualifiers(annotation.qualifiers);
1892+
1893+
if let Some(all_qualifiers) = annotation.is_bare_final() {
1894+
if let Some(value) = assignment.value(&module) {
1895+
// If we see an annotated assignment with a bare `Final` as in
1896+
// `self.SOME_CONSTANT: Final = 1`, infer the type from the value
1897+
// on the right-hand side.
18841898

1885-
return Place::bound(annotation_ty);
1899+
let inferred_ty = infer_expression_type(db, index.expression(value));
1900+
return Place::bound(inferred_ty).with_qualifiers(all_qualifiers);
1901+
}
1902+
1903+
// If there is no right-hand side, just record that we saw a `Final` qualifier
1904+
qualifiers |= all_qualifiers;
1905+
continue;
1906+
}
1907+
1908+
return annotation;
18861909
}
18871910
}
18881911

1912+
if !qualifiers.contains(TypeQualifiers::FINAL) {
1913+
union_of_inferred_types = union_of_inferred_types.add(Type::unknown());
1914+
}
1915+
18891916
for (attribute_assignments, method_scope_id) in
18901917
attribute_assignments(db, class_body_scope, name)
18911918
{
@@ -1962,25 +1989,10 @@ impl<'db> ClassLiteral<'db> {
19621989
}
19631990

19641991
match binding.kind(db) {
1965-
DefinitionKind::AnnotatedAssignment(ann_assign) => {
1966-
// We found an annotated assignment of one of the following forms (using 'self' in these
1967-
// examples, but we support arbitrary names for the first parameters of methods):
1968-
//
1969-
// self.name: <annotation>
1970-
// self.name: <annotation> = …
1971-
1972-
let annotation_ty = infer_expression_type(
1973-
db,
1974-
index.expression(ann_assign.annotation(&module)),
1975-
);
1976-
1977-
// TODO: check if there are conflicting declarations
1978-
if is_attribute_bound {
1979-
return Place::bound(annotation_ty);
1980-
}
1981-
unreachable!(
1982-
"If the attribute assignments are all invisible, inference of their types should be skipped"
1983-
);
1992+
DefinitionKind::AnnotatedAssignment(_) => {
1993+
// Annotated assignments were handled above. This branch is not
1994+
// unreachable (because of the `continue` above), but there is
1995+
// nothing to do here.
19841996
}
19851997
DefinitionKind::Assignment(assign) => {
19861998
match assign.target_kind() {
@@ -2110,9 +2122,9 @@ impl<'db> ClassLiteral<'db> {
21102122
}
21112123

21122124
if is_attribute_bound {
2113-
Place::bound(union_of_inferred_types.build())
2125+
Place::bound(union_of_inferred_types.build()).with_qualifiers(qualifiers)
21142126
} else {
2115-
Place::Unbound
2127+
Place::Unbound.with_qualifiers(qualifiers)
21162128
}
21172129
}
21182130

@@ -2158,6 +2170,7 @@ impl<'db> ClassLiteral<'db> {
21582170

21592171
if let Some(implicit_ty) =
21602172
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None)
2173+
.place
21612174
.ignore_possibly_unbound()
21622175
{
21632176
if declaredness == Boundness::Bound {
@@ -2197,6 +2210,7 @@ impl<'db> ClassLiteral<'db> {
21972210
name,
21982211
MethodDecorator::None,
21992212
)
2213+
.place
22002214
.ignore_possibly_unbound()
22012215
{
22022216
Place::Type(
@@ -2218,7 +2232,7 @@ impl<'db> ClassLiteral<'db> {
22182232
// The attribute is not *declared* in the class body. It could still be declared/bound
22192233
// in a method.
22202234

2221-
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None).into()
2235+
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None)
22222236
}
22232237
Err((declared, _conflicting_declarations)) => {
22242238
// There are conflicting declarations for this attribute in the class body.
@@ -2229,7 +2243,7 @@ impl<'db> ClassLiteral<'db> {
22292243
// This attribute is neither declared nor bound in the class body.
22302244
// It could still be implicitly defined in a method.
22312245

2232-
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None).into()
2246+
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None)
22332247
}
22342248
}
22352249

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

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3616,46 +3616,58 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
36163616
ensure_assignable_to(meta_attr_ty)
36173617
};
36183618

3619-
let assignable_to_instance_attribute =
3620-
if meta_attr_boundness == Boundness::PossiblyUnbound {
3621-
let (assignable, boundness) = if let Place::Type(
3622-
instance_attr_ty,
3623-
instance_attr_boundness,
3624-
) =
3625-
object_ty.instance_member(db, attribute).place
3626-
{
3627-
(
3628-
ensure_assignable_to(instance_attr_ty),
3629-
instance_attr_boundness,
3630-
)
3631-
} else {
3632-
(true, Boundness::PossiblyUnbound)
3633-
};
3634-
3635-
if boundness == Boundness::PossiblyUnbound {
3636-
report_possibly_unbound_attribute(
3637-
&self.context,
3638-
target,
3639-
attribute,
3640-
object_ty,
3641-
);
3619+
let assignable_to_instance_attribute = if meta_attr_boundness
3620+
== Boundness::PossiblyUnbound
3621+
{
3622+
let (assignable, boundness) = if let PlaceAndQualifiers {
3623+
place:
3624+
Place::Type(instance_attr_ty, instance_attr_boundness),
3625+
qualifiers,
3626+
} =
3627+
object_ty.instance_member(db, attribute)
3628+
{
3629+
if invalid_assignment_to_final(qualifiers) {
3630+
return false;
36423631
}
36433632

3644-
assignable
3633+
(
3634+
ensure_assignable_to(instance_attr_ty),
3635+
instance_attr_boundness,
3636+
)
36453637
} else {
3646-
true
3638+
(true, Boundness::PossiblyUnbound)
36473639
};
36483640

3641+
if boundness == Boundness::PossiblyUnbound {
3642+
report_possibly_unbound_attribute(
3643+
&self.context,
3644+
target,
3645+
attribute,
3646+
object_ty,
3647+
);
3648+
}
3649+
3650+
assignable
3651+
} else {
3652+
true
3653+
};
3654+
36493655
assignable_to_meta_attr && assignable_to_instance_attribute
36503656
}
36513657

36523658
PlaceAndQualifiers {
36533659
place: Place::Unbound,
36543660
..
36553661
} => {
3656-
if let Place::Type(instance_attr_ty, instance_attr_boundness) =
3657-
object_ty.instance_member(db, attribute).place
3662+
if let PlaceAndQualifiers {
3663+
place: Place::Type(instance_attr_ty, instance_attr_boundness),
3664+
qualifiers,
3665+
} = object_ty.instance_member(db, attribute)
36583666
{
3667+
if invalid_assignment_to_final(qualifiers) {
3668+
return false;
3669+
}
3670+
36593671
if instance_attr_boundness == Boundness::PossiblyUnbound {
36603672
report_possibly_unbound_attribute(
36613673
&self.context,
@@ -3967,7 +3979,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
39673979
} = assignment;
39683980
let annotated =
39693981
self.infer_annotation_expression(annotation, DeferredExpressionState::None);
3970-
self.infer_optional_expression(value.as_deref());
3982+
if let Some(value) = value {
3983+
self.infer_maybe_standalone_expression(value);
3984+
}
39713985

39723986
// If we have an annotated assignment like `self.attr: int = 1`, we still need to
39733987
// do type inference on the `self.attr` target to get types for all sub-expressions.
@@ -4046,7 +4060,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
40464060
debug_assert!(PlaceExpr::try_from(target).is_ok());
40474061

40484062
if let Some(value) = value {
4049-
let inferred_ty = self.infer_expression(value);
4063+
let inferred_ty = self.infer_maybe_standalone_expression(value);
40504064
let inferred_ty = if target
40514065
.as_name_expr()
40524066
.is_some_and(|name| &name.id == "TYPE_CHECKING")

0 commit comments

Comments
 (0)