Collate-free import sorting#4349
Conversation
| throw new Error(`Unsupported value for organizeImportsIgnoreCase: ${propValue.getText()}`); | ||
| } | ||
| } | ||
| else if (propName === "organizeImportsSort") { |
There was a problem hiding this comment.
You'll note that no tests change behavior with this mapping; no manual test changes etc etc. This gives me hope!
| caseInsensitiveOrganizeImportsComparer[0], | ||
| caseSensitiveOrganizeImportsComparer[0], | ||
| } | ||
| "golang.org/x/text/unicode/norm" |
There was a problem hiding this comment.
This is here solely to split letters from their accents for case insensitive sorting. We do test this.
Unfortunately, this is more tables internally, also out of date.
There was a problem hiding this comment.
Pull request overview
This PR replaces the existing import-sorting implementation that depended on x/text/collate with a collate-free approach, introducing a new OrganizeImportsSort preset enum and wiring it through LSP preferences, tests, and fourslash fixtures/baselines.
Changes:
- Added
OrganizeImportsSortpreference (including parsing/serialization) and updated config/baselines to emit"sort": "auto". - Reimplemented organize-import string comparison without
collate, including natural/numeric/diacritic-aware comparisons and updated sorting/detection logic. - Updated LSP/session tests and fourslash tests (and conversion script) to use the new
organizeImportsSortpreference.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/baselines/reference/fourslash/state/codeLensAcrossProjects.baseline | Baseline update to include new organizeImports.sort field. |
| internal/project/session_test.go | Updates session preference parsing expectations to use organizeImportsSort. |
| internal/ls/organizeimports.go | Integrates resolved sort mode into organize-imports comparer selection/detection. |
| internal/ls/lsutil/utilities_test.go | Adds tests for sort resolution and natural string comparison behavior. |
| internal/ls/lsutil/userpreferences.go | Adds OrganizeImportsSort enum + config plumbing; refactors raw-field config handling. |
| internal/ls/lsutil/userpreferences_test.go | Extends preference parsing tests for old/new organize-import settings. |
| internal/ls/lsutil/organizeimports.go | Removes collate-based comparer; adds collate-free comparison and detection updates. |
| internal/fourslash/tests/organizeImports_sortModuleSpecifiers_test.go | Switches fourslash preferences to OrganizeImportsSortOrdinalIgnoreCase. |
| internal/fourslash/tests/organizeImports_coalesceImports_test.go | Switches fourslash preferences to OrganizeImportsSortOrdinalIgnoreCase. |
| internal/fourslash/tests/organizeImports_coalesceExports_test.go | Switches fourslash preferences to OrganizeImportsSortOrdinalIgnoreCase. |
| internal/fourslash/_scripts/convertFourslash.mts | Teaches converter to map organizeImportsSort into Go preferences. |
|
|
||
| fullCollator := collate.New(tag, opts...) | ||
| if !accents { | ||
| return strings.Compare(b, a) |
There was a problem hiding this comment.
Strada actually did this wrong, which is terrible
We currently import
x/text/collateto implement import sorting. But, this has cost:While I think I can fix the latter with a CL stack I've been trying, the former is just unfixable.
I do not think anyone out there is actually writing non-ASCII imports, but the case sensitivity and "natural" sorting is useful. It turns out you can get really close to the old behavior with just the stdlib and
x/text/normfor accent normalization.My proposal is this:
As mentioned before, this drops 1.3MB out of our binary.
The new options are:
Which do what they say on the tin.
In terms of which tools are closest to which option:
eslint-plugin-simple-import-sort:naturalIgnoreCase.dprint:ordinalorordinalIgnoreCase.sort-imports:ordinalorordinalIgnoreCase.eslint-plugin-import/order:ordinalorordinalIgnoreCase.@trivago/prettier-plugin-sort-imports/@ianvs/prettier-plugin-sort-imports:naturalornaturalIgnoreCasebiome:naturalorordinaloxfmt:naturalornaturalIgnoreCaseeslint-plugin-perfectionist/sort-imports: innaturalmode,naturalornaturalIgnoreCase(plus a mode that is more likeordinalIgnoreCasebut it's actually system locale dependent, ouch)