refactor(rome_analyze): align the syntax of lint suppression with the corresponding diagnostic category
Summary
Fixes #3446
This PR changes the syntax of lint suppression comments from rome-ignore lint(group/rule) to rome-ignore lint/group/rule to align with how lint categories are presented in diagnostics. I've tried to keep backwards compatibility by continuing to parse the previous syntax, and emitting a diagnostic with a safe fix when such a comment is encountered by the linter.
Test Plan
I've extended the suppression comment tests in rome_js_analyze to check that both the new and old syntax remain supported, and that a diagnostic is correctly emitted for the deprecated syntax. Additionally I've also used a local build of the language server to apply the automatic fix to all the files with suppression comments that we have.
Deploy Preview for docs-rometools ready!
| Name | Link |
|---|---|
| Latest commit | da9e60c2ad0a5704c1fcc1abf93b3674bbbf2fd6 |
| Latest deploy log | https://app.netlify.com/sites/docs-rometools/deploys/637b815484f9fb00087ac96c |
| Deploy Preview | https://deploy-preview-3788--docs-rometools.netlify.app |
| 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% |
Also, can these warnings block
rome ci? If yes, we should demote them to simple info diagnostics. Otherwise, I think we can merge it!
Warnings should not cause rome ci to fail, only error-level diagnostics will make the CLI return with a non-zero error code