[ty] Add error context for intersection types - #24772

hmmm... when Protocol types are involved, I don't think it's necessarily true that at least one element of the intersection type must be assignable to the declared type. E.g. this feels like it should be fine?

from typing import Protocol, runtime_checkable

@runtime_checkable
class Foo(Protocol):
    def foo(self): ...

class Bar(Protocol):
    def bar(self): ...

class FooAndBar(Foo, Bar, Protocol): ...

def f(x: Bar):
    if isinstance(x, Foo):
        reveal_type(x)  # Bar & Foo
        y: FooAndBar = x

although... we currently emit a diagnostic on the last line there 🙈 not sure why yet https://play.ty.dev/3d1b9f3f-cbf9-423f-9c0c-495550da8d57

astral-sh/ty#338 feels related conceptually

not sure why yet

Isn't that the expected behavior based on our implementation of type relations for intersections, which (for assignability from an intersection) is precisely "break it apart and require at least one element to be assignable to the target"?

I don't disagree that Foo & Bar should be assignable to FooAndBar, but to support that we would need to add a new relation arm for intersection vs protocol that is higher priority than the main intersection arm.

but to support that we would need to add a new relation arm for intersection vs protocol that is higher priority than the main intersection arm.

Right, or just move the existing "anything vs a protocol" arm higher in relation.rs? I can experiment tomorrow

Interesting observation!

Does this suggest that we should change the "no element of intersection …" message here for the common case? I would think it's still fine?

Strictly speaking I think the "no element of interesection ..." message is only accurate if the right-hand side is a nominal type. Most types are nominal types, so your message is usually accurate. But if the right-hand side is a Callable/Protocol, or a union/intersection of Callable/Protocol types, I think the message would be inaccurate. Detection of those cases wouldn't be hard -- but perhaps then we need to figure out what to say instead in those cases

If we make changes in a future PR to allow intersections to be assignable to structural types without breaking them apart to test individual elements, then the error context collection for those assignability checks will naturally happen in a different code path and use a different error context variant. We wouldn't use NoIntersectionElementAssignableToTarget context at all; instead we'd give (e.g.) Protocol oriented error context about the members of Foo & Bar vs FooAndBar; we wouldn't even enter the assignability code path touched in this PR.

So I think we should consider that part of that (future) feature. NoIntersectionElementAssignableToTarget is clearly named, and is specific to a code path where that is exactly what we check, so the current error message is usefully precise about the actual assignability requirements in this code path.

Put differently: any "detection of those cases" that we would add in this PR would be duplicative of the actual reordering of check-pair cases discussed above, and would become dead code once we actually do that reordering.

So it seems best to me to land this PR without making changes to this error message.

If we make changes in a future PR to allow intersections to be assignable to structural types without breaking them apart to test individual elements, then the error context collection for those assignability checks will naturally happen in a different code path and use a different error context variant.

I don't think that's necessarily the case at all — fundamentally whether an intersection satisfies a protocol should be answerable in the same way as the question of whether a nominal instance type satisfies a protocol. Just check whether all protocol members are available on the intersection type with the correct type. Currently we have a separate, special-cased code path for Protocol <-> Protocol assignability, but ideally we wouldn't, there's no particularly principled reason for it being that way currently

@carljm pointed out in person that we're vigorously agreeing with each other, so yes, I think this PR is fine 😄

#24796 to fix the Intersection <-> Protocol assignability bug

ideally we could possibly look at making these more concise, in the future? e.g.

this is obviously a big improvement as-is, though