feat(rome_analyze): emit syntax error diagnostics for suppression 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
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...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
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% |