Skip to content

fix(format): don't remove trailing comments when formatting is suppressed#9790

Merged
dyc3 merged 1 commit intomainfrom
dyc3/fix-9781
Apr 3, 2026
Merged

fix(format): don't remove trailing comments when formatting is suppressed#9790
dyc3 merged 1 commit intomainfrom
dyc3/fix-9781

Conversation

@dyc3
Copy link
Copy Markdown
Contributor

@dyc3 dyc3 commented Apr 3, 2026

Summary

Previously, we were throwing away some trivia after suppressed nodes. This PR fixes that.

Generated by gpt 5.4

fixes #9781

Test Plan

added snapshot tests

Docs

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 3, 2026

🦋 Changeset detected

Latest commit: 3f83bec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions github-actions Bot added A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS and super languages L-JSON Language: JSON and super languages L-HTML Language: HTML and super languages L-Grit Language: GritQL L-Yaml Language: Yaml labels Apr 3, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 3, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 196 skipped benchmarks1


Comparing dyc3/fix-9781 (3f83bec) with main (5085424)

Open in CodSpeed

Footnotes

  1. 196 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@dyc3 dyc3 marked this pull request as ready for review April 3, 2026 11:51
@dyc3 dyc3 requested review from a team April 3, 2026 11:51
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 3, 2026

Walkthrough

This change addresses a formatting issue where trailing comments following a top-level biome-ignore-all format suppression were being removed. The fix introduces logic to conditionally preserve outer trivia (including trailing comments) when formatting suppressed nodes that have no parent. The changes apply consistently across multiple formatter implementations—JavaScript, CSS, HTML, JSONC, GraphQL, Grit, and YAML—by adjusting how source ranges are computed and which text segments are used during verbatim formatting. Test fixtures have been added across all supported file types to verify the trailing-comment preservation behaviour.

Suggested reviewers

  • ematipico
  • denbezrukov
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: preventing removal of trailing comments when formatting is suppressed, which is the core objective of this PR.
Description check ✅ Passed The description explains the problem (trivia being discarded after suppressed nodes) and solution, directly related to the changeset's objectives.
Linked Issues check ✅ Passed The PR fixes issue #9781 by preventing removal of trailing comments when formatting is suppressed across all formatters (JS, CSS, HTML, JSON, GraphQL, Grit, YAML), which addresses the core problem of failed formatting with global ignore comments.
Out of Scope Changes check ✅ Passed All changes are scoped to the issue: modifications to verbatim formatting logic across multiple formatters to preserve trailing comments, plus corresponding test fixtures and a changeset entry.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dyc3/fix-9781

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/biome_grit_formatter/src/verbatim.rs (1)

24-41: ⚠️ Potential issue | 🟡 Minor

Rename to use consistent Grit naming.

The struct FormatGraphqlVerbatimNode and functions format_graphql_verbatim_node, format_bogus_node, and format_suppressed_node operate on GritSyntaxNode and GritFormatContext, but use GraphQL naming. These should be renamed to FormatGritVerbatimNode and format_grit_verbatim_node for consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_grit_formatter/src/verbatim.rs` around lines 24 - 41, The
identifiers use "Graphql" but should use "Grit" consistently: rename the struct
FormatGraphqlVerbatimNode to FormatGritVerbatimNode and the constructor function
format_graphql_verbatim_node to format_grit_verbatim_node; likewise rename any
related functions format_bogus_node and format_suppressed_node to use "grit" if
they currently contain "graphql". Update the impl block and its
Format<GritFormatContext> implementation to reference the new struct name, and
change any uses of these symbols (calls/constructors, type annotations, derives)
to the new names so they bind to GritSyntaxNode and GritFormatContext
consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@crates/biome_grit_formatter/src/verbatim.rs`:
- Around line 24-41: The identifiers use "Graphql" but should use "Grit"
consistently: rename the struct FormatGraphqlVerbatimNode to
FormatGritVerbatimNode and the constructor function format_graphql_verbatim_node
to format_grit_verbatim_node; likewise rename any related functions
format_bogus_node and format_suppressed_node to use "grit" if they currently
contain "graphql". Update the impl block and its Format<GritFormatContext>
implementation to reference the new struct name, and change any uses of these
symbols (calls/constructors, type annotations, derives) to the new names so they
bind to GritSyntaxNode and GritFormatContext consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 18b468b4-0690-458e-abfd-c4fb30471fef

