[ty] Emit more specific diagnostics for "possibly unbound" errors from context manager dunder methods invoked on a union.

[lerebear]

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:

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 from a previous attempt to address improve diagnostics for possibly-unbound call-dunder with union 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 [ty] chore: ContextManagerError message update #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 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.

[astral-sh-bot]

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]

Memory usage report

Summary

Project	Old	New	Diff	Outcome
prefect	716.76MB	716.80MB	+0.00% (34.58kB)	⏫
trio	117.64MB	117.65MB	+0.00% (5.44kB)	⏫
flake8	47.94MB	47.94MB	-0.00% (96.00B)	⬇️
sphinx	262.78MB	262.78MB	-0.00% (3.02kB)	⬇️

Significant changes

Click to expand detailed breakdown

prefect

Name	Old	New	Diff	Outcome
infer_expression_types_impl	63.01MB	63.02MB	+0.02% (14.20kB)	⏫
infer_definition_types	90.38MB	90.39MB	+0.01% (11.14kB)	⏫
Type<'db>::member_lookup_with_policy_	17.24MB	17.23MB	-0.06% (10.98kB)	⏫
infer_expression_type_impl	13.41MB	13.42MB	+0.07% (9.04kB)	⏫
Type<'db>::member_lookup_with_policy_::interned_arguments	5.94MB	5.94MB	-0.10% (5.89kB)	⏫
infer_scope_types_impl	54.79MB	54.79MB	+0.01% (4.93kB)	⏫
all_narrowing_constraints_for_expression	7.20MB	7.20MB	+0.05% (3.46kB)	⏫
StaticClassLiteral<'db>::implicit_attribute_inner_	10.06MB	10.06MB	+0.02% (2.39kB)	⏫
all_negative_narrowing_constraints_for_expression	2.63MB	2.63MB	+0.08% (2.04kB)	⏫
loop_header_reachability	432.49kB	433.44kB	+0.22% (972.00B)	⏫
GenericAlias<'db>::variance_of_	579.91kB	580.83kB	+0.16% (936.00B)	⏫
infer_deferred_types	14.59MB	14.60MB	+0.00% (696.00B)	⏫
FunctionType<'db>::signature_	4.08MB	4.08MB	+0.01% (360.00B)	⏫
infer_unpack_types	899.07kB	899.35kB	+0.03% (288.00B)	⏫
is_redundant_with_impl	4.28MB	4.28MB	+0.01% (288.00B)	⏫
...			6 more

trio

Name	Old	New	Diff	Outcome
infer_definition_types	7.58MB	7.58MB	+0.03% (2.39kB)	⏫
infer_expression_types_impl	7.00MB	7.01MB	+0.03% (2.38kB)	⏫
Type<'db>::member_lookup_with_policy_	1.95MB	1.95MB	-0.08% (1.54kB)	⏫
infer_expression_type_impl	1.30MB	1.30MB	+0.11% (1.43kB)	⏫
Type<'db>::member_lookup_with_policy_::interned_arguments	925.44kB	924.62kB	-0.09% (832.00B)	⏫
all_narrowing_constraints_for_expression	590.41kB	591.07kB	+0.11% (672.00B)	⏫
loop_header_reachability	129.49kB	130.03kB	+0.42% (552.00B)	⏫
infer_scope_types_impl	4.75MB	4.75MB	+0.01% (288.00B)	⏫
all_negative_narrowing_constraints_for_expression	184.36kB	184.47kB	+0.06% (120.00B)	⏫

flake8

Name	Old	New	Diff	Outcome
infer_scope_types_impl	986.17kB	986.36kB	+0.02% (192.00B)	⬇️
Type<'db>::member_lookup_with_policy_	553.20kB	553.02kB	-0.03% (184.00B)	⬇️
Type<'db>::member_lookup_with_policy_::interned_arguments	230.75kB	230.65kB	-0.04% (104.00B)	⬇️

sphinx

Name	Old	New	Diff	Outcome
Type<'db>::member_lookup_with_policy_	6.86MB	6.85MB	-0.07% (5.12kB)	⬇️
infer_definition_types	23.63MB	23.63MB	+0.01% (2.53kB)	⬇️
Type<'db>::member_lookup_with_policy_::interned_arguments	2.67MB	2.67MB	-0.07% (2.03kB)	⬇️
infer_expression_types_impl	20.75MB	20.76MB	+0.00% (900.00B)	⬇️
infer_scope_types_impl	15.43MB	15.43MB	+0.00% (480.00B)	⬇️
loop_header_reachability	364.36kB	364.45kB	+0.02% (84.00B)	⬇️
all_narrowing_constraints_for_expression	2.34MB	2.34MB	+0.00% (84.00B)	⬇️
infer_expression_type_impl	2.90MB	2.90MB	+0.00% (84.00B)	⬇️

[astral-sh-bot]

ecosystem-analyzer results

No diagnostic changes detected ✅

Full report with detailed diff (timing results)

[MichaReiser]

@lerebear we use the assignee field to track who's responsible for reviewing a PR based on a round robin selection. For this to work, the Assignees field must be empty when marking a PR ready for review. You're obviously always allowed to manually change the assignee if you think someone else is a better fit.

[charliermarsh]

Just two questions. The Rust here looks good!

[lerebear]

Just two questions.

Thanks for the review @charliermarsh! I addressed both points, so this should be ready for a fresh review please.

[charliermarsh]

Thanks!