Skip to content

fix(biome_analyze): stop squashing multiple line suppression comments.#6650

Merged
ematipico merged 15 commits intobiomejs:mainfrom
sterliakov:bugfix/gh-6621-rule-squashing-v2
Jul 4, 2025
Merged

fix(biome_analyze): stop squashing multiple line suppression comments.#6650
ematipico merged 15 commits intobiomejs:mainfrom
sterliakov:bugfix/gh-6621-rule-squashing-v2

Conversation

@sterliakov
Copy link
Copy Markdown
Contributor

@sterliakov sterliakov commented Jun 30, 2025

Summary

Fixes #6621. Alternative to #6649.

Stop squashing multiple line suppression comments. Squashing has an inconvenient side effect of breaking unused comments check: if any of those comments was used, the whole block was marked as such. This PR removes the comment squashing logic and keeps all inline comments separate.

However, this PR is bigger in scope, and I might not be sufficiently familiar with biome codebase to make the right choice here. Feel free to close if that's the case, #6649 should be enough to patch the immediate hole.

Test Plan

Added a new test category that supports enabling multiple rules inline. Such tests must start with one or more lines of the shape

///! lint/{group}/{rule}

Added a testfile with multiple inline suppression comments. If the core idea of this PR is approved, I'll add more testcases covering non-jsx sources (that probably shouldn't matter, but there are too few tests for ignore comments, let's add more).

@github-actions github-actions Bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Jun 30, 2025
@sterliakov sterliakov changed the title Stop squashing multiple line suppression comments. fix(linter): stop squashing multiple line suppression comments. Jun 30, 2025
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 30, 2025

⚠️ No Changeset found

Latest commit: 96cd3ce

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@sterliakov sterliakov force-pushed the bugfix/gh-6621-rule-squashing-v2 branch from ec02e2d to f10a17d Compare June 30, 2025 21:39
@sterliakov sterliakov force-pushed the bugfix/gh-6621-rule-squashing-v2 branch from f10a17d to 96cd3ce Compare June 30, 2025 21:44
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 30, 2025

CodSpeed Performance Report

Merging #6650 will not alter performance

Comparing sterliakov:bugfix/gh-6621-rule-squashing-v2 (128f3bd) with main (cdd6e17)

Summary

✅ 114 untouched benchmarks

Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this solution is better, because there's a clear function that is charge for resolving overlapped suppresssions. I don't see any regressions, so I think I'm more keen to accept this PR

Can you add the changeset? https://github.com/biomejs/biome/blob/main/CONTRIBUTING.md#create-a-changeset

(sorry I only realised now that this fix was the alternative to the other one)

@sterliakov
Copy link
Copy Markdown
Contributor Author

sterliakov commented Jul 1, 2025

Great, thanks! I'll push several more tests and a changeset today.

@sterliakov sterliakov changed the title fix(linter): stop squashing multiple line suppression comments. fix(biome_analyze): stop squashing multiple line suppression comments. Jul 2, 2025
@ematipico ematipico merged commit 19aab18 into biomejs:main Jul 4, 2025
28 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

💅 Squashing suppression comments causes false negatives

2 participants