Skip to content

[ty] Complete support for more detailed diagnostics on possibly unbound errors from implicit dunder calls against unions.#24676

Open
lerebear wants to merge 3 commits intomainfrom
lerebear/push-ypypplmwkqpy
Open

[ty] Complete support for more detailed diagnostics on possibly unbound errors from implicit dunder calls against unions.#24676
lerebear wants to merge 3 commits intomainfrom
lerebear/push-ypypplmwkqpy

Conversation

@lerebear
Copy link
Copy Markdown
Contributor

@lerebear lerebear commented Apr 16, 2026

Summary

Building on #24662, and to resolve astral-sh/ty#940, this adds more "info" sub-diagnostics to possibly unbound method error messages that occur when a dunder is called implicitly against a union type. The implicit dunder calls covered here are __iter__, __await__ and __mro_entries__.

Test Plan

Please see new/updated mdtests and related snapshots.

@astral-sh-bot astral-sh-bot Bot added the ty Multi-file analysis & type inference label Apr 16, 2026
@lerebear lerebear changed the title Complete support for more detailed diagnostics on possibly unbound errors from implicit dunder calls against unions. [ty] Complete support for more detailed diagnostics on possibly unbound errors from implicit dunder calls against unions. Apr 16, 2026
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented Apr 16, 2026

Typing conformance results

No changes detected ✅

Current numbers
The percentage of diagnostics emitted that were expected errors held steady at 87.94%. The percentage of expected errors that received a diagnostic held steady at 83.36%. The number of fully passing files held steady at 79/133.

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented Apr 16, 2026

Memory usage report

Memory usage unchanged ✅

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented Apr 16, 2026

ecosystem-analyzer results

No diagnostic changes detected ✅

Flaky changes detected. This PR summary excludes flaky changes; see the HTML report for details.

Full report with detailed diff (timing results)

@lerebear lerebear force-pushed the lerebear/push-tuskvpoxpzyx branch 4 times, most recently from a77845f to 3f40b1a Compare April 18, 2026 20:13
lerebear added a commit that referenced this pull request Apr 20, 2026
…m context manager dunder methods invoked on a union. (#24662)

<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
  requests.)
- Does this pull request include references to any relevant issues?
- Does this PR follow our AI policy
(https://github.com/astral-sh/.github/blob/main/AI_POLICY.md)?
-->

## Summary

As part of astral-sh/ty#940, this helps us
emit more specific diagnostics for possibly unbound context manager
dunders (e.g., `__enter__`, `__exit__`) invoked on a union type.

Where previously the following snippet would produce just the top-level
diagnostic commented below:

```py
class Context:
    def __enter__(self): ...
    def __exit__(self, *args): ...

class NotContext:
    pass

def _(x: Context | NotContext):
    # error: [invalid-context-manager] "Object of type `Context | NotContext` cannot be used with `with` because the methods `__enter__` and `__exit__` are possibly unbound"
    with x:
        pass
```

We will now produce two further "info" sub-diagnostics:

```
info: `NotContext` does not implement `__enter__`
info: `NotContext` does not implement `__exit__`
```

## Approach

- This implements the approach suggested by `@carljm`
[here](#20199 (review))
from a previous attempt to address
astral-sh/ty#940; it extends
`CallDunderError::PossiblyUnbound` with a new `unbound_on` field that
stores a list of the union members on which a particular dunder is
unbound. We create the new, richer error with a new
`UnionType.try_call_dunder_with_policy` method that looks up the dunder
on each member of the union, and then aggregates the results. This is
supersedes the previous `UnionType.map_with_boundness_and_qualifiers`
approach, and allows us to preserve the per-member binding information
that we use to produce the more detailed diagnostic.

- There are two alternatives to this approach that I considered but
rejected:
- Rebuild the specific union member diagnositic information at each
callsite, and only when relevant. This was the approach originally taken
by #20199, but I think it will
lead to some unnecessary code duplication across callsites (of which
there are at least three more).
- Refactor such that `UnionType.map_with_boundness_and_qualifiers` such
that it no longer loses member-specific binding information when
producing its result. This would have required an extension to
`PlaceAndQualifiers`, which would have a large blast radius and also
introduce overhead in several cases where member-specific information
for unions is not necessary.

- There are more implicit dunder calls that can benefit from the new
shape of `CallDunderError::PossiblyUnbound`, but I have intentionally
deferred those to [a
follow-up](#24676) in order to
first collect feedback on a more targeted changeset.

- The first three commits in this PR
(926bcec,
65dc3fb,
988e81d) are "prefactors" that do not
change any observable behaviour. The fourth
(2422844) actually implements the
improvement, and deserves the most scrutiny.

## Test Plan

Please see updated mdtests and associated snapshots.
Base automatically changed from lerebear/push-tuskvpoxpzyx to main April 20, 2026 18:40
@lerebear lerebear force-pushed the lerebear/push-ypypplmwkqpy branch 2 times, most recently from 39073b8 to 37cb78f Compare April 21, 2026 03:49
@lerebear lerebear force-pushed the lerebear/push-ypypplmwkqpy branch from 37cb78f to 39da4f7 Compare April 21, 2026 03:52
info: Definition of class `Foo` will raise `TypeError` at runtime
info: An instance type is only a valid class base if it has a valid `__mro_entries__` method
info: Type `HasMroEntries | NoMroEntries` may have an `__mro_entries__` attribute, but it may be missing
info: `NoMroEntries` does not implement `__mro_entries__`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would appreciate feedback on whether or not we should consolidate or eliminate some info diagnostics in this case. The current pattern is clear and each sub-diagnostic is informative, but 4 separate diagnostics might be considered a bit excessive.

@lerebear lerebear marked this pull request as ready for review April 21, 2026 14:07
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.

improve diagnostics for possibly-unbound call-dunder with union

2 participants