Ignore related info in diagnostic deduplication - #50309
Conversation
TypeScript Bot (typescript-bot)
added
Author: Team
For Backlog Bug
|
TypeScript Bot (@typescript-bot) pack this |
|
Heya Andrew Branch (@andrewbranch), I've started to run the tarball bundle task on this PR at 95736c0. You can monitor the build here. |
|
Hey Andrew Branch (@andrewbranch), I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
Sheetal Nandi (sheetalkamat)
left a comment
Sheetal Nandi (sheetalkamat)
left a comment
There was a problem hiding this comment.
My understanding here is that lookup we were skipping related information for binary search (checker uses this to get diagnostic and attach related information to existing or new diagnostic) But while inserting we used diagnostic with related information to determine sorting order and that messed with lookup i think which resulted in duplicate errors in your scenario..
This change makes sense.
|
I wonder if this solves #49437 as well. |
|
Andrew Branch (@andrewbranch) Sheetal Nandi (@sheetalkamat) I figured out how that weird issue I filed manifested itself and described it on #49437 (comment) Andrew Branch (@andrewbranch) can you add something like the following test case to see if it passes now? Today it fails in TypeScript. /// <reference path="./fourslash.ts" />
////let lololol = 123;
////
////lololo/**/
verify.encodedSemanticClassificationsLength("2020", 3);
verify.numberOfErrorsInCurrentFile(1); |
|
Daniel Rosenwasser (@DanielRosenwasser) it passes. |
TypeScript Bot (typescript-bot)
added
the
For Milestone Bug
|
CI failure was in main and fixed in #50320 |
4 participants
Probably fixes #50243 but I can’t reproduce it. I noticed this first in #50088 (review) and worked around it. I think there may be more than one place we deduplicate diagnostics which makes this not noticeable on the CLI or in error baselines—the two baselines that changed actually had materially different related info, but I still think it’s better to deduplicate. In the fourslash test that changed, however, the language service actually had two identical copies
of each diagnostic.
Also fixes #49437