📥 Commits

Reviewing files that changed from the base of the PR and between 53b8e57 and 3f83bec.

⛔ Files ignored due to path filters (13)
  • crates/biome_css_formatter/tests/specs/css/global_suppression.css.snap is excluded by !**/*.snap and included by **
  • crates/biome_css_formatter/tests/specs/css/suppression_trailing_comments.css.snap is excluded by !**/*.snap and included by **
  • crates/biome_graphql_formatter/tests/specs/graphql/suppression.graphql.snap is excluded by !**/*.snap and included by **
  • crates/biome_graphql_formatter/tests/specs/graphql/suppression_trailing_comments.graphql.snap is excluded by !**/*.snap and included by **
  • crates/biome_grit_formatter/tests/specs/grit/global_suppression.grit.snap is excluded by !**/*.snap and included by **
  • crates/biome_grit_formatter/tests/specs/grit/suppression_trailing_comments.grit.snap is excluded by !**/*.snap and included by **
  • crates/biome_html_formatter/tests/specs/html/suppressions/global_suppression.html.snap is excluded by !**/*.snap and included by **
  • crates/biome_html_formatter/tests/specs/html/suppressions/suppression_trailing_comments.html.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_formatter/tests/specs/js/module/global_suppression.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_formatter/tests/specs/js/module/suppression_trailing_comments.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_formatter/tests/specs/ts/suppressions_trailing_comments.ts.snap is excluded by !**/*.snap and included by **
  • crates/biome_json_formatter/tests/specs/json/global_suppression.jsonc.snap is excluded by !**/*.snap and included by **
  • crates/biome_json_formatter/tests/specs/json/suppression_trailing_comments.jsonc.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (21)
  • .changeset/itchy-hats-refuse.md
  • crates/biome_css_formatter/src/verbatim.rs
  • crates/biome_css_formatter/tests/specs/css/global_suppression.css
  • crates/biome_css_formatter/tests/specs/css/suppression_trailing_comments.css
  • crates/biome_graphql_formatter/src/verbatim.rs
  • crates/biome_graphql_formatter/tests/specs/graphql/suppression.graphql
  • crates/biome_graphql_formatter/tests/specs/graphql/suppression_trailing_comments.graphql
  • crates/biome_grit_formatter/src/verbatim.rs
  • crates/biome_grit_formatter/tests/specs/grit/global_suppression.grit
  • crates/biome_grit_formatter/tests/specs/grit/suppression_trailing_comments.grit
  • crates/biome_html_formatter/src/verbatim.rs
  • crates/biome_html_formatter/tests/specs/html/suppressions/global_suppression.html
  • crates/biome_html_formatter/tests/specs/html/suppressions/suppression_trailing_comments.html
  • crates/biome_js_formatter/src/verbatim.rs
  • crates/biome_js_formatter/tests/specs/js/module/global_suppression.js
  • crates/biome_js_formatter/tests/specs/js/module/suppression_trailing_comments.js
  • crates/biome_js_formatter/tests/specs/ts/suppressions_trailing_comments.ts
  • crates/biome_json_formatter/src/verbatim.rs
  • crates/biome_json_formatter/tests/specs/json/global_suppression.jsonc
  • crates/biome_json_formatter/tests/specs/json/suppression_trailing_comments.jsonc
  • crates/biome_yaml_formatter/src/verbatim.rs

@dyc3 dyc3 merged commit 67df09d into main Apr 3, 2026
28 checks passed
@dyc3 dyc3 deleted the dyc3/fix-9781 branch April 3, 2026 18:41
@github-actions github-actions Bot mentioned this pull request Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Formatter Area: formatter L-CSS Language: CSS and super languages L-Grit Language: GritQL L-HTML Language: HTML and super languages L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages L-Yaml Language: Yaml

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 [v2.4.10] format cli command tries (and fails) to format files with disabled formatting via biome ignore comment

2 participants