mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-02 06:41:23 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[red-knot] Handle possibly-unbound instance members (#16363)

Browse source 
## Summary

Adds support for possibly-unbound/undeclared instance members.

## Test Plan

New MD tests.
This commit is contained in:
David Peter 2025-02-25 20:00:38 +01:00 • committed by GitHub
parent fa76f6cbb2
commit f88328eedd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 133 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

62
crates/red_knot_python_semantic/resources/mdtest/attributes.md
View file

@ -783,6 +783,9 @@ def _(flag1: bool, flag2: bool):
# error: [possibly-unbound-attribute] "Attribute `x` on type `Literal[C1, C2, C3]` is possibly unbound"
reveal_type(C.x) # revealed: Unknown | Literal[1, 3]
# error: [possibly-unbound-attribute] "Attribute `x` on type `C1 | C2 | C3` is possibly unbound"
reveal_type(C().x) # revealed: Unknown | Literal[1, 3]
```
### Possibly-unbound within a class
@ -806,6 +809,28 @@ def _(flag: bool, flag1: bool, flag2: bool):
# error: [possibly-unbound-attribute] "Attribute `x` on type `Literal[C1, C2, C3]` is possibly unbound"
reveal_type(C.x) # revealed: Unknown | Literal[1, 2, 3]
# Note: we might want to consider ignoring possibly-unbound diagnostics for instance attributes eventually,
# see the "Possibly unbound/undeclared instance attribute" section below.
# error: [possibly-unbound-attribute] "Attribute `x` on type `C1 | C2 | C3` is possibly unbound"
reveal_type(C().x) # revealed: Unknown | Literal[1, 2, 3]
```
### Possibly-unbound within gradual types
```py
from typing import Any
def _(flag: bool):
class Base:
x: Any
class Derived(Base):
if flag:
# Redeclaring `x` with a more static type is okay in terms of LSP.
x: int
reveal_type(Derived().x) # revealed: int | Any
```
### Attribute possibly unbound on a subclass but not on a superclass
@ -820,6 +845,8 @@ def _(flag: bool):
x = 2
reveal_type(Bar.x) # revealed: Unknown | Literal[2, 1]
reveal_type(Bar().x) # revealed: Unknown | Literal[2, 1]
```
### Attribute possibly unbound on a subclass and on a superclass
@ -836,6 +863,41 @@ def _(flag: bool):
# error: [possibly-unbound-attribute]
reveal_type(Bar.x) # revealed: Unknown | Literal[2, 1]
# error: [possibly-unbound-attribute]
reveal_type(Bar().x) # revealed: Unknown | Literal[2, 1]
```
### Possibly unbound/undeclared instance attribute
#### Possibly unbound and undeclared
```py
def _(flag: bool):
class Foo:
if flag:
x: int
def __init(self):
if flag:
self.x = 1
# error: [possibly-unbound-attribute]
reveal_type(Foo().x) # revealed: int
```
#### Possibly unbound
```py
def _(flag: bool):
class Foo:
def __init(self):
if flag:
self.x = 1
# Emitting a diagnostic in a case like this is not something we support, and it's unclear
# if we ever will (or want to)
reveal_type(Foo().x) # revealed: Unknown | Literal[1]
```
### Attribute access on `Any`

18
crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md
View file

@ -64,6 +64,24 @@ c = C()
c.a = 2
```
and similarly here:
```py
class Base:
a: ClassVar[int] = 1
class Derived(Base):
if flag():
a: int
reveal_type(Derived.a) # revealed: int
d = Derived()
# error: [invalid-attribute-access]
d.a = 2
```
## Too many arguments
```py

4
crates/red_knot_python_semantic/src/types/builder.rs
View file

@ -43,6 +43,10 @@ impl<'db> UnionBuilder<'db> {
}
}
pub(crate) fn is_empty(&self) -> bool {
self.elements.is_empty()
}
/// Collapse the union to a single type: `object`.
fn collapse_to_object(mut self) -> Self {
self.elements.clear();

68
crates/red_knot_python_semantic/src/types/class.rs
View file

@ -6,7 +6,7 @@ use crate::{
},
symbol::{
class_symbol, known_module_symbol, symbol_from_bindings, symbol_from_declarations,
LookupError, LookupResult, Symbol, SymbolAndQualifiers,
Boundness, LookupError, LookupResult, Symbol, SymbolAndQualifiers,
},
types::{
definition_expression_type, CallArguments, CallError, MetaclassCandidate, TupleType,
@ -383,6 +383,9 @@ impl<'db> Class<'db> {
///
/// The attribute might also be defined in a superclass of this class.
pub(super) fn instance_member(self, db: &'db dyn Db, name: &str) -> SymbolAndQualifiers<'db> {
let mut union = UnionBuilder::new(db);
let mut union_qualifiers = TypeQualifiers::empty();
for superclass in self.iter_mro(db) {
match superclass {
ClassBase::Dynamic(_) => {
@ -391,16 +394,43 @@ impl<'db> Class<'db> {
);
}
ClassBase::Class(class) => {
if let member @ SymbolAndQualifiers(Symbol::Type(_, _), _) =
if let member @ SymbolAndQualifiers(Symbol::Type(ty, boundness), qualifiers) =
class.own_instance_member(db, name)
{
return member;
// TODO: We could raise a diagnostic here if there are conflicting type qualifiers
union_qualifiers = union_qualifiers.union(qualifiers);
if boundness == Boundness::Bound {
if union.is_empty() {
// Short-circuit, no need to allocate inside the union builder
return member;
}
return SymbolAndQualifiers(
Symbol::bound(union.add(ty).build()),
union_qualifiers,
);
}
// If we see a possibly-unbound symbol, we need to keep looking
// higher up in the MRO.
union = union.add(ty);
}
}
}
}
SymbolAndQualifiers(Symbol::Unbound, TypeQualifiers::empty())
if union.is_empty() {
SymbolAndQualifiers(Symbol::Unbound, TypeQualifiers::empty())
} else {
// If we have reached this point, we know that we have only seen possibly-unbound symbols.
// This means that the final result is still possibly-unbound.
SymbolAndQualifiers(
Symbol::Type(union.build(), Boundness::PossiblyUnbound),
union_qualifiers,
)
}
}
/// Tries to find declarations/bindings of an instance attribute named `name` that are only
@ -409,16 +439,18 @@ impl<'db> Class<'db> {
db: &'db dyn Db,
class_body_scope: ScopeId<'db>,
name: &str,
inferred_type_from_class_body: Option<Type<'db>>,
inferred_from_class_body: &Symbol<'db>,
) -> Symbol<'db> {
// 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
// that attribute. We include `Unknown` in that union to account for the fact that the
// attribute might be externally modified.
let mut union_of_inferred_types = UnionBuilder::new(db).add(Type::unknown());
let mut union_boundness = Boundness::Bound;
if let Some(ty) = inferred_type_from_class_body {
union_of_inferred_types = union_of_inferred_types.add(ty);
if let Symbol::Type(ty, boundness) = inferred_from_class_body {
union_of_inferred_types = union_of_inferred_types.add(*ty);
union_boundness = *boundness;
}
let attribute_assignments = attribute_assignments(db, class_body_scope);
@ -427,10 +459,10 @@ impl<'db> Class<'db> {
.as_deref()
.and_then(|assignments| assignments.get(name))
else {
if inferred_type_from_class_body.is_some() {
return Symbol::bound(union_of_inferred_types.build());
if inferred_from_class_body.is_unbound() {
return Symbol::Unbound;
}
return Symbol::Unbound;
return Symbol::Type(union_of_inferred_types.build(), union_boundness);
};
for attribute_assignment in attribute_assignments {
@ -484,7 +516,7 @@ impl<'db> Class<'db> {
}
}
Symbol::bound(union_of_inferred_types.build())
Symbol::Type(union_of_inferred_types.build(), union_boundness)
}
/// A helper function for `instance_member` that looks up the `name` attribute only on
@ -493,7 +525,6 @@ impl<'db> Class<'db> {
// TODO: There are many things that are not yet implemented here:
// - `typing.Final`
// - Proper diagnostics
// - Handling of possibly-undeclared/possibly-unbound attributes
let body_scope = self.body_scope(db);
let table = symbol_table(db, body_scope);
@ -504,7 +535,7 @@ impl<'db> Class<'db> {
let declarations = use_def.public_declarations(symbol_id);
match symbol_from_declarations(db, declarations) {
Ok(SymbolAndQualifiers(Symbol::Type(declared_ty, _), qualifiers)) => {
Ok(SymbolAndQualifiers(declared @ Symbol::Type(declared_ty, _), qualifiers)) => {
// The attribute is declared in the class body.
if let Some(function) = declared_ty.into_function_literal() {
@ -514,7 +545,7 @@ impl<'db> Class<'db> {
if function.has_known_class_decorator(db, KnownClass::Classmethod)
&& function.decorators(db).len() == 1
{
SymbolAndQualifiers(Symbol::bound(declared_ty), qualifiers)
SymbolAndQualifiers(declared, qualifiers)
} else if function.has_known_class_decorator(db, KnownClass::Property) {
SymbolAndQualifiers::todo("@property")
} else if function.has_known_function_decorator(db, KnownFunction::Overload)
@ -523,10 +554,10 @@ impl<'db> Class<'db> {
} else if !function.decorators(db).is_empty() {
SymbolAndQualifiers::todo("decorated method")
} else {
SymbolAndQualifiers(Symbol::bound(declared_ty), qualifiers)
SymbolAndQualifiers(declared, qualifiers)
}
} else {
SymbolAndQualifiers(Symbol::bound(declared_ty), qualifiers)
SymbolAndQualifiers(declared, qualifiers)
}
}
Ok(SymbolAndQualifiers(Symbol::Unbound, _)) => {
@ -535,9 +566,8 @@ impl<'db> Class<'db> {
let bindings = use_def.public_bindings(symbol_id);
let inferred = symbol_from_bindings(db, bindings);
let inferred_ty = inferred.ignore_possibly_unbound();
Self::implicit_instance_attribute(db, body_scope, name, inferred_ty).into()
Self::implicit_instance_attribute(db, body_scope, name, &inferred).into()
}
Err((declared_ty, _conflicting_declarations)) => {
// There are conflicting declarations for this attribute in the class body.
@ -551,7 +581,7 @@ impl<'db> Class<'db> {
// This attribute is neither declared nor bound in the class body.
// It could still be implicitly defined in a method.
Self::implicit_instance_attribute(db, body_scope, name, None).into()
Self::implicit_instance_attribute(db, body_scope, name, &Symbol::Unbound).into()
}
}