Skip to content

fix(formatter/sort_imports): Handle alignable_comment correctly#17646

Merged
graphite-app[bot] merged 1 commit intomainfrom
01-05-fix_formatter_sort_imports_handle_alignable_comment_correctly
Jan 5, 2026
Merged

fix(formatter/sort_imports): Handle alignable_comment correctly#17646
graphite-app[bot] merged 1 commit intomainfrom
01-05-fix_formatter_sort_imports_handle_alignable_comment_correctly

Conversation

@leaysgur
Copy link
Member

@leaysgur leaysgur commented Jan 5, 2026

Fixes #17467

See #17467 (comment) for details.

Copilot AI review requested due to automatic review settings January 5, 2026 01:21
@leaysgur leaysgur requested a review from Dunqing as a code owner January 5, 2026 01:21
@github-actions github-actions bot added A-formatter Area - Formatter C-bug Category - Bug labels Jan 5, 2026
Copy link
Member Author

leaysgur commented Jan 5, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the handling of alignable multiline block comments in the import sorting feature. Alignable comments (those where each line starts with *) are formatted by the formatter as multiple FormatElement::Text nodes separated by line breaks. The sort_imports transform now correctly treats these multi-element comments as single logical units when determining whether they should move with their associated import statements or remain standalone.

Key changes:

  • Added state tracking to detect when processing elements that are part of a multiline alignable comment
  • Modified line flushing logic to keep multiline comment elements together and classify them appropriately
  • Added comprehensive test coverage for various multiline comment scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/oxc_formatter/src/ir_transform/sort_imports/mod.rs Implements multiline comment boundary detection and ensures alignable comments spanning multiple FormatElement lines are treated as atomic units during import sorting
crates/oxc_formatter/tests/ir_transform/sort_imports.rs Adds comprehensive test cases covering multiline comments attached to imports, consecutive comments, partitioned comments, and mixed single/multiline scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 5, 2026

CodSpeed Performance Report

Merging #17646 will not alter performance

Comparing 01-05-fix_formatter_sort_imports_handle_alignable_comment_correctly (9e1552d) with main (f0fdf83)

Summary

✅ 38 untouched
⏩ 7 skipped1

Footnotes

  1. 7 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.

@leaysgur leaysgur force-pushed the 01-05-fix_formatter_sort_imports_handle_alignable_comment_correctly branch from 59b78d4 to 9e1552d Compare January 5, 2026 01:37
@Dunqing
Copy link
Member

Dunqing commented Jan 5, 2026

I am thinking about whether we can improve the formatting of alignable comments to avoid this nicely.

@leaysgur leaysgur marked this pull request as draft January 5, 2026 07:54
@Dunqing
Copy link
Member

Dunqing commented Jan 5, 2026

I am thinking about whether we can improve the formatting of alignable comments to avoid this nicely.

Checked, we can't. It has to print a hard_line_break element

@Dunqing Dunqing marked this pull request as ready for review January 5, 2026 08:38
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Jan 5, 2026
Copy link
Member

Dunqing commented Jan 5, 2026

Merge activity

@graphite-app graphite-app bot force-pushed the 01-05-fix_formatter_sort_imports_handle_alignable_comment_correctly branch from 9e1552d to c9b5d7d Compare January 5, 2026 08:38
@graphite-app graphite-app bot merged commit c9b5d7d into main Jan 5, 2026
20 checks passed
@graphite-app graphite-app bot deleted the 01-05-fix_formatter_sort_imports_handle_alignable_comment_correctly branch January 5, 2026 08:45
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 5, 2026
graphite-app bot pushed a commit that referenced this pull request Jan 5, 2026
# Oxlint
### 💥 BREAKING CHANGES

- f7da875 oxlint: [**BREAKING**] Remove oxc_language_server binary (#17457) (Boshen)

### 🚀 Features

- 659c23e linter: Init note field boilerplate  (#17589) (Shrey Sudhir)
- 6870b64 parser: Add TS1363 error code (#17609) (Sysix)
- 6154c8c linter/eslint-plugin-vitest: Implemented vitest/warn-todo rule (#17228) (Said Atrahouch)
- 0043cd6 linter/eslint-plugin-vitest: Implement consistent-vitest-vi rule (#17389) (Said Atrahouch)
- a6d773d linter: Add full TS support to eslint/no-useless-constructor (#17592) (camc314)
- f02c0e7 linter/eslint: Implement complexity (#17569) (Nguyen Tran)
- bc7aae7 linter/no-unused-vars: Add fixer to remove unused catch bindings (#17567) (Don Isaac)
- 9e8ec78 linter/only-throw-error rule: Add `allowRethrowing` option for  (#17554) (camc314)
- b67e819 linter: Add fixer for `unicorn/prefer-response-static-json` rule (#17559) (Mikhail Baev)
- 44b0361 linter/vue: Implement no-this-in-before-route-enter (#17525) (yefan)
- ee34716 linter/react: Implement no-will-update-set-state (#17530) (Kenzo Wada)
- 3088e1d linter/react: Implement no-this-in-sfc (#17535) (Kenzo Wada)
- 29a2868 linter/jsx-a11y: Implement no-static-element-interactions (#17538) (Kenzo Wada)
- eadf057 linter: Enable tsconfig auto discovery by default (#17489) (Boshen)
- 12a7d6e website_linter: Add a count of rules with fixes available to rules table. (#17476) (Connor Shea)

### 🐛 Bug Fixes

- a702f13 oxlint/lsp: Correct position for "disable for this file" with shebang (#17613) (Sysix)
- 19fdfb6 linter: Panic in `sort-keys` rule with Unicode numeric characters (#17629) (Adel Rodríguez)
- 2e8f469 vscode: Search for `node_modules/.bin/oxlint.exe` too (bun setup) (#17597) (Sysix)
- be39906 linter/aria-proptypes: Allow template literals with expressions for string-type ARIA props (#17460) (Jökull Sólberg Auðunsson)
- 529901c linter: Include JS plugin rules when calculating total rule count (#17520) (connorshea)
- 96ef2cc linter: Print total rule # when using a single nested config (#17517) (connorshea)
- 9ad0f29 oxlint: Do not enable external plugin store when no external linter is passed (#17498) (Sysix)
- 174375d oxfmt,oxlint: Disable mimalloc for 32-bit Arm targets (#17473) (Yaksh Bariya)
- ff70fe9 linter/no-standalone-expect: Allows expect in wrapper functions passed to test blocks (#17427) (Copilot)
- dab232f linter/catch-or-return: Handle arrow functions with implicit returns correctly (#17440) (Copilot)
- a38892a linter: Update no-unnecessary-template-expression docs and test case (#17453) (camc314)

### ⚡ Performance

- 605dbf1 vscode: Restrict searching for oxlint/oxfmt binaries only 3 levels deep + 10s timeout (#17345) (Sysix)

### 📚 Documentation

- 884fb63 linter/react: Improve docs for jsx-curly-brace-presence (#17579) (connorshea)
- 1d3ee07 linter: Improve rule explanation for `vue/no-this-in-before-route-enter`. (#17581) (connorshea)
- 5f189f8 linter/arrow-body-style: Correctly document default mode option (#17566) (Rägnar O'ock)
- bb2e8e4 linter: Add a note to the `typescript/no-var-requires` rule about the missing `allow` option (#17551) (connorshea)
- 655afc1 linter: Improve docs for `import/extensions` and add a few more tests (#17539) (connorshea)
- 7e5fc90 linter: Update list of plugins that are reserved. (#17516) (connorshea)
# Oxfmt
### 💥 BREAKING CHANGES

- f7da875 oxlint: [**BREAKING**] Remove oxc_language_server binary (#17457) (Boshen)

### 🚀 Features

- 8fd4ea9 oxfmt: `options.embeddedLanguageFormatting` is now `"auto"` by default (#17649) (leaysgur)

### 🐛 Bug Fixes

- c9b5d7d formatter/sort_imports: Handle alignable_comment correctly (#17646) (leaysgur)
- 453222d formatter: Missing comment handling for end-of-line comments in member chains (#17659) (Dunqing)
- 0805ff2 formatter: Incorrect inline comment placement in try-catch (#17657) (Dunqing)
- 3a0c782 formatter: Don't move comments into optional call parentheses (#17582) (magic-akari)
- 174375d oxfmt,oxlint: Disable mimalloc for 32-bit Arm targets (#17473) (Yaksh Bariya)

### ⚡ Performance

- abb28dc oxfmt: Turn of pretty print from sort-package-json (#17452) (Boshen)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-formatter Area - Formatter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

formatter: type import moved inside multiline header comment

3 participants