mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-27 02:16:54 +00:00
[ty] Implicit instance attributes declared Final (#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](
5f99385635/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`.
This commit is contained in:
parent
dc66019fbc
commit
b8dec79182
4 changed files with 111 additions and 66 deletions
|
|
@ -19,6 +19,10 @@ FINAL_A: Final[int] = 1
|
||||||
FINAL_B: Annotated[Final[int], "the annotation for FINAL_B"] = 1
|
FINAL_B: Annotated[Final[int], "the annotation for FINAL_B"] = 1
|
||||||
FINAL_C: Final[Annotated[int, "the annotation for FINAL_C"]] = 1
|
FINAL_C: Final[Annotated[int, "the annotation for FINAL_C"]] = 1
|
||||||
FINAL_D: "Final[int]" = 1
|
FINAL_D: "Final[int]" = 1
|
||||||
|
# Note: Some type checkers do not support a separate declaration and
|
||||||
|
# assignment for `Final` symbols, but it's possible to support this in
|
||||||
|
# ty, and is useful for code that declares symbols `Final` inside
|
||||||
|
# `if TYPE_CHECKING` blocks.
|
||||||
FINAL_F: Final[int]
|
FINAL_F: Final[int]
|
||||||
FINAL_F = 1
|
FINAL_F = 1
|
||||||
|
|
||||||
|
|
@ -87,6 +91,8 @@ class C:
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
self.FINAL_C: Final[int] = 1
|
self.FINAL_C: Final[int] = 1
|
||||||
self.FINAL_D: Final = 1
|
self.FINAL_D: Final = 1
|
||||||
|
self.FINAL_E: Final
|
||||||
|
self.FINAL_E = 1
|
||||||
|
|
||||||
reveal_type(C.FINAL_A) # revealed: int
|
reveal_type(C.FINAL_A) # revealed: int
|
||||||
reveal_type(C.FINAL_B) # revealed: Literal[1]
|
reveal_type(C.FINAL_B) # revealed: Literal[1]
|
||||||
|
|
@ -94,8 +100,8 @@ reveal_type(C.FINAL_B) # revealed: Literal[1]
|
||||||
reveal_type(C().FINAL_A) # revealed: int
|
reveal_type(C().FINAL_A) # revealed: int
|
||||||
reveal_type(C().FINAL_B) # revealed: Literal[1]
|
reveal_type(C().FINAL_B) # revealed: Literal[1]
|
||||||
reveal_type(C().FINAL_C) # revealed: int
|
reveal_type(C().FINAL_C) # revealed: int
|
||||||
# TODO: this should be `Literal[1]`
|
reveal_type(C().FINAL_D) # revealed: Literal[1]
|
||||||
reveal_type(C().FINAL_D) # revealed: Unknown
|
reveal_type(C().FINAL_E) # revealed: Literal[1]
|
||||||
```
|
```
|
||||||
|
|
||||||
## Not modifiable
|
## Not modifiable
|
||||||
|
|
@ -181,6 +187,8 @@ class C(metaclass=Meta):
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
self.INSTANCE_FINAL_A: Final[int] = 1
|
self.INSTANCE_FINAL_A: Final[int] = 1
|
||||||
self.INSTANCE_FINAL_B: Final = 1
|
self.INSTANCE_FINAL_B: Final = 1
|
||||||
|
self.INSTANCE_FINAL_C: Final[int]
|
||||||
|
self.INSTANCE_FINAL_C = 1
|
||||||
|
|
||||||
# error: [invalid-assignment] "Cannot assign to final attribute `META_FINAL_A` on type `<class 'C'>`"
|
# error: [invalid-assignment] "Cannot assign to final attribute `META_FINAL_A` on type `<class 'C'>`"
|
||||||
C.META_FINAL_A = 2
|
C.META_FINAL_A = 2
|
||||||
|
|
@ -197,10 +205,12 @@ c = C()
|
||||||
c.CLASS_FINAL_A = 2
|
c.CLASS_FINAL_A = 2
|
||||||
# error: [invalid-assignment] "Cannot assign to final attribute `CLASS_FINAL_B` on type `C`"
|
# error: [invalid-assignment] "Cannot assign to final attribute `CLASS_FINAL_B` on type `C`"
|
||||||
c.CLASS_FINAL_B = 2
|
c.CLASS_FINAL_B = 2
|
||||||
# TODO: this should be an error
|
# error: [invalid-assignment] "Cannot assign to final attribute `INSTANCE_FINAL_A` on type `C`"
|
||||||
c.INSTANCE_FINAL_A = 2
|
c.INSTANCE_FINAL_A = 2
|
||||||
# TODO: this should be an error
|
# error: [invalid-assignment] "Cannot assign to final attribute `INSTANCE_FINAL_B` on type `C`"
|
||||||
c.INSTANCE_FINAL_B = 2
|
c.INSTANCE_FINAL_B = 2
|
||||||
|
# error: [invalid-assignment] "Cannot assign to final attribute `INSTANCE_FINAL_C` on type `C`"
|
||||||
|
c.INSTANCE_FINAL_C = 2
|
||||||
```
|
```
|
||||||
|
|
||||||
## Mutability
|
## Mutability
|
||||||
|
|
|
||||||
|
|
@ -1421,6 +1421,13 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
||||||
self.visit_expr(&node.annotation);
|
self.visit_expr(&node.annotation);
|
||||||
if let Some(value) = &node.value {
|
if let Some(value) = &node.value {
|
||||||
self.visit_expr(value);
|
self.visit_expr(value);
|
||||||
|
if self.is_method_of_class().is_some() {
|
||||||
|
// Record the right-hand side of the assignment as a standalone expression
|
||||||
|
// if we're inside a method. This allows type inference to infer the type
|
||||||
|
// of the value for annotated assignments like `self.CONSTANT: Final = 1`,
|
||||||
|
// where the type itself is not part of the annotation.
|
||||||
|
self.add_standalone_expression(value);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if let ast::Expr::Name(name) = &*node.target {
|
if let ast::Expr::Name(name) = &*node.target {
|
||||||
|
|
|
||||||
|
|
@ -23,7 +23,7 @@ use crate::types::tuple::TupleType;
|
||||||
use crate::types::{
|
use crate::types::{
|
||||||
BareTypeAliasType, Binding, BoundSuperError, BoundSuperType, CallableType, DataclassParams,
|
BareTypeAliasType, Binding, BoundSuperError, BoundSuperType, CallableType, DataclassParams,
|
||||||
DeprecatedInstance, DynamicType, KnownInstanceType, TypeAliasType, TypeMapping, TypeRelation,
|
DeprecatedInstance, DynamicType, KnownInstanceType, TypeAliasType, TypeMapping, TypeRelation,
|
||||||
TypeTransformer, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind,
|
TypeTransformer, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind, declaration_type,
|
||||||
infer_definition_types,
|
infer_definition_types,
|
||||||
};
|
};
|
||||||
use crate::{
|
use crate::{
|
||||||
|
|
@ -1477,8 +1477,7 @@ impl<'db> ClassLiteral<'db> {
|
||||||
return Place::bound(synthesized_member).into();
|
return Place::bound(synthesized_member).into();
|
||||||
}
|
}
|
||||||
// The symbol was not found in the class scope. It might still be implicitly defined in `@classmethod`s.
|
// The symbol was not found in the class scope. It might still be implicitly defined in `@classmethod`s.
|
||||||
return Self::implicit_attribute(db, body_scope, name, MethodDecorator::ClassMethod)
|
return Self::implicit_attribute(db, body_scope, name, MethodDecorator::ClassMethod);
|
||||||
.into();
|
|
||||||
}
|
}
|
||||||
symbol
|
symbol
|
||||||
}
|
}
|
||||||
|
|
@ -1824,12 +1823,13 @@ impl<'db> ClassLiteral<'db> {
|
||||||
class_body_scope: ScopeId<'db>,
|
class_body_scope: ScopeId<'db>,
|
||||||
name: &str,
|
name: &str,
|
||||||
target_method_decorator: MethodDecorator,
|
target_method_decorator: MethodDecorator,
|
||||||
) -> Place<'db> {
|
) -> PlaceAndQualifiers<'db> {
|
||||||
// If we do not see any declarations of an attribute, neither in the class body nor in
|
// If we do not see any declarations of an attribute, neither in the class body nor in
|
||||||
// any method, we build a union of `Unknown` with the inferred types of all bindings of
|
// any method, we build a union of `Unknown` with the inferred types of all bindings of
|
||||||
// that attribute. We include `Unknown` in that union to account for the fact that the
|
// that attribute. We include `Unknown` in that union to account for the fact that the
|
||||||
// attribute might be externally modified.
|
// attribute might be externally modified.
|
||||||
let mut union_of_inferred_types = UnionBuilder::new(db).add(Type::unknown());
|
let mut union_of_inferred_types = UnionBuilder::new(db);
|
||||||
|
let mut qualifiers = TypeQualifiers::empty();
|
||||||
|
|
||||||
let mut is_attribute_bound = false;
|
let mut is_attribute_bound = false;
|
||||||
|
|
||||||
|
|
@ -1864,14 +1864,21 @@ impl<'db> ClassLiteral<'db> {
|
||||||
}
|
}
|
||||||
|
|
||||||
for attribute_declaration in attribute_declarations {
|
for attribute_declaration in attribute_declarations {
|
||||||
let DefinitionState::Defined(decl) = attribute_declaration.declaration else {
|
let DefinitionState::Defined(declaration) = attribute_declaration.declaration
|
||||||
|
else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
|
|
||||||
let DefinitionKind::AnnotatedAssignment(annotated) = decl.kind(db) else {
|
let DefinitionKind::AnnotatedAssignment(assignment) = declaration.kind(db) else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// We found an annotated assignment of one of the following forms (using 'self' in these
|
||||||
|
// examples, but we support arbitrary names for the first parameters of methods):
|
||||||
|
//
|
||||||
|
// self.name: <annotation>
|
||||||
|
// self.name: <annotation> = …
|
||||||
|
|
||||||
if use_def_map(db, method_scope)
|
if use_def_map(db, method_scope)
|
||||||
.is_declaration_reachable(db, &attribute_declaration)
|
.is_declaration_reachable(db, &attribute_declaration)
|
||||||
.is_always_false()
|
.is_always_false()
|
||||||
|
|
@ -1879,11 +1886,31 @@ impl<'db> ClassLiteral<'db> {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
let annotation_ty =
|
let annotation = declaration_type(db, declaration);
|
||||||
infer_expression_type(db, index.expression(annotated.annotation(&module)));
|
let annotation =
|
||||||
|
Place::bound(annotation.inner).with_qualifiers(annotation.qualifiers);
|
||||||
|
|
||||||
return Place::bound(annotation_ty);
|
if let Some(all_qualifiers) = annotation.is_bare_final() {
|
||||||
|
if let Some(value) = assignment.value(&module) {
|
||||||
|
// If we see an annotated assignment with a bare `Final` as in
|
||||||
|
// `self.SOME_CONSTANT: Final = 1`, infer the type from the value
|
||||||
|
// on the right-hand side.
|
||||||
|
|
||||||
|
let inferred_ty = infer_expression_type(db, index.expression(value));
|
||||||
|
return Place::bound(inferred_ty).with_qualifiers(all_qualifiers);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If there is no right-hand side, just record that we saw a `Final` qualifier
|
||||||
|
qualifiers |= all_qualifiers;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
return annotation;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if !qualifiers.contains(TypeQualifiers::FINAL) {
|
||||||
|
union_of_inferred_types = union_of_inferred_types.add(Type::unknown());
|
||||||
}
|
}
|
||||||
|
|
||||||
for (attribute_assignments, method_scope_id) in
|
for (attribute_assignments, method_scope_id) in
|
||||||
|
|
@ -1962,25 +1989,10 @@ impl<'db> ClassLiteral<'db> {
|
||||||
}
|
}
|
||||||
|
|
||||||
match binding.kind(db) {
|
match binding.kind(db) {
|
||||||
DefinitionKind::AnnotatedAssignment(ann_assign) => {
|
DefinitionKind::AnnotatedAssignment(_) => {
|
||||||
// We found an annotated assignment of one of the following forms (using 'self' in these
|
// Annotated assignments were handled above. This branch is not
|
||||||
// examples, but we support arbitrary names for the first parameters of methods):
|
// unreachable (because of the `continue` above), but there is
|
||||||
//
|
// nothing to do here.
|
||||||
// self.name: <annotation>
|
|
||||||
// self.name: <annotation> = …
|
|
||||||
|
|
||||||
let annotation_ty = infer_expression_type(
|
|
||||||
db,
|
|
||||||
index.expression(ann_assign.annotation(&module)),
|
|
||||||
);
|
|
||||||
|
|
||||||
// TODO: check if there are conflicting declarations
|
|
||||||
if is_attribute_bound {
|
|
||||||
return Place::bound(annotation_ty);
|
|
||||||
}
|
|
||||||
unreachable!(
|
|
||||||
"If the attribute assignments are all invisible, inference of their types should be skipped"
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
DefinitionKind::Assignment(assign) => {
|
DefinitionKind::Assignment(assign) => {
|
||||||
match assign.target_kind() {
|
match assign.target_kind() {
|
||||||
|
|
@ -2110,9 +2122,9 @@ impl<'db> ClassLiteral<'db> {
|
||||||
}
|
}
|
||||||
|
|
||||||
if is_attribute_bound {
|
if is_attribute_bound {
|
||||||
Place::bound(union_of_inferred_types.build())
|
Place::bound(union_of_inferred_types.build()).with_qualifiers(qualifiers)
|
||||||
} else {
|
} else {
|
||||||
Place::Unbound
|
Place::Unbound.with_qualifiers(qualifiers)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -2158,6 +2170,7 @@ impl<'db> ClassLiteral<'db> {
|
||||||
|
|
||||||
if let Some(implicit_ty) =
|
if let Some(implicit_ty) =
|
||||||
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None)
|
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None)
|
||||||
|
.place
|
||||||
.ignore_possibly_unbound()
|
.ignore_possibly_unbound()
|
||||||
{
|
{
|
||||||
if declaredness == Boundness::Bound {
|
if declaredness == Boundness::Bound {
|
||||||
|
|
@ -2197,6 +2210,7 @@ impl<'db> ClassLiteral<'db> {
|
||||||
name,
|
name,
|
||||||
MethodDecorator::None,
|
MethodDecorator::None,
|
||||||
)
|
)
|
||||||
|
.place
|
||||||
.ignore_possibly_unbound()
|
.ignore_possibly_unbound()
|
||||||
{
|
{
|
||||||
Place::Type(
|
Place::Type(
|
||||||
|
|
@ -2218,7 +2232,7 @@ impl<'db> ClassLiteral<'db> {
|
||||||
// The attribute is not *declared* in the class body. It could still be declared/bound
|
// The attribute is not *declared* in the class body. It could still be declared/bound
|
||||||
// in a method.
|
// in a method.
|
||||||
|
|
||||||
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None).into()
|
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None)
|
||||||
}
|
}
|
||||||
Err((declared, _conflicting_declarations)) => {
|
Err((declared, _conflicting_declarations)) => {
|
||||||
// There are conflicting declarations for this attribute in the class body.
|
// There are conflicting declarations for this attribute in the class body.
|
||||||
|
|
@ -2229,7 +2243,7 @@ impl<'db> ClassLiteral<'db> {
|
||||||
// This attribute is neither declared nor bound in the class body.
|
// This attribute is neither declared nor bound in the class body.
|
||||||
// It could still be implicitly defined in a method.
|
// It could still be implicitly defined in a method.
|
||||||
|
|
||||||
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None).into()
|
Self::implicit_attribute(db, body_scope, name, MethodDecorator::None)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -3616,14 +3616,20 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
ensure_assignable_to(meta_attr_ty)
|
ensure_assignable_to(meta_attr_ty)
|
||||||
};
|
};
|
||||||
|
|
||||||
let assignable_to_instance_attribute =
|
let assignable_to_instance_attribute = if meta_attr_boundness
|
||||||
if meta_attr_boundness == Boundness::PossiblyUnbound {
|
== Boundness::PossiblyUnbound
|
||||||
let (assignable, boundness) = if let Place::Type(
|
|
||||||
instance_attr_ty,
|
|
||||||
instance_attr_boundness,
|
|
||||||
) =
|
|
||||||
object_ty.instance_member(db, attribute).place
|
|
||||||
{
|
{
|
||||||
|
let (assignable, boundness) = if let PlaceAndQualifiers {
|
||||||
|
place:
|
||||||
|
Place::Type(instance_attr_ty, instance_attr_boundness),
|
||||||
|
qualifiers,
|
||||||
|
} =
|
||||||
|
object_ty.instance_member(db, attribute)
|
||||||
|
{
|
||||||
|
if invalid_assignment_to_final(qualifiers) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
(
|
(
|
||||||
ensure_assignable_to(instance_attr_ty),
|
ensure_assignable_to(instance_attr_ty),
|
||||||
instance_attr_boundness,
|
instance_attr_boundness,
|
||||||
|
|
@ -3653,9 +3659,15 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
place: Place::Unbound,
|
place: Place::Unbound,
|
||||||
..
|
..
|
||||||
} => {
|
} => {
|
||||||
if let Place::Type(instance_attr_ty, instance_attr_boundness) =
|
if let PlaceAndQualifiers {
|
||||||
object_ty.instance_member(db, attribute).place
|
place: Place::Type(instance_attr_ty, instance_attr_boundness),
|
||||||
|
qualifiers,
|
||||||
|
} = object_ty.instance_member(db, attribute)
|
||||||
{
|
{
|
||||||
|
if invalid_assignment_to_final(qualifiers) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
if instance_attr_boundness == Boundness::PossiblyUnbound {
|
if instance_attr_boundness == Boundness::PossiblyUnbound {
|
||||||
report_possibly_unbound_attribute(
|
report_possibly_unbound_attribute(
|
||||||
&self.context,
|
&self.context,
|
||||||
|
|
@ -3967,7 +3979,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
} = assignment;
|
} = assignment;
|
||||||
let annotated =
|
let annotated =
|
||||||
self.infer_annotation_expression(annotation, DeferredExpressionState::None);
|
self.infer_annotation_expression(annotation, DeferredExpressionState::None);
|
||||||
self.infer_optional_expression(value.as_deref());
|
if let Some(value) = value {
|
||||||
|
self.infer_maybe_standalone_expression(value);
|
||||||
|
}
|
||||||
|
|
||||||
// If we have an annotated assignment like `self.attr: int = 1`, we still need to
|
// If we have an annotated assignment like `self.attr: int = 1`, we still need to
|
||||||
// do type inference on the `self.attr` target to get types for all sub-expressions.
|
// 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> {
|
||||||
debug_assert!(PlaceExpr::try_from(target).is_ok());
|
debug_assert!(PlaceExpr::try_from(target).is_ok());
|
||||||
|
|
||||||
if let Some(value) = value {
|
if let Some(value) = value {
|
||||||
let inferred_ty = self.infer_expression(value);
|
let inferred_ty = self.infer_maybe_standalone_expression(value);
|
||||||
let inferred_ty = if target
|
let inferred_ty = if target
|
||||||
.as_name_expr()
|
.as_name_expr()
|
||||||
.is_some_and(|name| &name.id == "TYPE_CHECKING")
|
.is_some_and(|name| &name.id == "TYPE_CHECKING")
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue