-
Notifications
You must be signed in to change notification settings - Fork 772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #16708 / Some errors are not reported if any of previous files contain errors #16719
Conversation
❗ Release notes required
|
If it's fairly recent, also #15840 comes to mind. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you find the reason why the check originally was where it was? I mean the fix can probably go in since everything is green, but I am still paranoid if this will not break or regress something else.
I did not. Git blame only lead to refactoring/cleanup commits and I was too lazy to dig deeper or set up a bisect script. The only side-effect of this change that comes to mind is that the diagnostics will potentially be in different order. But I don't think you can say one ordering is better than the other. File related diagnostics will still be in the same order as before, just whatever else might be at the end now. And if this breaks something that we don't have tests for then we can fix it and add those tests when it comes up 🤷 |
Description
Fixes #16708
I don't know what originally caused this, but here's a proposed fix. We move the playback of background diagnostics to after we do a type check, so that previous errors don't trigger the suppression check for cascading errors.
Checklist
Performance benchmarks added in case of performance changes