refactor: various renaming/chore #620
Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRefactors per-group options computation across many sorting rules by introducing an options-by-group-index computer, centralizes string formatting, tightens import-sorting option types/defaults, and adds side-effect group helpers and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Rule as ESLint Rule
participant Builder as buildDefaultOptionsByGroupIndexComputer
participant Grouper as sortNodesByGroups
participant Filter as isNodeIgnoredForGroup
participant Sort as sortNodes
Rule->>Builder: initialize with global options
Builder-->>Grouper: returns (groupIndex) -> { options, nodeValueGetter, fallbackSortNodeValueGetter }
loop for each groupIndex
Grouper->>Builder: call(groupIndex)
Builder-->>Grouper: group-specific options & getters
Grouper->>Filter: call({ node, groupOptions, groupIndex })
Filter-->>Grouper: boolean (ignore?)
Grouper->>Sort: sort subset with group options & getters
Sort-->>Grouper: sorted subset
end
Grouper-->>Rule: merged sorted result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (2 warnings, 1 inconclusive)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #620 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 111 113 +2
Lines 9098 9101 +3
Branches 1747 1743 -4
=========================================
+ Hits 9098 9101 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hugop95
changed the title
hugop95
force-pushed
the
refactor/sorting-1
branch
from
45a533a to
196605d
Compare
- It's directly used in `sort-imports`.
- We can simply use `isNodeIgnoredForGroup` rather than transforming `type` to `unsorted`.
- It prevents some annoying compilation error due to types sometimes.
- And add a missing one.
…uter` - Complex functions make more sense with the `compute` wording than the simple `get`.
- To `buildDefaultOptionsByGroupIndexComputer`.
- We'll use it later.
hugop95
force-pushed
the
refactor/sorting-1
branch
from
196605d to
dfc6650
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
utils/build-default-options-by-group-index-computer.ts (1)
77-105: Custom groupfallbackSort.orderfallback logic is currently brokenIn
getCustomGroupsCompareOptions, thecustomGroupbranch does:fallbackSort = { type: customGroup.fallbackSort?.type ?? fallbackSort.type, } let fallbackOrder = customGroup.fallbackSort?.order ?? fallbackSort.order if (fallbackOrder) { fallbackSort.order = fallbackOrder }Because
fallbackSortis reassigned before computingfallbackOrder,fallbackSort.orderin the??expression refers to the new object, not the original global fallback sort. As a result, when the custom group does not specifyfallbackSort.order, you lose the originaloptions.fallbackSort.orderinstead of inheriting it.You can fix this by computing the new type and order before reassigning
fallbackSort:if (customGroup) { - fallbackSort = { - type: customGroup.fallbackSort?.type ?? fallbackSort.type, - } - let fallbackOrder = customGroup.fallbackSort?.order ?? fallbackSort.order - if (fallbackOrder) { - fallbackSort.order = fallbackOrder - } + let nextFallbackType = + customGroup.fallbackSort?.type ?? fallbackSort.type + let fallbackOrder = + customGroup.fallbackSort?.order ?? fallbackSort.order + + fallbackSort = { type: nextFallbackType } + if (fallbackOrder) { + fallbackSort.order = fallbackOrder + } order = customGroup.order ?? order type = customGroup.type ?? type }This preserves the previous fallback order when the custom group does not override it, while still honoring any explicit
fallbackSort.orderon the custom group.
🧹 Nitpick comments (6)
rules/sort-object-types.ts (1)
48-48: Per-group options computer and ignore logic look correct; consider a small guard tweak.
- Using
optionsByGroupIndexComputerto merge the baseoptionswithgetCustomGroupsCompareOptionsoutput (includingnodeValueGetterandfallbackSortNodeValueGetter) brings this rule in line with the shared group-sorting framework.isNodeIgnoredForGroupnow cleanly separates behavior bygroupOptions.sortByand theUnreachableCaseErrordefault provides an exhaustive guard for futuresortByvariants.If you ever allow empty-string
values, you might want to tighten the'value'branch to checknode.value === nullinstead of general falsiness, but with the currentvaluepopulation that’s only a theoretical edge.Also applies to: 310-346
rules/sort-maps.ts (1)
22-23: Usage ofbuildDefaultOptionsByGroupIndexComputerwithsortNodesByGroupslooks correctThe new wiring to
optionsByGroupIndexComputer: buildDefaultOptionsByGroupIndexComputer(options)matches the updated API and keeps all per‑group overrides centralized. Givenoptionsis already the fully completed rule config, this should preserve existing behavior.If you want to micro‑optimize, you could compute the
optionsByGroupIndexComputeronce peroptionsrather than recreating it inside eachcreateSortNodesExcludingEslintDisabledinvocation, but that’s purely optional.Also applies to: 173-179
rules/sort-imports/is-side-effect-only-group.ts (1)
3-4: Safe handling ofundefinedgroup plus helper reuse looks goodThe change to accept
GroupsOptions<string>[0] | undefinedand early‑returnfalseprevents crashes whenoptions.groups[groupIndex]is out of bounds and nicely reuses the sharedisStringGroupSideEffectOnlyGrouphelper.As a tiny refinement, you might prefer an explicit check like
if (group === undefined)instead ofif (!group)to avoid ever treating an empty string group name as “no group”, should that configuration ever appear.Also applies to: 17-21
rules/sort-imports.ts (1)
312-320: Side‑effect group ignoring and per‑group options wiring are consistent with the new API
isNodeIgnoredForGroupnow delegates toisSideEffectOnlyGroup(options.groups[groupIndex])whensortSideEffectsis false, and always returnsfalsewhensortSideEffectsis true. That cleanly centralizes the “skip pure side‑effect groups” behavior and meshes with the updatedisSideEffectOnlyGroupsignature that acceptsundefined.optionsByGroupIndexComputer: buildDefaultOptionsByGroupIndexComputer(options)matches the newsortNodesByGroupscontract and ensures custom group overrides are applied per group without duplicating logic here.- The new
getNodeNamehelper correctly handles:
ImportDeclarationvianode.source.value.TSImportEqualsDeclarationvia either external module reference or printed qualified reference.require()variable declarations based on the literal argument, relying on theVariableDeclarationvisitor’s guard to make the cast safe.If desired, you could reduce the
as TSESTree.CallExpression/as TSESTree.Literalcasts by narrowing onnode.typeplus a small runtime assertion, but that’s optional polish.Also applies to: 511-536, 538-548
test/utils/sort-nodes-by-groups.test.ts (1)
3-22: OptionsByGroupIndexComputer wiring and CommonOptions usage look consistentThe switch to
CommonOptionsplus the sharedoptionsByGroupIndexComputerdefault (() => ({ options })) matches the new API surface fromutils/sort-nodes-by-groupsand keeps the tests straightforward. Type-wise this is sound: all call sites that just need default options can rely on this single function, and tests that need overrides redefine it locally as needed.Minor nit: since
optionsis already explicitly typed asCommonOptions, theas conston the initializer is redundant and could be dropped for slightly clearer intent, but it’s not functionally problematic.utils/sort-nodes-by-groups.ts (1)
130-158: Consider avoidingtoSortedfor broader runtime compatibilityThe change to:
for (let groupIndexString of Object.keys(nodesByNonIgnoredGroupIndex).toSorted( (a, b) => Number(a) - Number(b), )) { let groupIndex = Number(groupIndexString) let { fallbackSortNodeValueGetter, nodeValueGetter, options } = optionsByGroupIndexComputer(groupIndex) // ... }is behaviorally fine (explicitly sorts group indices numerically), but
Array.prototype.toSortedis still relatively new and not available in older Node versions without polyfills/transpilation to.sort.If this plugin is expected to run in environments that may not yet support
toSorted, consider a more widely compatible pattern:for (let groupIndexString of Object.keys(nodesByNonIgnoredGroupIndex).sort( (a, b) => Number(a) - Number(b), )) { // same body }This preserves the intended ordering without depending on newer built-ins.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
docs/content/rules/sort-imports.mdx(2 hunks)rules/sort-array-includes.ts(2 hunks)rules/sort-classes.ts(2 hunks)rules/sort-decorators.ts(2 hunks)rules/sort-enums.ts(3 hunks)rules/sort-enums/types.ts(2 hunks)rules/sort-export-attributes.ts(2 hunks)rules/sort-exports.ts(2 hunks)rules/sort-heritage-clauses.ts(2 hunks)rules/sort-import-attributes.ts(2 hunks)rules/sort-imports.ts(7 hunks)rules/sort-imports/is-side-effect-only-group.ts(2 hunks)rules/sort-imports/is-string-group-side-effect-only-group.ts(1 hunks)rules/sort-imports/types.ts(2 hunks)rules/sort-imports/validate-side-effects-configuration.ts(2 hunks)rules/sort-jsx-props.ts(2 hunks)rules/sort-maps.ts(2 hunks)rules/sort-modules.ts(2 hunks)rules/sort-named-exports.ts(2 hunks)rules/sort-named-imports.ts(2 hunks)rules/sort-object-types.ts(3 hunks)rules/sort-object-types/build-node-value-getter.ts(1 hunks)rules/sort-object-types/get-custom-groups-compare-options.ts(1 hunks)rules/sort-objects.ts(2 hunks)rules/sort-switch-case.ts(1 hunks)rules/sort-union-types.ts(2 hunks)rules/sort-variable-declarations.ts(3 hunks)test/rules/sort-imports/is-side-effect-only-group.test.ts(1 hunks)test/rules/sort-imports/is-string-group-side-effect-only-group.test.ts(1 hunks)test/rules/sort-imports/validate-side-effects-configuration.test.ts(1 hunks)test/utils/compare/build-string-formatter.test.ts(1 hunks)test/utils/compare/compare.test.ts(1 hunks)test/utils/get-custom-groups-compare-options.test.ts(1 hunks)test/utils/sort-nodes-by-groups.test.ts(9 hunks)utils/build-default-options-by-group-index-computer.ts(2 hunks)utils/compare/build-string-formatter.ts(1 hunks)utils/compare/compare.ts(6 hunks)utils/sort-nodes-by-groups.ts(5 hunks)utils/sort-nodes.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (30)
test/rules/sort-imports/is-side-effect-only-group.test.ts (1)
rules/sort-imports/is-side-effect-only-group.ts (1)
isSideEffectOnlyGroup(16-29)
test/utils/compare/build-string-formatter.test.ts (1)
utils/compare/build-string-formatter.ts (1)
buildStringFormatter(26-59)
test/rules/sort-imports/validate-side-effects-configuration.test.ts (1)
rules/sort-imports/validate-side-effects-configuration.ts (1)
validateSideEffectsConfiguration(21-46)
utils/compare/build-string-formatter.ts (2)
types/common-options.ts (1)
SpecialCharactersOption(227-244)utils/unreachable-case-error.ts (1)
UnreachableCaseError(31-43)
rules/sort-modules.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-array-includes.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-objects.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-union-types.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-imports/is-side-effect-only-group.ts (1)
types/common-options.ts (1)
GroupsOptions(436-441)
rules/sort-named-exports.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-decorators.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-import-attributes.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
utils/build-default-options-by-group-index-computer.ts (1)
types/common-options.ts (1)
CommonOptions(23-73)
utils/compare/compare.ts (2)
types/common-options.ts (1)
CommonOptions(23-73)utils/compare/build-string-formatter.ts (1)
buildStringFormatter(26-59)
rules/sort-exports.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-export-attributes.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
test/rules/sort-imports/is-string-group-side-effect-only-group.test.ts (1)
rules/sort-imports/is-string-group-side-effect-only-group.ts (1)
isStringGroupSideEffectOnlyGroup(1-3)
rules/sort-object-types.ts (1)
utils/unreachable-case-error.ts (1)
UnreachableCaseError(31-43)
rules/sort-classes.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-imports/validate-side-effects-configuration.ts (3)
utils/compute-groups-names.ts (1)
computeGroupsNames(13-15)rules/sort-imports/is-side-effect-only-group.ts (1)
isSideEffectOnlyGroup(16-29)rules/sort-imports/is-string-group-side-effect-only-group.ts (1)
isStringGroupSideEffectOnlyGroup(1-3)
utils/sort-nodes.ts (1)
types/common-options.ts (1)
CommonOptions(23-73)
rules/sort-maps.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-enums/types.ts (1)
utils/sort-nodes-by-dependencies.ts (1)
SortingNodeWithDependencies(16-21)
test/utils/sort-nodes-by-groups.test.ts (3)
types/common-options.ts (1)
CommonOptions(23-73)utils/sort-nodes-by-groups.ts (1)
OptionsByGroupIndexComputer(8-15)types/sorting-node.ts (1)
SortingNode(18-102)
rules/sort-named-imports.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-heritage-clauses.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-imports.ts (4)
rules/sort-imports/types.ts (1)
Options(22-69)rules/sort-imports/read-closest-ts-config-by-path.ts (1)
readClosestTsConfigByPath(58-99)rules/sort-imports/is-side-effect-only-group.ts (1)
isSideEffectOnlyGroup(16-29)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
utils/sort-nodes-by-groups.ts (3)
types/common-options.ts (1)
CommonOptions(23-73)types/sorting-node.ts (1)
SortingNode(18-102)utils/compare/compare.ts (1)
NodeValueGetterFunction(17-17)
rules/sort-variable-declarations.ts (3)
utils/validate-generated-groups-configuration.ts (1)
validateGeneratedGroupsConfiguration(75-93)rules/sort-variable-declarations/types.ts (1)
allSelectors(80-80)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
rules/sort-jsx-props.ts (1)
utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(130-141)
🔇 Additional comments (49)
rules/sort-switch-case.ts (1)
15-15: Import path refactor forcompareis consistent with the new utility layout.Switching to
../utils/compare/compareremoves reliance on a barrel and keeps behavior identical at call sites.test/utils/get-custom-groups-compare-options.test.ts (1)
5-5: Test import updated to the new options utility module.Pointing
getCustomGroupsCompareOptionsat../../utils/build-default-options-by-group-index-computerkeeps the tests aligned with the refactored options-computation utility without changing test behavior.rules/sort-imports/types.ts (1)
28-38: StrictertsconfigandmaxLineLengthtypings match the intended defaults.Making
tsconfig.rootDirandmaxLineLengthrequired at the nested level (while keepingOptions[number]wrapped inPartial) ensures that:
- configs that provide
tsconfigmust specify arootDir, and- internal consumers using
Required<Options[number]>can rely on bothtsconfig.rootDirandmaxLineLengthbeing defined.This aligns with the idea of having concrete defaults; just confirm that the sort-imports JSON schema and docs reflect
rootDirandmaxLineLengthas effectively required when set.Also applies to: 61-65
rules/sort-object-types/get-custom-groups-compare-options.ts (1)
1-1: Updated imports to new compare/options utilities without changing behavior.Referencing
NodeValueGetterFunctionfromcompare/compareand delegating to the newbuild-default-options-by-group-index-computerhelper keeps this wrapper consistent with the rest of the codebase while preserving its contract.Also applies to: 4-4
rules/sort-enums/types.ts (1)
2-2: CentralizingSortEnumsSortingNodein the types module improves consistency.Exporting
SortEnumsSortingNodehere, based onSortingNodeWithDependencies<TSEnumMember>plusnumericValue/value, mirrors how other rules expose their node types and removes the need for a local interface insort-enums.ts.Also applies to: 4-4, 52-56
test/utils/compare/compare.test.ts (1)
3-3: Compare tests now use explicit, non-barrel imports.Switching to
../../../types/sorting-node,../../../utils/compare/compare, and../../../utils/alphabetremoves reliance on index exports while keeping the covered behavior identical.Also applies to: 5-6
rules/sort-object-types.ts (1)
76-91: AddingfallbackSort.sortBydefault keeps value-getter logic well-defined.Initializing
fallbackSortas{ type: 'unsorted', sortBy: 'name' }ensures that downstream helpers (likegetCustomGroupsCompareOptions) can always rely on a concretefallbackSort.sortBy, matching the rule’s top-levelsortBy: 'name'default.rules/sort-export-attributes.ts (2)
24-24: LGTM! Import updated consistently with API refactor.The import change from
buildGetCustomGroupOverriddenOptionsFunctiontobuildDefaultOptionsByGroupIndexComputeraligns with the broader refactoring across multiple sort rules.
139-140: LGTM! Parameter naming improved for clarity.The rename from
getOptionsByGroupIndextooptionsByGroupIndexComputerbetter conveys that this is a computer function rather than just a getter, and the usage ofbuildDefaultOptionsByGroupIndexComputer(options)is consistent with the new API.test/rules/sort-imports/validate-side-effects-configuration.test.ts (2)
6-16: LGTM! Test parameterization improves coverage.The refactoring to use
it.eachtests both array-based and object-based group configurations systematically, improving maintainability while reducing duplication.
18-39: LGTM! Parameterization enhances test clarity.The parameterized approach now clearly tests both flat array groups and object-wrapped groups for the side-effect validation error case, making the test intent more explicit.
rules/sort-imports/validate-side-effects-configuration.ts (3)
4-6: LGTM! New imports support refactored validation logic.The imports of
isStringGroupSideEffectOnlyGroupandcomputeGroupsNamesenable cleaner separation of concerns for side-effect group validation.
35-40: LGTM! Validation logic refactored for clarity.The change from inline filtering to mapping with
computeGroupsNamesfollowed by checking withhasSideEffectGroupmakes the validation logic more readable and maintainable.
48-50: LGTM! Helper function improves code clarity.The
hasSideEffectGrouphelper encapsulates the check for whether any group in a nested structure is a side-effect group, making the validation logic easier to understand.utils/sort-nodes.ts (2)
1-1: LGTM! Import paths updated consistently.The import paths for
NodeValueGetterFunctionandcomparehave been updated to reflect the new organization under./compare/compare.Also applies to: 5-5
17-17: No issues found with the maxLineLength removal.Verification confirms that
maxLineLengthis never passed tosortNodesby any caller in the codebase. The parameter only exists in thesort-importsrule context (where it's part of that rule's specific options type). All three call sites—inrules/sort-switch-case.ts,utils/sort-nodes-by-groups.ts, and test files—only pass fields that are part ofCommonOptions, making the type simplification a safe change with no breaking impact.rules/sort-jsx-props.ts (3)
25-25: LGTM! Import updated for new options computer.The import change to
buildDefaultOptionsByGroupIndexComputeris consistent with the API refactoring across all sort rules.
27-31: LGTM! Additional imports organized properly.The explicit imports of
singleCustomGroupJsonSchema,allModifiers, andallSelectorsfrom the types file improve code organization and make dependencies clearer.
189-190: LGTM! Parameter naming aligns with refactoring.The change from
getOptionsByGroupIndextooptionsByGroupIndexComputerwith the new builder function is consistent with the broader API improvements.rules/sort-named-imports.ts (2)
23-23: LGTM! Import consistent with API refactor.The import of
buildDefaultOptionsByGroupIndexComputeraligns with the standardized per-group options computation across sort rules.
175-176: LGTM! Options computation updated correctly.The parameter rename to
optionsByGroupIndexComputerand use ofbuildDefaultOptionsByGroupIndexComputer(options)follows the established refactoring pattern.rules/sort-array-includes.ts (2)
25-25: LGTM! Import updated for consistency.The import change to
buildDefaultOptionsByGroupIndexComputermaintains consistency with the refactoring across all sort rules.
254-255: LGTM! Parameter usage follows new API.The change to
optionsByGroupIndexComputerwithbuildDefaultOptionsByGroupIndexComputer(options)is consistent with the standardized approach for per-group option computation.rules/sort-import-attributes.ts (2)
24-24: LGTM! Final import change completes refactoring.The import of
buildDefaultOptionsByGroupIndexComputercompletes the consistent API refactoring across all sort-related rules.
139-140: LGTM! Options computation standardized.The parameter change to
optionsByGroupIndexComputerwithbuildDefaultOptionsByGroupIndexComputer(options)completes the standardization of per-group option computation across the codebase.rules/sort-exports.ts (1)
22-22: LGTM! Consistent refactoring to the new options computer API.The rename from
buildGetCustomGroupOverriddenOptionsFunctiontobuildDefaultOptionsByGroupIndexComputerimproves clarity, and the option key change tooptionsByGroupIndexComputeraligns with the new naming convention. The changes are applied consistently.Also applies to: 165-166
test/rules/sort-imports/is-string-group-side-effect-only-group.test.ts (1)
1-16: LGTM! Good test coverage for the new utility.The test suite properly covers both positive cases (the two side-effect group names) and the negative case using parameterized testing. This provides adequate coverage for the simple utility function.
test/rules/sort-imports/is-side-effect-only-group.test.ts (1)
6-8: LGTM! Good defensive test coverage.Adding test coverage for
undefinedinput ensures the function handles this edge case gracefully, which aligns with the updated function signature that acceptsundefined.rules/sort-object-types/build-node-value-getter.ts (1)
1-1: LGTM! Import path updated to reflect module reorganization.The import path change aligns with the restructuring of comparison utilities under
utils/compare/compare. No functional changes.rules/sort-imports/is-string-group-side-effect-only-group.ts (1)
1-3: LGTM! Clean utility extraction.The function is simple, correct, and enables reuse of the side-effect group detection logic. The implementation properly checks for both 'side-effect' and 'side-effect-style' group names.
rules/sort-named-exports.ts (1)
23-23: LGTM! Consistent with the broader refactoring pattern.The changes mirror those in
sort-exports.ts, adopting the newbuildDefaultOptionsByGroupIndexComputerAPI andoptionsByGroupIndexComputeroption key. The refactoring is applied consistently across rules.Also applies to: 178-179
utils/compare/build-string-formatter.ts (1)
1-59: LGTM! Well-designed string formatting utility.The function properly centralizes string formatting logic for comparison operations. Key points:
- Handles case normalization, special character processing, and whitespace removal
- Uses appropriate Unicode ranges for Latin-based alphabets with diacritics
- Includes exhaustive case handling with
UnreachableCaseError- Well-documented with JSDoc comments
The regex patterns correctly use the
uflag for full Unicode support and handle extended Latin character ranges.docs/content/rules/sort-imports.mdx (1)
265-265: Documentation defaults verified as correct.Verification confirms all three documentation defaults match the sort-imports rule implementation:
maxLineLength: Infinity✓tsconfig: { rootDir: '' }✓- Empty
rootDirconfiguration present ✓rules/sort-variable-declarations.ts (2)
22-27: LGTM! Clean refactoring with appropriate validation.The import updates and addition of
validateGeneratedGroupsConfigurationare well-placed. The validation ensures generated groups configuration is correct before processing, which improves error handling.Also applies to: 89-93
179-180: LGTM! Consistent API refactoring.The parameter rename from
getOptionsByGroupIndextooptionsByGroupIndexComputeris more descriptive and aligns with the function name, improving code clarity.test/utils/compare/build-string-formatter.test.ts (1)
1-49: LGTM! Comprehensive test coverage.The test file thoroughly covers the
buildStringFormatterfunctionality:
- Case-insensitive transformation
- Whitespace removal
- Special characters handling (remove, keep, trim)
The test cases are well-structured and validate the expected behavior across different configuration options.
rules/sort-modules.ts (1)
29-29: LGTM! Consistent with the refactoring pattern.The import and parameter updates align with the systematic refactoring across all sorting rules.
Also applies to: 350-351
rules/sort-union-types.ts (1)
23-23: LGTM! Refactoring applied consistently.The changes mirror the pattern established in other sorting rule files.
Also applies to: 280-281
rules/sort-objects.ts (1)
28-28: LGTM! Consistent refactoring.The changes follow the established pattern across the codebase.
Also applies to: 356-357
rules/sort-classes.ts (1)
27-27: LGTM! Refactoring pattern maintained.The changes are consistent with the systematic refactoring across all sorting rules.
Also applies to: 605-606
rules/sort-decorators.ts (1)
24-24: LGTM! Consistent with refactoring objectives.The changes align with the systematic refactoring pattern applied throughout the PR.
Also applies to: 265-266
rules/sort-heritage-clauses.ts (1)
22-22: LGTM! Final file completes the consistent refactoring.The changes complete the systematic refactoring across all affected sorting rule files, maintaining consistency throughout.
Also applies to: 189-190
rules/sort-imports.ts (2)
30-37: Defaults fortsconfigandmaxLineLengthalign with the new typing and schemaSwitching
defaultOptionstoRequired<Options[number]>withtsconfig: { rootDir: '' }andmaxLineLength: Infinitygives you non‑optional access tooptions.tsconfig.rootDir/options.tsconfig.filenameandoptions.maxLineLength, while preserving previous behavior:
- An empty
rootDirmeans the tsconfig search is completely skipped.Infinitymeans the “use name length for very long imports” path is effectively disabled unless a finitemaxLineLengthis configured.This looks consistent with the JSON schema (tsconfig object is only required to have
rootDirwhen the user specifies it) and with howreadClosestTsConfigByPathis guarded.Also applies to: 77-105, 124-133
241-243:maxLineLengththreshold logic matches the documented “exceeded” semanticsThe new condition:
if (hasMultipleImportDeclarations && size > options.maxLineLength) { size = name.length + 10 }correctly triggers only when the actual node span exceeds
maxLineLength, which matches the documentation wording (“When exceeded, import names are used for sorting…”). With the default ofInfinity, this remains a no‑op unless the user opts in.utils/build-default-options-by-group-index-computer.ts (1)
123-141:buildDefaultOptionsByGroupIndexComputercleanly encapsulates per‑group overridesThe new
buildDefaultOptionsByGroupIndexComputerand updatedgetCustomGroupOverriddenOptionssignatures look consistent with the rest of the refactor:
- Accepting
GroupRelatedOptions & CommonOptionsmatches how rule‑leveloptionsare shaped.- Returning
{ options: CommonOptions }aligns with the newoptionsByGroupIndexComputercontract insortNodesByGroups, while still passing through all other options properties at runtime viagetCustomGroupOverriddenOptions.This gives rules a simple, reusable way to plug in group‑specific options without duplicating override logic.
Also applies to: 156-167
rules/sort-enums.ts (1)
3-5: Enums refactor integrates cleanly with the new per‑group options APIThe changes here look cohesive:
Importing
NodeValueGetterFunction,SortEnumsSortingNode, andOptionsclarifies the public type surface and tightens the typing ofcomputeNodeValueGetter.Wiring
sortNodesByGroupsthroughoptionsByGroupIndexComputer: groupIndex => ({ options: getCustomGroupOverriddenOptions({ options: overriddenOptions, groupIndex, }), nodeValueGetter, })ensures each group sees the numeric‑enum‑aware
typewhile still honoring any group/custom‑group overrides.This keeps the rule aligned with the new
optionsByGroupIndexComputerabstraction without changing its observable behavior.Also applies to: 23-24, 217-223
test/utils/sort-nodes-by-groups.test.ts (1)
90-107: Per-group callbacks (isNodeIgnoredForGroup, nodeValueGetter, fallbackSortNodeValueGetter) are well coveredThe tests exercising:
isNodeIgnoredForGroup: ({ node }) => node === nodeB- Per-call
optionsByGroupIndexComputerthat injectnodeValueGetter/fallbackSortNodeValueGetteralongsideoptionscorrectly reflect the new object-based callback signature and the per-group options computer pattern. The use of
createTestNodeto extendSortingNodewithactualValuekeeps the value-getter tests type-safe and realistic.No issues from a behavior or typing standpoint here.
Also applies to: 110-157
utils/sort-nodes-by-groups.ts (1)
8-37: New per-group options computer and parameter typing are coherentThe introduction of:
OptionsByGroupIndexComputer<Options extends CommonOptions, T extends SortingNode>optionsByGroupIndexComputerinSortNodesByGroupsParameters- The object-arg
isNodeIgnoredForGroup({ groupOptions, groupIndex, node })lines up cleanly with how
sortNodesByGroupsand the tests use these callbacks. The generics (Options extends CommonOptions,T extends SortingNode) keep the core utility flexible while ensuring the options object always matches the rest of the compare/sort pipeline.From this file’s perspective, the public surface and internal plumbing look sound.
utils/compare/compare.ts (1)
3-8: CommonOptions-only compare pipeline and shared formatter integration look solidThe refactor to:
- Use
CommonOptionsdirectly inCompareParametersandcomputeCompareValue- Export
NodeValueGetterFunctionfor reuse- Centralize string normalization in
buildStringFormatterfor custom, natural, and alphabetical sortingkeeps the compare utility aligned with the new options model and removes the ad‑hoc formatter logic from this file. The same
specialCharacters/ignoreCasehandling now consistently applies across the different sorting strategies, which is a nice simplification with no obvious behavioral regressions given the shared formatter implementation.Also applies to: 37-39, 164-167, 229-230, 288-291, 322-325
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
utils/build-default-options-by-group-index-computer.ts (1)
119-132: JSDoc example should reflect{ options }wrapper return shapeThe example assigns
const group1Options = getOverriddenOptions(0), but the function actually returns{ options: CommonOptions }. To avoid confusion, consider updating the example to destructure:- * const getOverriddenOptions = - * buildDefaultOptionsByGroupIndexComputer(options) - * const group1Options = getOverriddenOptions(0) - * const group2Options = getOverriddenOptions(1) + * const getOverriddenOptions = + * buildDefaultOptionsByGroupIndexComputer(options) + * const group1Options = getOverriddenOptions(0).options + * const group2Options = getOverriddenOptions(1).optionsso docs match the actual API.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/utils/get-custom-groups-compare-options.test.ts(2 hunks)utils/build-default-options-by-group-index-computer.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/utils/get-custom-groups-compare-options.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
utils/build-default-options-by-group-index-computer.ts (1)
types/common-options.ts (1)
CommonOptions(23-73)
🔇 Additional comments (2)
utils/build-default-options-by-group-index-computer.ts (2)
94-102: Fallback sort override refactor keeps semantics and avoids undefinedorderThe
fallbackOrdertemp plus conditional spread cleanly preserves the basefallbackSort.orderwhen no custom override is provided and avoids emittingorder: undefined. The behavior for all combinations of global/customfallbackSortlooks correct.
154-165: Options merge helper is correctly typed and orderedTyping
optionsasGroupRelatedOptions & CommonOptionsand returning{ ...options, ...getCustomGroupsCompareOptions(...) }cleanly preserves all common options while allowing per-group overrides offallbackSort,order, andtype. Merge order is correct and non-mutating; no issues spotted.
| let settings = getSettings(context.settings) | ||
| let options = complete(context.options.at(0), settings, defaultOptions) | ||
| validateCustomSortConfiguration(options) | ||
| validateGeneratedGroupsConfiguration({ |
There was a problem hiding this comment.
Why did you decide to delete this?
There was a problem hiding this comment.
Oops, that's an oversight. I forgot that this does more than check predefined/generated groups.
- Restore:
e704e04(#620) - Rename function to
validateGroupsConfigurationto make it more explicit:fe9b971(#620)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
utils/validate-groups-configuration.ts (1)
28-55: JSDoc examples reference the old function name.The
@exampleblocks still showvalidateGeneratedGroupsConfigurationbut the function was renamed tovalidateGroupsConfiguration. Update these references for consistency.* @example * // Valid predefined groups for React imports -* validateGeneratedGroupsConfiguration({ +* validateGroupsConfiguration({ * options: { * groups: ['react', 'external', 'internal', 'side-effect-import'], ... * @example * // Invalid group that doesn't exist -* validateGeneratedGroupsConfiguration({ +* validateGroupsConfiguration({ * options: { ... * @example * // Valid with custom groups for class members -* validateGeneratedGroupsConfiguration({ +* validateGroupsConfiguration({ * options: {
🧹 Nitpick comments (1)
utils/validate-groups-configuration.ts (1)
7-15: Consider renaming the interface for consistency.The interface
ValidateGenerateGroupsConfigurationParameterscould be renamed toValidateGroupsConfigurationParametersto match the renamed function. This is optional but would improve naming consistency.-interface ValidateGenerateGroupsConfigurationParameters { +interface ValidateGroupsConfigurationParameters { options: { customGroups: CustomGroupsOption groups: GroupsOptions<string> } selectors: string[] modifiers: string[] }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
rules/sort-array-includes.ts(3 hunks)rules/sort-classes.ts(3 hunks)rules/sort-decorators.ts(3 hunks)rules/sort-enums.ts(4 hunks)rules/sort-export-attributes.ts(3 hunks)rules/sort-exports.ts(3 hunks)rules/sort-heritage-clauses.ts(3 hunks)rules/sort-import-attributes.ts(3 hunks)rules/sort-imports.ts(8 hunks)rules/sort-jsx-props.ts(3 hunks)rules/sort-maps.ts(3 hunks)rules/sort-modules.ts(3 hunks)rules/sort-named-exports.ts(3 hunks)rules/sort-named-imports.ts(3 hunks)rules/sort-object-types.ts(4 hunks)rules/sort-objects.ts(4 hunks)rules/sort-union-types.ts(3 hunks)rules/sort-variable-declarations.ts(3 hunks)test/utils/validate-groups-configuration.test.ts(7 hunks)utils/validate-groups-configuration.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- rules/sort-classes.ts
- rules/sort-variable-declarations.ts
- rules/sort-union-types.ts
- rules/sort-decorators.ts
- rules/sort-array-includes.ts
- rules/sort-export-attributes.ts
- rules/sort-import-attributes.ts
- rules/sort-exports.ts
🧰 Additional context used
🧬 Code graph analysis (10)
test/utils/validate-groups-configuration.test.ts (1)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)
rules/sort-heritage-clauses.ts (2)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(128-139)
rules/sort-object-types.ts (2)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)utils/unreachable-case-error.ts (1)
UnreachableCaseError(31-43)
rules/sort-modules.ts (2)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(128-139)
rules/sort-named-imports.ts (2)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(128-139)
rules/sort-maps.ts (2)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(128-139)
rules/sort-jsx-props.ts (2)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(128-139)
rules/sort-enums.ts (1)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)
rules/sort-named-exports.ts (2)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(128-139)
rules/sort-objects.ts (2)
utils/validate-groups-configuration.ts (1)
validateGroupsConfiguration(75-93)utils/build-default-options-by-group-index-computer.ts (1)
buildDefaultOptionsByGroupIndexComputer(128-139)
🔇 Additional comments (24)
rules/sort-jsx-props.ts (3)
25-33: LGTM! Import updates align with the refactoring pattern.The new imports for
buildDefaultOptionsByGroupIndexComputer,validateGroupsConfiguration, and the type exports from./sort-jsx-props/typesare consistent with the broader refactoring effort across the codebase.
96-100: Validation call correctly updated.The
validateGroupsConfigurationinvocation properly passesallSelectors,allModifiers, andoptionsin the expected structure.
189-190: Correct usage of the new options-by-group-index computer.The
optionsByGroupIndexComputerkey andbuildDefaultOptionsByGroupIndexComputer(options)call match the expected API signature fromutils/build-default-options-by-group-index-computer.ts.utils/validate-groups-configuration.ts (1)
75-93: Core function rename looks correct.The function signature and implementation are unchanged except for the name. The validation logic correctly validates groups against predefined groups and custom groups.
rules/sort-object-types.ts (3)
42-48: Import additions are appropriate.The new imports for
validateGroupsConfigurationandUnreachableCaseErrorsupport the updated validation and exhaustive switch pattern.
200-204: Validation call correctly updated.The
validateGroupsConfigurationinvocation uses the proper structure withallSelectors,allModifiers, andoptions.
315-340: Well-structured options computer and node ignore logic.The
optionsByGroupIndexComputerimplementation correctly delegates togetCustomGroupsCompareOptionsand returns the expected structure. TheisNodeIgnoredForGroupfunction uses an exhaustive switch withUnreachableCaseErroras a guard, which is a good defensive pattern for handling thesortBydiscriminant.rules/sort-heritage-clauses.ts (3)
22-24: Import updates align with the refactoring pattern.The new imports for
buildDefaultOptionsByGroupIndexComputerandvalidateGroupsConfigurationare consistent with other rules.
105-109: Validation call correctly passes empty arrays.Heritage clauses don't use modifiers or selectors, so passing empty arrays is appropriate here.
189-190: Correct usage of the new options-by-group-index computer.The
optionsByGroupIndexComputerkey andbuildDefaultOptionsByGroupIndexComputer(options)call match the expected API.rules/sort-named-exports.ts (3)
23-30: Import updates are consistent with the refactoring pattern.The new imports for
buildDefaultOptionsByGroupIndexComputerandvalidateGroupsConfigurationfollow the same pattern as other rule files.
85-89: Validation call correctly updated.The
validateGroupsConfigurationinvocation properly passesallModifiers,allSelectors, andoptionsin the expected structure.
178-179: Correct usage of the new options-by-group-index computer.The
optionsByGroupIndexComputerkey andbuildDefaultOptionsByGroupIndexComputer(options)call match the expected API signature.test/utils/validate-groups-configuration.test.ts (1)
3-3: LGTM! Consistent function rename.The import and all usage sites have been consistently updated from
validateGeneratedGroupsConfigurationtovalidateGroupsConfiguration.Also applies to: 6-6, 29-29, 42-42, 61-61, 75-75, 88-88, 110-110
rules/sort-objects.ts (1)
28-28: LGTM! Consistent refactoring to new options computer pattern.The changes correctly update both the imports and usage to align with the new per-group options computation approach using
buildDefaultOptionsByGroupIndexComputer.Also applies to: 37-37, 111-115, 356-357
rules/sort-modules.ts (1)
29-29: LGTM! Consistent refactoring applied.The imports and usage have been updated consistently to use the new validation function and options computer pattern.
Also applies to: 36-36, 138-142, 350-351
rules/sort-named-imports.ts (1)
23-23: LGTM! Consistent with the refactoring pattern.All changes align with the standardized approach to per-group options computation and validation.
Also applies to: 30-30, 88-92, 175-176
rules/sort-imports.ts (4)
77-104: LGTM! Default options expanded appropriately.The new default options for
tsconfigandmaxLineLengthare sensible:
tsconfig: { rootDir: '' }provides a safe default (empty rootDir will skip tsconfig reading)maxLineLength: Infinitymeans no line length limit by default
124-131: LGTM! Tsconfig access pattern updated correctly.The changes properly access the nested tsconfig properties with appropriate fallbacks.
511-536: LGTM! Good extraction of name computation logic.The new
getNodeNamehelper function consolidates the logic for extracting module names from different import node types, improving code organization.
115-119: LGTM! Import sorting refactoring applied consistently.The changes properly integrate the new validation and options computation patterns, along with the updated tsconfig handling and maxLineLength usage.
Also applies to: 241-243, 312-319
rules/sort-maps.ts (1)
22-22: LGTM! Consistent refactoring for map sorting.The changes follow the same pattern as other rules, correctly using empty arrays for selectors and modifiers since maps don't have those concepts.
Also applies to: 25-25, 97-101, 180-181
rules/sort-enums.ts (2)
3-4: LGTM! Type organization improved and validation updated.The type imports from a separate module improve code organization, and the validation function has been consistently updated.
Also applies to: 23-25, 86-90
223-229: LGTM! Custom options computer correctly handles enum-specific needs.Unlike other rules, this uses an inline
optionsByGroupIndexComputerimplementation because enums need to pass bothoptionsandnodeValueGetter. The logic correctly overrides options per group while including the enum-specific value getter.
2 participants
Description
This PR is just some various renaming and chore commits to prepare for #617.
What is the purpose of this pull request?