This repository has been archived by the owner on Aug 31, 2023. It is now read-only.
feat(rome_analyze): add a warning for unused suppression comments #3718
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for docs-rometools canceled.
|
|
!bench_analyzer |
MichaReiser
reviewed
Nov 14, 2022
Analyzer Benchmark Results |
MichaReiser
approved these changes
Nov 14, 2022
jeysal
added a commit
to jeysal/rometools
that referenced
this pull request
Nov 14, 2022
* upstream/main: (45 commits) website(docs): set `color-scheme` on the root element (rome#3721) feat(rome_analyze): add a warning for unused suppression comments (rome#3718) feat(rome_js_analyze): Implement prefer-numeric-literals lint (rome#3558) feat(rome_js_formatter): jestEach template literals rome#3308 (rome#3582) doc(website): Add context about Romes philosophy (rome#3714) fix(rome_js_formatter): Single-line comment below a JSX prop triggers… (rome#3641) test(rome_js_formatter): update prettier tests (rome#3684) fix(rome_js_parser): improve await handling in non-async context (rome#3573) fix(rome_js_parser): improve yield parsing in non generator function (rome#3622) More playground polish Fix backgrounds Fix height Align docs.rome.tools with rome.tools Reenable compression Add missing width website(docs): More playground IDE features (rome#3711) fix(rome_js_formatter): new expression attribute (rome#3686) docs(website): added checkbox to toggle linter in playground (rome#3699) website(docs): More website tweaks (rome#3707) website(docs): Add default layout property (rome#3705) ...
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3655
This PR tracks whether any signal was suppressed by a suppression comment. Once all analyzer passes have completed, a warning is emitted for all line suppressions that never actually recorder any hit. While the change is mostly straightforward, it still requires some special handling in
pull_diagnosticsto ensure we don't report unused suppression diagnostics in syntax-only passes of the analyzer (were all lint rules are disabled, and thus suppression comments are never hit)Test Plan
I updated the existing suppression tests to ensure the new diagnostic is correctly emitted, as well as the
SuppressionCommentssnapshot test for thenoUnreachablerule that was also impacted by this change (since it has a negative case to check that certain comment positions do not suppress the rule)