mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-30 05:44:56 +00:00
[ty] Allow declared-only class-level attributes to be accessed on the class (#19071)
Some checks are pending
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run
Some checks are pending
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run
## Summary Allow declared-only class-level attributes to be accessed on the class: ```py class C: attr: int C.attr # this is now allowed ``` closes https://github.com/astral-sh/ty/issues/384 closes https://github.com/astral-sh/ty/issues/553 ## Ecosystem analysis * We see many removed `unresolved-attribute` false-positives for code that makes use of sqlalchemy, as expected (see changes for `prefect`) * We see many removed `call-non-callable` false-positives for uses of `pytest.skip` and similar, as expected * Most new diagnostics seem to be related to cases like the following, where we previously inferred `int` for `Derived().x`, but now we infer `int | None`. I think this should be a conflicting-declarations/bad-override error anyway? The new behavior may even be preferred here? ```py class Base: x: int | None class Derived(Base): def __init__(self): self.x: int = 1 ```
This commit is contained in:
parent
5f426b9f8b
commit
f76d3f87cf
6 changed files with 37 additions and 68 deletions
|
@ -87,13 +87,8 @@ c_instance = C()
|
||||||
|
|
||||||
reveal_type(c_instance.declared_and_bound) # revealed: str | None
|
reveal_type(c_instance.declared_and_bound) # revealed: str | None
|
||||||
|
|
||||||
# Note that both mypy and pyright show no error in this case! So we may reconsider this in
|
reveal_type(C.declared_and_bound) # revealed: str | None
|
||||||
# the future, if it turns out to produce too many false positives. We currently emit:
|
|
||||||
# error: [unresolved-attribute] "Attribute `declared_and_bound` can only be accessed on instances, not on the class object `<class 'C'>` itself."
|
|
||||||
reveal_type(C.declared_and_bound) # revealed: Unknown
|
|
||||||
|
|
||||||
# Same as above. Mypy and pyright do not show an error here.
|
|
||||||
# error: [invalid-attribute-access] "Cannot assign to instance attribute `declared_and_bound` from the class object `<class 'C'>`"
|
|
||||||
C.declared_and_bound = "overwritten on class"
|
C.declared_and_bound = "overwritten on class"
|
||||||
|
|
||||||
# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `declared_and_bound` of type `str | None`"
|
# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `declared_and_bound` of type `str | None`"
|
||||||
|
@ -102,8 +97,11 @@ c_instance.declared_and_bound = 1
|
||||||
|
|
||||||
#### Variable declared in class body and not bound anywhere
|
#### Variable declared in class body and not bound anywhere
|
||||||
|
|
||||||
If a variable is declared in the class body but not bound anywhere, we still consider it a pure
|
If a variable is declared in the class body but not bound anywhere, we consider it to be accessible
|
||||||
instance variable and allow access to it via instances.
|
on instances and the class itself. It would be more consistent to treat this as a pure instance
|
||||||
|
variable (and require the attribute to be annotated with `ClassVar` if it should be accessible on
|
||||||
|
the class as well), but other type checkers allow this as well. This is also heavily relied on in
|
||||||
|
the Python ecosystem:
|
||||||
|
|
||||||
```py
|
```py
|
||||||
class C:
|
class C:
|
||||||
|
@ -113,11 +111,8 @@ c_instance = C()
|
||||||
|
|
||||||
reveal_type(c_instance.only_declared) # revealed: str
|
reveal_type(c_instance.only_declared) # revealed: str
|
||||||
|
|
||||||
# Mypy and pyright do not show an error here. We treat this as a pure instance variable.
|
reveal_type(C.only_declared) # revealed: str
|
||||||
# error: [unresolved-attribute] "Attribute `only_declared` can only be accessed on instances, not on the class object `<class 'C'>` itself."
|
|
||||||
reveal_type(C.only_declared) # revealed: Unknown
|
|
||||||
|
|
||||||
# error: [invalid-attribute-access] "Cannot assign to instance attribute `only_declared` from the class object `<class 'C'>`"
|
|
||||||
C.only_declared = "overwritten on class"
|
C.only_declared = "overwritten on class"
|
||||||
```
|
```
|
||||||
|
|
||||||
|
@ -1235,6 +1230,16 @@ def _(flag: bool):
|
||||||
reveal_type(Derived().x) # revealed: int | Any
|
reveal_type(Derived().x) # revealed: int | Any
|
||||||
|
|
||||||
Derived().x = 1
|
Derived().x = 1
|
||||||
|
|
||||||
|
# TODO
|
||||||
|
# The following assignment currently fails, because we first check if "a" is assignable to the
|
||||||
|
# attribute on the meta-type of `Derived`, i.e. `<class 'Derived'>`. When accessing the class
|
||||||
|
# member `x` on `Derived`, we only see the `x: int` declaration and do not union it with the
|
||||||
|
# type of the base class attribute `x: Any`. This could potentially be improved. Note that we
|
||||||
|
# see a type of `int | Any` above because we have the full union handling of possibly-unbound
|
||||||
|
# *instance* attributes.
|
||||||
|
|
||||||
|
# error: [invalid-assignment] "Object of type `Literal["a"]` is not assignable to attribute `x` of type `int`"
|
||||||
Derived().x = "a"
|
Derived().x = "a"
|
||||||
```
|
```
|
||||||
|
|
||||||
|
@ -1299,10 +1304,8 @@ def _(flag: bool):
|
||||||
if flag:
|
if flag:
|
||||||
self.x = 1
|
self.x = 1
|
||||||
|
|
||||||
# error: [possibly-unbound-attribute]
|
|
||||||
reveal_type(Foo().x) # revealed: int | Unknown
|
reveal_type(Foo().x) # revealed: int | Unknown
|
||||||
|
|
||||||
# error: [possibly-unbound-attribute]
|
|
||||||
Foo().x = 1
|
Foo().x = 1
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
|
@ -120,8 +120,7 @@ def _(flag: bool):
|
||||||
|
|
||||||
### Dunder methods as class-level annotations with no value
|
### Dunder methods as class-level annotations with no value
|
||||||
|
|
||||||
Class-level annotations with no value assigned are considered instance-only, and aren't available as
|
Class-level annotations with no value assigned are considered to be accessible on the class:
|
||||||
dunder methods:
|
|
||||||
|
|
||||||
```py
|
```py
|
||||||
from typing import Callable
|
from typing import Callable
|
||||||
|
@ -129,10 +128,8 @@ from typing import Callable
|
||||||
class C:
|
class C:
|
||||||
__call__: Callable[..., None]
|
__call__: Callable[..., None]
|
||||||
|
|
||||||
# error: [call-non-callable]
|
|
||||||
C()()
|
C()()
|
||||||
|
|
||||||
# error: [invalid-assignment]
|
|
||||||
_: Callable[..., None] = C()
|
_: Callable[..., None] = C()
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
|
@ -810,21 +810,6 @@ D(1) # OK
|
||||||
D() # error: [missing-argument]
|
D() # error: [missing-argument]
|
||||||
```
|
```
|
||||||
|
|
||||||
### Accessing instance attributes on the class itself
|
|
||||||
|
|
||||||
Just like for normal classes, accessing instance attributes on the class itself is not allowed:
|
|
||||||
|
|
||||||
```py
|
|
||||||
from dataclasses import dataclass
|
|
||||||
|
|
||||||
@dataclass
|
|
||||||
class C:
|
|
||||||
x: int
|
|
||||||
|
|
||||||
# error: [unresolved-attribute] "Attribute `x` can only be accessed on instances, not on the class object `<class 'C'>` itself."
|
|
||||||
C.x
|
|
||||||
```
|
|
||||||
|
|
||||||
### Return type of `dataclass(...)`
|
### Return type of `dataclass(...)`
|
||||||
|
|
||||||
A call like `dataclass(order=True)` returns a callable itself, which is then used as the decorator.
|
A call like `dataclass(order=True)` returns a callable itself, which is then used as the decorator.
|
||||||
|
|
|
@ -533,7 +533,13 @@ class FooSubclassOfAny:
|
||||||
x: SubclassOfAny
|
x: SubclassOfAny
|
||||||
|
|
||||||
static_assert(not is_subtype_of(FooSubclassOfAny, HasX))
|
static_assert(not is_subtype_of(FooSubclassOfAny, HasX))
|
||||||
static_assert(not is_assignable_to(FooSubclassOfAny, HasX))
|
|
||||||
|
# `FooSubclassOfAny` is assignable to `HasX` for the following reason. The `x` attribute on `FooSubclassOfAny`
|
||||||
|
# is accessible on the class itself. When accessing `x` on an instance, the descriptor protocol is invoked, and
|
||||||
|
# `__get__` is looked up on `SubclassOfAny`. Every member access on `SubclassOfAny` yields `Any`, so `__get__` is
|
||||||
|
# also available, and calling `Any` also yields `Any`. Thus, accessing `x` on an instance of `FooSubclassOfAny`
|
||||||
|
# yields `Any`, which is assignable to `int` and vice versa.
|
||||||
|
static_assert(is_assignable_to(FooSubclassOfAny, HasX))
|
||||||
|
|
||||||
class FooWithY(Foo):
|
class FooWithY(Foo):
|
||||||
y: int
|
y: int
|
||||||
|
@ -1586,11 +1592,7 @@ def g(a: Truthy, b: FalsyFoo, c: FalsyFooSubclass):
|
||||||
reveal_type(bool(c)) # revealed: Literal[False]
|
reveal_type(bool(c)) # revealed: Literal[False]
|
||||||
```
|
```
|
||||||
|
|
||||||
It is not sufficient for a protocol to have a callable `__bool__` instance member that returns
|
The same works with a class-level declaration of `__bool__`:
|
||||||
`Literal[True]` for it to be considered always truthy. Dunder methods are looked up on the class
|
|
||||||
rather than the instance. If a protocol `X` has an instance-attribute `__bool__` member, it is
|
|
||||||
unknowable whether that attribute can be accessed on the type of an object that satisfies `X`'s
|
|
||||||
interface:
|
|
||||||
|
|
||||||
```py
|
```py
|
||||||
from typing import Callable
|
from typing import Callable
|
||||||
|
@ -1599,7 +1601,7 @@ class InstanceAttrBool(Protocol):
|
||||||
__bool__: Callable[[], Literal[True]]
|
__bool__: Callable[[], Literal[True]]
|
||||||
|
|
||||||
def h(obj: InstanceAttrBool):
|
def h(obj: InstanceAttrBool):
|
||||||
reveal_type(bool(obj)) # revealed: bool
|
reveal_type(bool(obj)) # revealed: Literal[True]
|
||||||
```
|
```
|
||||||
|
|
||||||
## Callable protocols
|
## Callable protocols
|
||||||
|
@ -1832,7 +1834,8 @@ def _(r: Recursive):
|
||||||
reveal_type(r.direct) # revealed: Recursive
|
reveal_type(r.direct) # revealed: Recursive
|
||||||
reveal_type(r.union) # revealed: None | Recursive
|
reveal_type(r.union) # revealed: None | Recursive
|
||||||
reveal_type(r.intersection1) # revealed: C & Recursive
|
reveal_type(r.intersection1) # revealed: C & Recursive
|
||||||
reveal_type(r.intersection2) # revealed: C & ~Recursive
|
# revealed: @Todo(map_with_boundness: intersections with negative contributions) | (C & ~Recursive)
|
||||||
|
reveal_type(r.intersection2)
|
||||||
reveal_type(r.t) # revealed: tuple[int, tuple[str, Recursive]]
|
reveal_type(r.t) # revealed: tuple[int, tuple[str, Recursive]]
|
||||||
reveal_type(r.callable1) # revealed: (int, /) -> Recursive
|
reveal_type(r.callable1) # revealed: (int, /) -> Recursive
|
||||||
reveal_type(r.callable2) # revealed: (Recursive, /) -> int
|
reveal_type(r.callable2) # revealed: (Recursive, /) -> int
|
||||||
|
|
|
@ -64,24 +64,6 @@ c = C()
|
||||||
c.a = 2
|
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
|
## Too many arguments
|
||||||
|
|
||||||
```py
|
```py
|
||||||
|
|
|
@ -235,29 +235,28 @@ pub(crate) fn class_symbol<'db>(
|
||||||
) -> PlaceAndQualifiers<'db> {
|
) -> PlaceAndQualifiers<'db> {
|
||||||
place_table(db, scope)
|
place_table(db, scope)
|
||||||
.place_id_by_name(name)
|
.place_id_by_name(name)
|
||||||
.map(|symbol| {
|
.map(|place| {
|
||||||
let symbol_and_quals = place_by_id(
|
let place_and_quals = place_by_id(
|
||||||
db,
|
db,
|
||||||
scope,
|
scope,
|
||||||
symbol,
|
place,
|
||||||
RequiresExplicitReExport::No,
|
RequiresExplicitReExport::No,
|
||||||
ConsideredDefinitions::EndOfScope,
|
ConsideredDefinitions::EndOfScope,
|
||||||
);
|
);
|
||||||
|
|
||||||
if symbol_and_quals.is_class_var() {
|
if !place_and_quals.place.is_unbound() {
|
||||||
// For declared class vars we do not need to check if they have bindings,
|
// Trust the declared type if we see a class-level declaration
|
||||||
// we just trust the declaration.
|
return place_and_quals;
|
||||||
return symbol_and_quals;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if let PlaceAndQualifiers {
|
if let PlaceAndQualifiers {
|
||||||
place: Place::Type(ty, _),
|
place: Place::Type(ty, _),
|
||||||
qualifiers,
|
qualifiers,
|
||||||
} = symbol_and_quals
|
} = place_and_quals
|
||||||
{
|
{
|
||||||
// Otherwise, we need to check if the symbol has bindings
|
// Otherwise, we need to check if the symbol has bindings
|
||||||
let use_def = use_def_map(db, scope);
|
let use_def = use_def_map(db, scope);
|
||||||
let bindings = use_def.end_of_scope_bindings(symbol);
|
let bindings = use_def.end_of_scope_bindings(place);
|
||||||
let inferred = place_from_bindings_impl(db, bindings, RequiresExplicitReExport::No);
|
let inferred = place_from_bindings_impl(db, bindings, RequiresExplicitReExport::No);
|
||||||
|
|
||||||
// TODO: we should not need to calculate inferred type second time. This is a temporary
|
// TODO: we should not need to calculate inferred type second time. This is a temporary
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue