Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

🐛 npx rome format prints lots of Unknown lint rule _______ in suppression comment #3406

Closed
1 task done
lgarron opened this issue Oct 12, 2022 · 2 comments · Fixed by #3412
Closed
1 task done
Assignees
Labels
A-Diagnostic Area: errors and diagnostics A-Linter Area: linter S-Bug: confirmed Status: report has been confirmed as a valid bug
Milestone

Comments

@lgarron
Copy link

lgarron commented Oct 12, 2022

Environment information

macOS 12.6
Rome CLI v0.10.1-next

What happened?

Repro steps:

shell
git clone https://github.com/cubing/cubing.js
cd cubing.js
git checkout 4b8891790c651de031d8a35dd7687f59db5e84e2
npm install
npx rome format --write ./script ./src

Output (truncated):

./src/sites/experiments.cubing.net/cubing.js/speechcubing/index.ts:39:5 suppressions/unknownRule ━━━

  ⚠ Unknown lint rule correctness/useTemplate in suppression comment

    37 │   const latestResult = event.results.item(event.results.length - 1);
    38 │   alternativeListElem.textContent =
  > 39 │     // rome-ignore lint(correctness/useTemplate): Using a template would make this more confusing.
       │     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    40 │     "Raw alternatives: " +
    41 │     Array.from(latestResult)


./src/cubing/search/instantiator.ts:17:3 suppressions/unknownRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ Unknown lint rule correctness/noAsyncPromiseExecutor in suppression comment

    16 │ export async function instantiateModuleWorker(): Promise<WorkerInsideAPI> {
  > 17 │   // rome-ignore lint(correctness/noAsyncPromiseExecutor): TODO
       │   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    18 │   return new Promise<WorkerInsideAPI>(async (resolve, reject) => {
    19 │     const timeoutID = setTimeout(() => {


./src/cubing/alg/parseAlg.ts:140:9 suppressions/unknownRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ Unknown lint rule correctness/noUnnecessaryContinue in suppression comment

    138 │           algStartIdx = this.#idx;
    139 │         }
  > 140 │         // rome-ignore lint(correctness/noUnnecessaryContinue): This line allows for more robust refactoring.
        │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    141 │         continue mainLoop;
    142 │       } else if (MOVE_START_REGEX.test(this.#input[this.#idx])) {


./src/cubing/alg/parseAlg.ts:149:9 suppressions/unknownRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ Unknown lint rule correctness/noUnnecessaryContinue in suppression comment

    147 │         algEndIdx = this.#idx;
    148 │
  > 149 │         // rome-ignore lint(correctness/noUnnecessaryContinue): This line allows for more robust refactoring.
        │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    150 │         continue mainLoop;
    151 │       } else if (this.tryConsumeNext("(")) {


./src/cubing/alg/parseAlg.ts:179:11 suppressions/unknownRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ Unknown lint rule correctness/noUnnecessaryContinue in suppression comment

    177 │           algEndIdx = this.#idx;
    178 │
  > 179 │           // rome-ignore lint(correctness/noUnnecessaryContinue): This line allows for more robust refactoring.
        │           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    180 │           continue mainLoop;
    181 │         } else {


./src/cubing/alg/parseAlg.ts:195:11 suppressions/unknownRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ Unknown lint rule correctness/noUnnecessaryContinue in suppression comment

    193 │           algEndIdx = this.#idx;
    194 │
  > 195 │           // rome-ignore lint(correctness/noUnnecessaryContinue): This line allows for more robust refactoring.
        │           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    196 │           continue mainLoop;
    197 │         }


./src/cubing/alg/parseAlg.ts:231:13 suppressions/unknownRule ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ Unknown lint rule correctness/noUnnecessaryContinue in suppression comment

    229 │             algEndIdx = this.#idx;
    230 │
  > 231 │             // rome-ignore lint(correctness/noUnnecessaryContinue): This line allows for more robust refactoring.
        │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    232 │             continue mainLoop;
    233 │           }

Expected result

No warnings about suppressed errors.

The VSCode extension seems to recognize these // rome-ignore comments as valid syntax, I'm not sure why the CLI doesn't.

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@lgarron lgarron added the S-To triage Status: user report of a possible bug that needs to be triaged label Oct 12, 2022
@lgarron lgarron changed the title 🐛 Formatter prints lots of Unknown lint rule_______ in suppression comment 🐛 npx rome format prints lots of Unknown lint rule_______ in suppression comment Oct 12, 2022
@lgarron lgarron changed the title 🐛 npx rome format prints lots of Unknown lint rule_______ in suppression comment 🐛 npx rome format prints lots of Unknown lint rule _______ in suppression comment Oct 12, 2022
lgarron added a commit to cubing/cubing.js that referenced this issue Oct 12, 2022
@ematipico ematipico added A-Linter Area: linter A-Diagnostic Area: errors and diagnostics labels Oct 12, 2022
@ematipico ematipico added this to the 10.0.0 milestone Oct 12, 2022
@ematipico ematipico added S-Bug: confirmed Status: report has been confirmed as a valid bug and removed S-To triage Status: user report of a possible bug that needs to be triaged labels Oct 12, 2022
@ematipico
Copy link
Contributor

I can reproduce the issue, even in our repository. @leops any idea what's the issue?

@leops
Copy link
Contributor

leops commented Oct 12, 2022

This is because the suppression comment parser is checking for the existence of the rule in the "dynamic registry" that's built for every call into the analyzer and only contains the rules enabled for this analysis pass. The formatter doesn't run the full analyzer, it only needs the syntax rules so the registry instance used for formatting doesn't contain any of the lint rules.
I think the fix would be to explicitly split off a "static registry" that would only contains the rule metadata for all the rules and would be used to check the suppressions or run lintdoc, and a "dynamic registry" that would be used to actually execute the rules but only contains the enabled rules. The downside I see to this is that we would lose the "unknown rule" diagnostic when the rule does exist but is disabled in the configuration, it would need to be reintroduced through some additional logic but that would be a good opportunity to improve the messaging for that diagnostic to better state that the rule is known, but the suppression comment is not doing anything because the rule is disabled for the whole project

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Diagnostic Area: errors and diagnostics A-Linter Area: linter S-Bug: confirmed Status: report has been confirmed as a valid bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants