-
Notifications
You must be signed in to change notification settings - Fork 640
Filter out plainJSErrors #930
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
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.
Pull Request Overview
This PR introduces filtering of semantic diagnostics for plain JavaScript files so that only a whitelist of allowed error codes is reported.
- Moves the initial collection of diagnostics earlier in
getSemanticDiagnosticsForFile
to apply filtering before other checks. - Adds a
plainJSErrors
set and applies it to filter diagnostics whenIsPlainJSFile
is true. - Introduces
IsPlainJSFile
helper and updates related utilities; updates test baselines to remove now-filtered JS errors.
Reviewed Changes
Copilot reviewed 117 out of 117 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
testdata/baselines/reference/...errors.txt | Removed all plain-JS-specific error baselines after filtering. |
internal/compiler/program.go | Moved diagnostics collection and added plain-JS filtering. |
internal/checker/utilities.go | Replaced local isPlainJSFile with call to new helper. |
internal/ast/utilities.go | Added IsPlainJSFile helper function. |
Comments suppressed due to low confidence (1)
internal/compiler/program.go:811
- [nitpick] The identifier
plainJSErrors
could be misleading. Consider renaming it to something likeallowedPlainJSDiagnostics
orplainJSWhitelist
to clarify that it's a whitelist of diagnostics to keep.
var plainJSErrors = core.NewSetFromItems(
--- old.esDecorators-classDeclaration-exportModifier.errors.txt | ||
+++ new.esDecorators-classDeclaration-exportModifier.errors.txt | ||
@@= skipped -0, +0 lines =@@ | ||
-file3.js(2,8): error TS1206: Decorators are not valid here. |
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.
No clue why these errors are dropped
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.
Thanks for getting to this. I'm a little sad to lose some errors in JS tests while I'm working on jsdoc but that's what type baselines are for.
Probably should delete that dupe before merging.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Will need another review now that this repo is seen as a production repo. |
Fixes #928
This really nerd sniped me, oops.
This isn't exactly the same; missing the idea of a "partial" check, and we do JSDoc differently, but it seems correct enough. Didn't know that we did this filtering.