Skip to content

Conversation

sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Aug 27, 2025

Summary

Properly preserve type qualifiers when accessing attributes on unions and intersections. This is a prerequisite for #19579.

Also fix a completely wrong implementation of map_with_boundness_and_qualifiers. It now closely follows map_with_boundness (just above).

Test Plan

I thought about it, but didn't find any easy way to test this. This only affected Type::member. Things like validation of attribute writes (where type qualifiers like ClassVar and Final are important) were already handling things correctly.

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Aug 27, 2025
@sharkdp sharkdp changed the title [ty] Fix attribute access on intersection types [ty] Preserve qualifiers when accessing attributes on unions/intersections Aug 27, 2025
Copy link
Contributor

github-actions bot commented Aug 27, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

Copy link
Contributor

github-actions bot commented Aug 27, 2025

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@sharkdp sharkdp force-pushed the david/attribute-access-on-intersections-again branch from f0c8f05 to 1760fd6 Compare August 27, 2025 15:12
@@ -398,7 +398,7 @@ def f_okay(c: Callable[[], None]):
c.__qualname__ = "my_callable"

result = getattr_static(c, "__qualname__")
reveal_type(result) # revealed: Never
reveal_type(result) # revealed: property
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly surprised that there was no TODO here. This looks fine?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this looks fine!

Comment on lines +9741 to +9742
let mut all_unbound = true;
let mut any_definitely_bound = false;
Copy link
Contributor Author

@sharkdp sharkdp Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation here now follows map_with_boundness just above. It was wrong before (see test change in callable.md).

@sharkdp sharkdp marked this pull request as ready for review August 27, 2025 15:20
@sharkdp sharkdp marked this pull request as draft August 27, 2025 15:24
@sharkdp sharkdp marked this pull request as ready for review August 27, 2025 15:28
Comment on lines +9753 to 9755
if member_boundness == Boundness::Bound {
any_definitely_bound = true;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if member_boundness == Boundness::Bound {
any_definitely_bound = true;
}
any_definitely_bound |= member_boundness == Boundness::Bound;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I prefer what we have in terms of readability. Not sure if it compiles to worse code (probably not?), but this is not a hot code path anyway.

Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@sharkdp sharkdp merged commit 0b35487 into main Aug 27, 2025
38 checks passed
@sharkdp sharkdp deleted the david/attribute-access-on-intersections-again branch August 27, 2025 18:01
carljm added a commit to leandrobbraga/ruff that referenced this pull request Aug 27, 2025
* main:
  [`ruff`] Preserve relative whitespace in multi-line expressions (`RUF033`) (astral-sh#19647)
  [ty] Optimize TDD atom ordering (astral-sh#20098)
  [`airflow`] Extend `AIR311` and `AIR312` rules (astral-sh#20082)
  [ty] Preserve qualifiers when accessing attributes on unions/intersections (astral-sh#20114)
  [ty] Fix the inferred interface of specialized generic protocols (astral-sh#19866)
  [ty] Infer slightly more precise types for comprehensions (astral-sh#20111)
  [ty] Add more tests for protocols (astral-sh#20095)
  [ty] don't eagerly unpack aliases in user-authored unions (astral-sh#20055)
  [`flake8-use-pathlib`] Update links to the table showing the correspondence between `os` and `pathlib` (astral-sh#20103)
  [`flake8-use-pathlib`] Make `PTH100` fix unsafe because it can change behavior (astral-sh#20100)
  [`flake8-use-pathlib`] Delete unused `Rule::OsSymlink` enabled check (astral-sh#20099)
  [ty] Add search paths info to unresolved import diagnostics (astral-sh#20040)
  [`flake8-logging-format`] Add auto-fix for f-string logging calls (`G004`) (astral-sh#19303)
  Add a `ScopeKind` for the `__class__` cell (astral-sh#20048)
  Fix incorrect D413 links in docstrings convention FAQ (astral-sh#20089)
  [ty] Refactor inlay hints structure to use separate parts (astral-sh#20052)
second-ed pushed a commit to second-ed/ruff that referenced this pull request Sep 9, 2025
…tions (astral-sh#20114)

## Summary

Properly preserve type qualifiers when accessing attributes on unions
and intersections. This is a prerequisite for
astral-sh#19579.

Also fix a completely wrong implementation of
`map_with_boundness_and_qualifiers`. It now closely follows
`map_with_boundness` (just above).

## Test Plan

I thought about it, but didn't find any easy way to test this. This only
affected `Type::member`. Things like validation of attribute writes
(where type qualifiers like `ClassVar` and `Final` are important) were
already handling things correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ty Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants