tools icon indicating copy to clipboard operation
tools copied to clipboard

feat(rome_analyze): emit syntax error diagnostics for suppression comments

Open leops opened this issue 3 years ago • 2 comments

Summary

Fixes #3841

With the existing suppression comment parsing logic, any comment that failed to parse as a suppression was simply ignored by the analyzer and formatter. I've modified the signature of the suppression parser so that any comment line that starts with rome-ignore (I've also changed this to be case insensitive and allow underscores so ROME_IGNORE would work too) is now detected as a suppression comment, and everything that comes after must parse correctly or a diagnostic will get emitted. I expect this change to improve the developer experience of using suppression comments since incorrectly written suppression will now emit an explicit syntax error instead of being silently ignored.

Test Plan

I've added additional tests for the newly introduced errors to both the suppression parser in rome_js_syntax and the overall analyzer infrastructure in rome_js_analyze

leops avatar Nov 24 '22 13:11 leops

Deploy Preview for docs-rometools ready!

Name Link
Latest commit 8925c0259c89729dc33a303ed60c0fc54c8e132d
Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/6384dc84b281660009c5e5a0
Deploy Preview https://deploy-preview-3847--docs-rometools.netlify.app/playground
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify[bot] avatar Nov 24 '22 13:11 netlify[bot]

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44936 44936 0
Failed 943 943 0
Panics 0 0 0
Coverage 97.94% 97.94% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 1757 1757 0
Failed 4189 4189 0
Panics 0 0 0
Coverage 29.55% 29.55% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12397 12397 0
Failed 3860 3860 0
Panics 0 0 0
Coverage 76.26% 76.26% 0.00%

github-actions[bot] avatar Nov 24 '22 13:11 github-actions[bot]