From c0768dfd96a9432f71149dcb1c64d8fb298ef1a8 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 25 Jul 2025 14:56:14 +0200 Subject: [PATCH] [ty] Attribute access on intersections with negative parts (#19524) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 `~` 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. --- .../resources/mdtest/attributes.md | 15 +++ .../resources/mdtest/narrow/hasattr.md | 104 +++++++++++------- .../resources/mdtest/protocols.md | 3 +- crates/ty_python_semantic/src/types.rs | 27 ++--- 4 files changed, 92 insertions(+), 57 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/attributes.md b/crates/ty_python_semantic/resources/mdtest/attributes.md index f4b2d8c14b..1d31b08118 100644 --- a/crates/ty_python_semantic/resources/mdtest/attributes.md +++ b/crates/ty_python_semantic/resources/mdtest/attributes.md @@ -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 diff --git a/crates/ty_python_semantic/resources/mdtest/narrow/hasattr.md b/crates/ty_python_semantic/resources/mdtest/narrow/hasattr.md index c8148f54f6..f6839562de 100644 --- a/crates/ty_python_semantic/resources/mdtest/narrow/hasattr.md +++ b/crates/ty_python_semantic/resources/mdtest/narrow/hasattr.md @@ -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 & - reveal_type(x.spam) # revealed: object +def _(obj: NonFinalClass): + if hasattr(obj, "spam"): + reveal_type(obj) # revealed: NonFinalClass & + reveal_type(obj.spam) # revealed: object else: - reveal_type(x) # revealed: Foo & ~ - - # 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 & ~ # 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. `` is a supertype of `WithSpam`, and so +`WithSpam & ` 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 & - reveal_type(obj.x) # revealed: int + if hasattr(obj, "spam"): + reveal_type(obj) # revealed: MaybeWithSpam & + reveal_type(obj.spam) # revealed: int else: - reveal_type(obj) # revealed: Baz & ~ + reveal_type(obj) # revealed: MaybeWithSpam & ~ - # 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 & ) | LiteralString - else: - reveal_type(x) # revealed: int & ~ + # TODO: Ideally, we would emit `[unresolved-attribute]` and reveal `Unknown` here: + # error: [possibly-unbound-attribute] + reveal_type(obj.spam) # revealed: int ``` diff --git a/crates/ty_python_semantic/resources/mdtest/protocols.md b/crates/ty_python_semantic/resources/mdtest/protocols.md index bfb85f784a..1c49a8c9a7 100644 --- a/crates/ty_python_semantic/resources/mdtest/protocols.md +++ b/crates/ty_python_semantic/resources/mdtest/protocols.md @@ -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 diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index cef4757e07..1c14cf6d18 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -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> { + 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 => {