ruff
8e738339 - [ty] Error context for assignability diagnostics (#24309)

Commit
4 days ago
[ty] Error context for assignability diagnostics (#24309) ## Summary This PR adds error context to diagnostics that involve some kind of assignability check (`invalid-assignment`, `invalid-argument-type`, `invalid-override`). This context is "tree shaped", which is useful if the involved types are complex. Especially if there are multiple possible options on the "target" side of the assignability check, like when assigning to a union: ```py def f(xs: tuple[int, Buffer | list[bytes] | None]): ... def g(xs: tuple[int, str | bytes]): f(xs) ``` <img width="1180" height="331" alt="image" src="https://github.com/user-attachments/assets/aebc84d6-4418-4a9f-940f-2418de9682e8" /> The current implementation has some limitations: * This PR only adds new hints for a handful of cases (tuples, unions, as well as basic support for callables and protocols). There is a lot more that we can do here (intersections, inconsistent specializations, overloads, TypedDicts, …), but this can be done as a follow-up. In a few places, we currently use `self.without_error_context(|| { … })` to suppress context collection because we would need to properly combine multiple pieces of context into a parent node that does not exist yet. * We only add very basic support for callables here. ~~For example, we only say "incompatible parameter types" without pointing at a specific parameter. This can obviously be improved.~~ For example, there is no support for overloads yet. * Everything is just rendered using additional `info` subdiagnostics. There are many cases where it would help to add subdiagnostics with annotations, but that requires some more design. For example, we should probably only do that when the context tree is linear/degenerate (only points to one specific reason for why the assignment failed). * We do currently short circuit in some cases. For example, when we check assignability of two tuples of equal length, we only show context for the first failing element. We can change this later if we want. closes https://github.com/astral-sh/ty/issues/163 (I will open more detailed follow-up tickets) and the following issues: <details> <summary>closes <a href="https://github.com/astral-sh/ty/issues/2662">#2662</a></summary> <img width="971" height="294" alt="image" src="https://github.com/user-attachments/assets/db52d9d2-5c0c-4823-a09e-29a825a01566" /> </details> <details> <summary>closes <a href="https://github.com/astral-sh/ty/issues/1644">#1644</a></summary> <img width="1183" height="441" alt="image" src="https://github.com/user-attachments/assets/0e6ca28f-b08a-410a-ad24-769aa2182066" /> </details> <details> <summary>closes <a href="https://github.com/astral-sh/ty/issues/1646">#1646</a></summary> <img width="1079" height="461" alt="image" src="https://github.com/user-attachments/assets/8f8ef48f-be72-4ac3-a744-61073343537b" /> </details> <details> <summary>Addresses the already closed <a href="https://github.com/astral-sh/ty/issues/1591">#1591</a> in the sense that we could now use ty to debug the problem</summary> <img width="1866" height="308" alt="image" src="https://github.com/user-attachments/assets/23b67de7-3e8e-4c63-83f1-be4ce2bccbc6" /> </details> ## Ecosystem impact Well, you don't see anything on this PR because we (currently) do not attach sub-diagnostics in "concise" diagnostic mode, but I did play with this a lot in the IDE. I also performed some experiments locally to see if there are any pathologically large context trees and didn't find anything truly absurd. ## Performance Now that we avoid collecting the context if the diagnostics will be suppressed anyway, the larger performance regressions are gone (thanks @carljm). ~~Only `colour_science` shows a 4% regression, which seems acceptable.~~ (this is also fixed after yet another optimization). The ecosystem timing report also shows nothing dramatic (at least it didn't in a previous run. I think the report is currently broken, see Discord). ## Test Plan Updated Markdown tests
Author
Parents
Loading