[ty] Avoid expression reinference for diagnostics#21267
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
I may have underestimated the size of those intersection types... |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
0 | 0 | 211 |
invalid-assignment |
0 | 0 | 37 |
| Total | 0 | 0 | 248 |
|
Can you take a look at the failing mdtest |
|
@ibraheemdev Not sure how much work is required to get this ready -- it has some failing mdtests (and now some conflicts), but if it's not a lot, it would make sense to get it landed for the beta |
|
CI was failing spuriously because of an apparent non-determinism in intersection type ordering. I suspect that was because of the ad-hoc |
Not sure if you did a rebase, but doesn't look like it was pushed? |
9fe1f3c to
beeb48b
Compare
|
Hmm, looks like the problem is still there. It seems perhaps to not me non-deterministic but just platform specific? The tests pass locally on my machine. |
|
Hmm, that's unfortunate! I'm assuming this is in your court to look further into the cause of those platform-specific failures, when you have a chance, but ping if you could use another pair of eyes on it |
beeb48b to
87bb2f3
Compare
|
I rebased again as well as updated my local rustc version, and the test results have changed locally (not sure which of those caused it), so hopefully the issue is resolved. The diagnostics in cases where we perform multi-inference are not great -- the intersected types can be quite large, despite most of the elements typically being irrelevant. However, our current solution of reinference can lead to an assignable type being outputted as part of an invalid-assignment error, so I think we should still merge this and work on improving those diagnostics over time. |
87bb2f3 to
d209542
Compare
|
Yes, the test failures repro for me: |
|
Sorry, I had forgotten to commit the changes after the last rebase. CI should be passing now. |
carljm
left a comment
There was a problem hiding this comment.
Looks great! Spot-checked the ecosystem and I agree, in the vast majority of cases this gives a better diagnostic. Thank you!
* main: [ty] Implement `typing.override` (astral-sh#21627) [ty] Avoid expression reinference for diagnostics (astral-sh#21267) [ty] Improve autocomplete suppressions of keywords in variable bindings [ty] Only suggest completions based on text before the cursor Implement goto-definition and find-references for global/nonlocal statements (astral-sh#21616) [ty] Inlay Hint edit follow up (astral-sh#21621) [ty] Implement lsp support for string annotations (astral-sh#21577) [ty] Add 'remove unused ignore comment' code action (astral-sh#21582) [ty] Refactor `CheckSuppressionContext` to use `DiagnosticGuard` (astral-sh#21587) [ty] Improve several "Did you mean?" suggestions (astral-sh#21597) [ty] Add more and update existing projects in `ty_benchmark` (astral-sh#21536) [ty] fix ty playground initialization and vite optimization issues (astral-sh#21471)
Summary
We now use the type context for a lot of things, so re-inferring without type context actually makes diagnostics more confusing (in most cases).