mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 13:24:57 +00:00
[ty] Attribute access on intersections with negative parts (#19524)
## Summary We currently infer a `@Todo` type whenever we access an attribute on an intersection type with negative components. This can happen very naturally. Consequently, this `@Todo` type is rather pervasive and hides a lot of true positives that ty could otherwise detect: ```py class Foo: attr: int = 1 def _(f: Foo | None): if f: reveal_type(f) # Foo & ~AlwaysFalsy reveal_type(f.attr) # now: int, previously: @Todo ``` The changeset here proposes to handle member access on these intersection types by simply ignoring all negative contributions. This is not always ideal: a negative contribution like `~<Protocol with members 'attr'>` could be a hint that `.attr` should not be accessible on the full intersection type. The behavior can certainly be improved in the future, but this seems like a reasonable initial step to get rid of this unnecessary `@Todo` type. ## Ecosystem analysis There are quite a few changes here. I spot-checked them and found one bug where attribute access on pure negation types (`~P == object & ~P`) would not allow attributes on `object` to be accessed. After that was fixed, I only see true positives and known problems. The fact that a lot of `unused-ignore-comment` diagnostics go away are also evidence for the fact that this touches a sensitive area, where static analysis clashes with dynamically adding attributes to objects: ```py … # type: ignore # Runtime attribute access ``` ## Test Plan Updated tests.
This commit is contained in:
parent
d4eb4277ad
commit
c0768dfd96
4 changed files with 92 additions and 57 deletions
|
@ -1451,6 +1451,21 @@ def _(a_and_b: Intersection[type[A], type[B]]):
|
|||
a_and_b.x = R()
|
||||
```
|
||||
|
||||
### Negation types
|
||||
|
||||
Make sure that attributes accessible on `object` are also accessible on a negation type like `~P`,
|
||||
which is equivalent to `object & ~P`:
|
||||
|
||||
```py
|
||||
class P: ...
|
||||
|
||||
def _(obj: object):
|
||||
if not isinstance(obj, P):
|
||||
reveal_type(obj) # revealed: ~P
|
||||
|
||||
reveal_type(obj.__dict__) # revealed: dict[str, Any]
|
||||
```
|
||||
|
||||
### Possible unboundness
|
||||
|
||||
```py
|
||||
|
|
|
@ -7,60 +7,80 @@ accomplished using an intersection with a synthesized protocol:
|
|||
from typing import final
|
||||
from typing_extensions import LiteralString
|
||||
|
||||
class Foo: ...
|
||||
class NonFinalClass: ...
|
||||
|
||||
@final
|
||||
class Bar: ...
|
||||
|
||||
def f(x: Foo):
|
||||
if hasattr(x, "spam"):
|
||||
reveal_type(x) # revealed: Foo & <Protocol with members 'spam'>
|
||||
reveal_type(x.spam) # revealed: object
|
||||
def _(obj: NonFinalClass):
|
||||
if hasattr(obj, "spam"):
|
||||
reveal_type(obj) # revealed: NonFinalClass & <Protocol with members 'spam'>
|
||||
reveal_type(obj.spam) # revealed: object
|
||||
else:
|
||||
reveal_type(x) # revealed: Foo & ~<Protocol with members 'spam'>
|
||||
|
||||
# TODO: should error and reveal `Unknown`
|
||||
reveal_type(x.spam) # revealed: @Todo(map_with_boundness: intersections with negative contributions)
|
||||
|
||||
if hasattr(x, "not-an-identifier"):
|
||||
reveal_type(x) # revealed: Foo
|
||||
else:
|
||||
reveal_type(x) # revealed: Foo
|
||||
|
||||
def g(x: Bar):
|
||||
if hasattr(x, "spam"):
|
||||
reveal_type(x) # revealed: Never
|
||||
reveal_type(x.spam) # revealed: Never
|
||||
else:
|
||||
reveal_type(x) # revealed: Bar
|
||||
reveal_type(obj) # revealed: NonFinalClass & ~<Protocol with members 'spam'>
|
||||
|
||||
# error: [unresolved-attribute]
|
||||
reveal_type(x.spam) # revealed: Unknown
|
||||
reveal_type(obj.spam) # revealed: Unknown
|
||||
|
||||
if hasattr(obj, "not-an-identifier"):
|
||||
reveal_type(obj) # revealed: NonFinalClass
|
||||
else:
|
||||
reveal_type(obj) # revealed: NonFinalClass
|
||||
```
|
||||
|
||||
For a final class, we recognize that there is no way that an object of `FinalClass` could ever have
|
||||
a `spam` attribute, so the type is narrowed to `Never`:
|
||||
|
||||
```py
|
||||
@final
|
||||
class FinalClass: ...
|
||||
|
||||
def _(obj: FinalClass):
|
||||
if hasattr(obj, "spam"):
|
||||
reveal_type(obj) # revealed: Never
|
||||
reveal_type(obj.spam) # revealed: Never
|
||||
else:
|
||||
reveal_type(obj) # revealed: FinalClass
|
||||
|
||||
# error: [unresolved-attribute]
|
||||
reveal_type(obj.spam) # revealed: Unknown
|
||||
```
|
||||
|
||||
When the corresponding attribute is already defined on the class, `hasattr` narrowing does not
|
||||
change the type. `<Protocol with members 'spam'>` is a supertype of `WithSpam`, and so
|
||||
`WithSpam & <Protocol …>` simplifies to `WithSpam`:
|
||||
|
||||
```py
|
||||
class WithSpam:
|
||||
spam: int = 42
|
||||
|
||||
def _(obj: WithSpam):
|
||||
if hasattr(obj, "spam"):
|
||||
reveal_type(obj) # revealed: WithSpam
|
||||
reveal_type(obj.spam) # revealed: int
|
||||
else:
|
||||
reveal_type(obj) # revealed: Never
|
||||
```
|
||||
|
||||
When a class may or may not have a `spam` attribute, `hasattr` narrowing can provide evidence that
|
||||
the attribute exists. Here, no `possibly-unbound-attribute` error is emitted in the `if` branch:
|
||||
|
||||
```py
|
||||
def returns_bool() -> bool:
|
||||
return False
|
||||
|
||||
class Baz:
|
||||
class MaybeWithSpam:
|
||||
if returns_bool():
|
||||
x: int = 42
|
||||
spam: int = 42
|
||||
|
||||
def h(obj: Baz):
|
||||
reveal_type(obj) # revealed: Baz
|
||||
def _(obj: MaybeWithSpam):
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(obj.x) # revealed: int
|
||||
reveal_type(obj.spam) # revealed: int
|
||||
|
||||
if hasattr(obj, "x"):
|
||||
reveal_type(obj) # revealed: Baz & <Protocol with members 'x'>
|
||||
reveal_type(obj.x) # revealed: int
|
||||
if hasattr(obj, "spam"):
|
||||
reveal_type(obj) # revealed: MaybeWithSpam & <Protocol with members 'spam'>
|
||||
reveal_type(obj.spam) # revealed: int
|
||||
else:
|
||||
reveal_type(obj) # revealed: Baz & ~<Protocol with members 'x'>
|
||||
reveal_type(obj) # revealed: MaybeWithSpam & ~<Protocol with members 'spam'>
|
||||
|
||||
# TODO: should emit `[unresolved-attribute]` and reveal `Unknown`
|
||||
reveal_type(obj.x) # revealed: @Todo(map_with_boundness: intersections with negative contributions)
|
||||
|
||||
def i(x: int | LiteralString):
|
||||
if hasattr(x, "capitalize"):
|
||||
reveal_type(x) # revealed: (int & <Protocol with members 'capitalize'>) | LiteralString
|
||||
else:
|
||||
reveal_type(x) # revealed: int & ~<Protocol with members 'capitalize'>
|
||||
# TODO: Ideally, we would emit `[unresolved-attribute]` and reveal `Unknown` here:
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(obj.spam) # revealed: int
|
||||
```
|
||||
|
|
|
@ -1894,8 +1894,7 @@ def _(r: Recursive):
|
|||
reveal_type(r.direct) # revealed: Recursive
|
||||
reveal_type(r.union) # revealed: None | Recursive
|
||||
reveal_type(r.intersection1) # revealed: C & Recursive
|
||||
# revealed: @Todo(map_with_boundness: intersections with negative contributions) | (C & ~Recursive)
|
||||
reveal_type(r.intersection2)
|
||||
reveal_type(r.intersection2) # revealed: C
|
||||
reveal_type(r.t) # revealed: tuple[int, tuple[str, Recursive]]
|
||||
reveal_type(r.callable1) # revealed: (int, /) -> Recursive
|
||||
reveal_type(r.callable2) # revealed: (Recursive, /) -> int
|
||||
|
|
|
@ -8375,21 +8375,27 @@ impl<'db> IntersectionType<'db> {
|
|||
sorted_self == other.normalized(db)
|
||||
}
|
||||
|
||||
/// Returns an iterator over the positive elements of the intersection. If
|
||||
/// there are no positive elements, returns a single `object` type.
|
||||
fn positive_elements_or_object(&self, db: &'db dyn Db) -> impl Iterator<Item = Type<'db>> {
|
||||
if self.positive(db).is_empty() {
|
||||
Either::Left(std::iter::once(Type::object(db)))
|
||||
} else {
|
||||
Either::Right(self.positive(db).iter().copied())
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn map_with_boundness(
|
||||
self,
|
||||
db: &'db dyn Db,
|
||||
mut transform_fn: impl FnMut(&Type<'db>) -> Place<'db>,
|
||||
) -> Place<'db> {
|
||||
if !self.negative(db).is_empty() {
|
||||
return Place::todo("map_with_boundness: intersections with negative contributions");
|
||||
}
|
||||
|
||||
let mut builder = IntersectionBuilder::new(db);
|
||||
|
||||
let mut all_unbound = true;
|
||||
let mut any_definitely_bound = false;
|
||||
for ty in self.positive(db) {
|
||||
let ty_member = transform_fn(ty);
|
||||
for ty in self.positive_elements_or_object(db) {
|
||||
let ty_member = transform_fn(&ty);
|
||||
match ty_member {
|
||||
Place::Unbound => {}
|
||||
Place::Type(ty_member, member_boundness) => {
|
||||
|
@ -8422,21 +8428,16 @@ impl<'db> IntersectionType<'db> {
|
|||
db: &'db dyn Db,
|
||||
mut transform_fn: impl FnMut(&Type<'db>) -> PlaceAndQualifiers<'db>,
|
||||
) -> PlaceAndQualifiers<'db> {
|
||||
if !self.negative(db).is_empty() {
|
||||
return Place::todo("map_with_boundness: intersections with negative contributions")
|
||||
.into();
|
||||
}
|
||||
|
||||
let mut builder = IntersectionBuilder::new(db);
|
||||
let mut qualifiers = TypeQualifiers::empty();
|
||||
|
||||
let mut any_unbound = false;
|
||||
let mut any_possibly_unbound = false;
|
||||
for ty in self.positive(db) {
|
||||
for ty in self.positive_elements_or_object(db) {
|
||||
let PlaceAndQualifiers {
|
||||
place: member,
|
||||
qualifiers: new_qualifiers,
|
||||
} = transform_fn(ty);
|
||||
} = transform_fn(&ty);
|
||||
qualifiers |= new_qualifiers;
|
||||
match member {
|
||||
Place::Unbound => {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue