-
Notifications
You must be signed in to change notification settings - Fork 56
feat: add newlinesInside at rule options and settings level
#651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a new newlinesInside option (number | 'ignore' | 'newlinesBetween'), exposes it in many rules' defaultOptions, updates common group types and JSON schemas, rewires newline computation and validation, updates fix/report helper signatures, and expands tests and documentation to distinguish inside-group vs between-group newline semantics. Changes
Sequence Diagram(s)mermaid UserConfig->>Validator: provide options (newlinesBetween, newlinesInside, groups, customGroups) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 @@
## main #651 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 143 143
Lines 3521 3554 +33
Branches 1085 1106 +21
=========================================
+ Hits 3521 3554 +33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
6afb7c0 to
7ebee9e
Compare
4142242 to
e9f434d
Compare
newlinesInside at rule options and settings levelnewlinesInside at rule options and settings level
e9f434d to
5a31529
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/content/rules/sort-maps.mdx (1)
205-236: FixnewlinesBetween/newlinesInsidetype and example inconsistenciesA few doc nits around the new options here:
newlinesBetweenis now documented asnumber | 'ignore'with default'ignore', but the Flat/Legacy config examples still usenewlinesBetween: false. That’s confusing and doesn’t match the stated type/default. Those examples should either use'ignore'(or a number) or the type/default text should acknowledgefalseif it remains supported.newlinesInsideis typed at the top level asnumber | 'ignore', yet thegroupsandCustomGroup*definitions only listnewlinesInside?: number;. If'ignore'is also valid per-group/per-custom-group, those type snippets should shownumber | 'ignore'; if not, calling out that difference would be helpful.- The Flat/Legacy config examples don’t surface
newlinesInsideat all. Given this PR introduces it as a first-class option, consider addingnewlinesInside: 'ignore'(or the appropriate default) in the example configs next tonewlinesBetween.Also applies to: 291-307, 368-376, 384-393, 420-447, 454-477
test/rules/sort-exports.test.ts (2)
2471-2516: Duplicate test blocks detected in the 'natural' section.The test "enforces newlines if the global option is %s and the group option is %s" at lines 2618-2663 is an exact duplicate of the test at lines 2471-2515. Similarly, subsequent tests in this range appear to be duplicated:
- Lines 2664-2706 duplicates 2517-2559
- Lines 2708-2763 duplicates 2561-2616
🔎 Suggested fix: Remove duplicate test blocks
Remove the duplicate test blocks at lines 2618-2763.
Also applies to: 2618-2663
3942-3986: Duplicate test blocks detected in the 'line-length' section.Similar to the 'natural' section, there are duplicate test blocks:
- Lines 4089-4133 duplicates 3942-3986
- Lines 4135-4177 duplicates 3988-4030
- Lines 4179-4234 duplicates 4032-4087
🔎 Suggested fix: Remove duplicate test blocks
Remove the duplicate test blocks at lines 4089-4234.
Also applies to: 4089-4133
♻️ Duplicate comments (4)
test/rules/sort-classes.test.ts (4)
8381-8420: Natural-sorting variant correctly mirrors alphabeticalnewlinesBetween: 0behaviorThis block is a natural‑order clone of the alphabetical
newlinesBetween: 0test:
- Same grouping and spacing expectations, differing only in the
type: 'natural'preset behind...options.- Confirms newline semantics are independent of the string‑comparison strategy.
Looks consistent with the alphabetical version; no additional changes needed.
Optional: you could DRY these three nearly identical tests (alphabetical/natural/line‑length) via
describe.eachon thetype/orderpresets if the duplication becomes hard to maintain.
8708-8714: Natural-sorting comment-preservation options kept in syncAdding
newlinesInside: 0here to match the alphabetical test keeps behavior consistent across sort types while still validating comment attachment after fixes. No issues found.
12835-12870: Line-length variant ofnewlinesBetween: 0behaves consistentlyThis is the line‑length counterpart of the earlier
newlinesBetween: 0tests:
- Same grouping and spacing logic, with ordering driven by line length instead of lexical compare.
- Ensures newline enforcement remains orthogonal to the chosen sort strategy.
The expectations line up with the other two variants.
13158-13164: Line-length comment-preservation test updated consistently withnewlinesInsideThe options mirror the alphabetical/natural variants (
groups: ['property', 'method'], newlinesBetween: 1, newlinesInside: 0), giving a third safety net thatnewlinesInsidedoesn’t interfere with preserving inline comments under different sort strategies.
🧹 Nitpick comments (16)
utils/json-schemas/common-groups-json-schemas.ts (1)
28-40: Schema validation properly supports the new option.The
newlinesInsideJsonSchemacorrectly mirrors the structure ofnewlinesBetweenJsonSchemawithoneOfallowing either'ignore'or a non-negative number.Optional: Enhance schema description
The description on line 31 could be slightly more precise:
- description: 'Specifies how to handle newlines between groups elements', + description: 'Specifies how to handle newlines between elements within the same group.',This clarifies "within the same group" vs "between groups."
test/rules/sort-variable-declarations.test.ts (1)
1381-1414: Clarify test description to matchnewlinesInsidesemanticsThe new test correctly exercises
newlinesInside: 0with groups/customGroups and the updated spacing behavior, but the description says “between groups” even thoughnewlinesInsidegoverns spacing inside groups. Consider renaming to something like:
it('removes newlines inside groups when newlinesInside is 0', …)to avoid confusion with
newlinesBetween.docs/content/rules/sort-enums.mdx (1)
224-245:newlinesBetween/newlinesInsidedocumentation matches behaviorThe new
newlinesInsidesection and the clarifiednewlinesBetweennotes accurately describe the split between inter‑group and intra‑group newline handling and correctly mention the dependency onpartitionByNewLine === false. You might optionally addnewlinesInside: 'ignore'to the sample config in the “Usage” section for discoverability, but the current docs are technically correct.docs/content/rules/sort-variable-declarations.mdx (1)
378-392: Consider addingnewlinesInsideto the usage example for completeness.The usage example includes
newlinesBetween: 'ignore'but omits the newnewlinesInsideoption. For consistency and to help users discover the new option, consider adding it.Suggested addition
partitionByNewLine: false, partitionByComment: false, newlinesBetween: 'ignore', + newlinesInside: 'ignore', groups: [], customGroups: [],docs/content/rules/sort-export-attributes.mdx (1)
146-176: AlignnewlinesInsidedocs, group types, and examplesThe new
newlinesInsidesection looks good, but there are a couple of inconsistencies:
- You document
newlinesInsideasnumber | 'ignore'with default'ignore', yet thegroupsandcustomGroupstype snippets only shownewlinesInside?: number;. If'ignore'is allowed at group/customGroup level (to override a numeric global setting), those snippets should reflectnumber | 'ignore'as well; if it is not allowed, that restriction should be stated explicitly.- The Flat/Legacy config examples in “Usage” expose
newlinesBetweenbut notnewlinesInside, even though the option is now first‑class for this rule. Consider addingnewlinesInside: 'ignore'(or the rule’s effective default) alongsidenewlinesBetweenso readers see how to configure it.Also applies to: 178-222, 250-266
docs/content/rules/sort-import-attributes.mdx (1)
146-176: Expose and typenewlinesInsideconsistently for this ruleHere as well:
- Top-level
newlinesInsideis typed asnumber | 'ignore', but thegroupsandcustomGroupsblocks documentnewlinesInside?: number;. If'ignore'is valid per-group/per-custom-group, the docs should shownumber | 'ignore'to avoid implying only numeric values are accepted.- The Flat/Legacy config examples only show
newlinesBetween, so the new option isn’t discoverable from the examples. It would help to addnewlinesInside: 'ignore'(or the correct default for this rule) nearnewlinesBetweenin both examples.Also applies to: 178-222, 250-266
docs/content/rules/sort-named-exports.mdx (1)
241-247: Docs fornewlinesBetween/newlinesInsidelook consistent with behaviorThe updated text clearly separates between-group (
newlinesBetween) and in-group (newlinesInside) semantics, and the applicability constraint withpartitionByNewLine: falsematches the implementation and tests. Only very minor: “between each group” could be “between each pair of groups” for precision, but the current wording is understandable.Also applies to: 248-261
docs/content/rules/sort-sets.mdx (1)
259-265:newlinesBetween/newlinesInsidedocumentation is accurate and consistentThe revised
newlinesBetweendescription and the newnewlinesInsidesection match the underlying behavior: between vs inside groups are clearly separated, both are scoped topartitionByNewLine: false, and overrides viagroups/customGroupsare documented with the right anchors. Wording is clear; if you want to be ultra-precise, “between each group” could be “between each pair of groups”.Also applies to: 266-279
utils/get-newlines-between-option.ts (1)
42-65:newlinesInsideintegration and simplifiednewlinesBetweenlookup look correct
- Same‑group pairs now go through
computeNewlinesInsideOption, which correctly resolves in this order: matchingcustomGroups[groupName].newlinesInside, thengroupoverride ({ group, newlinesInside }), then the globaloptions.newlinesInside. That matches the intended precedence.- Cross‑group pairs use either:
options.newlinesBetweendirectly for adjacent defined groups, orcomputeNewlinesBetweenOptionForDifferentGroupswhen there are explicit{ newlinesBetween }entries in between. That helper:
- Seeds missing transitions via
buildGroupsWithAllNewlinesBetween(..., options.newlinesBetween).- Chooses the maximum numeric value when any positive numbers are present.
- Falls back to
'ignore'if present, otherwise0, which matches the test matrix for combinations of global vs group overrides (2/0/ignore).- The fallback to
options.newlinesInsidewhengroupis missing incomputeNewlinesInsideOptionis reasonable under the invariant that the mergedCommonGroupsOptionsalways sets a global default.No functional issues spotted; the refactor also makes the flow easier to follow by removing the extra “global” helper.
Also applies to: 67-112, 113-137
test/rules/sort-jsx-props.test.ts (1)
1193-1235: Optional: consider adding a symmetricnewlinesInsidetest for non‑alphabetical JSX modesRight now only the alphabetical suite (Line [848]–[895]) has a dedicated “removes newlines inside groups when newlinesInside is 0” test; natural and line‑length rely on shared implementation to behave the same way.
Given the newline logic is type‑agnostic, that’s probably fine, but if you ever see regressions diverging by type it might be worth adding one minimal inside‑group testcase per mode for symmetry and clearer diagnostics.
Also applies to: 2444-2485, 3697-3738, 848-895
test/rules/sort-union-types.test.ts (1)
822-861: Optional: consider one explicit precedence test for rule-level vs group-levelnewlinesInsideYou already have:
- Rule‑level
newlinesInside: 0coverage (lines 822-861).- Custom‑group-level
newlinesInsidetests (group1cases around 1571-1638).If not already covered elsewhere, it might be worth adding a single test where both are set (e.g., global
newlinesInside: 1and a specific group overriding to0) to lock in the precedence rules for union types, mirroring the thoroughnewlinesBetweenprecedence matrix.Also applies to: 1100-1135, 1571-1638
test/rules/sort-enums.test.ts (1)
1094-1141: Consider expanding test coverage fornewlinesInsideto matchnewlinesBetweenthoroughness.The new
newlinesInsideoption currently has limited test coverage compared to the comprehensive testing fornewlinesBetween(which includes.eachtests for various value combinations, precedence rules, and edge cases at lines 1340-1497). Consider adding:
- Tests for
newlinesInside: 1andnewlinesInside: 2at the rule level.eachtests covering combinations ofnewlinesInsidevalues withnewlinesBetweenvalues- Tests for precedence: group-level → rule-level → settings-level
- Tests in "natural", "line-length", and "custom" sections (currently only in "alphabetical")
While the current tests correctly verify the basic functionality, expanded coverage would help ensure edge cases are handled correctly for this breaking change feature.
utils/validate-newlines-and-partition-configuration.ts (1)
130-163: Inconsistent error types:ErrorvsTypeError.Line 132 throws
Errorwhile line 161 throwsTypeErrorfor the same error message. This inconsistency may cause confusion for error handling consumers.🔎 Suggested fix: Use consistent error types
function validateNewlinesInsideConfiguration({ newlinesInside, customGroups, groups, }: Pick< CommonGroupsOptions<unknown, unknown, string>, 'newlinesInside' | 'customGroups' | 'groups' >): void { if (newlinesInside !== 'ignore') { - throw new Error(NEWLINES_INSIDE_ERROR_MESSAGE) + throw new TypeError(NEWLINES_INSIDE_ERROR_MESSAGE) } validateGroups() validateCustomGroups() function validateCustomGroups(): void { for (let customGroup of customGroups) { throwErrorIfNeeded(customGroup.newlinesInside) } } function validateGroups(): void { for (let group of groups) { if (!isGroupWithOverridesOption(group)) { continue } throwErrorIfNeeded(group.newlinesInside) } } function throwErrorIfNeeded( newlinesInsideOptions: NewlinesInsideOption | undefined, ): void { if (newlinesInsideOptions === undefined) { return } if (newlinesInsideOptions !== 'ignore') { throw new TypeError(NEWLINES_INSIDE_ERROR_MESSAGE) } } }Alternatively, use
Errorconsistently in both places.TypeErroris typically used for invalid argument types, whileErroris more generic. Since this is a configuration validation error,Errormight be more appropriate.types/common-groups-options.ts (1)
46-47: Consider aligning comment wording with the sibling option.The comment says "Specify the exact number of newlines required" but
NewlinesInsideOptionalso allows'ignore'. The comment at line 96 for the interface-level property is more accurate: "Specify the exact number of newlines required between elements of groups."🔎 Suggested improvement
- /** Specify the exact number of newlines required. */ + /** Specify the exact number of newlines required inside the group, or 'ignore' to preserve existing. */ newlinesInside?: NewlinesInsideOptiontest/rules/sort-objects.test.ts (2)
5861-5880: Optional: spread...optionsso this test actually exercises natural sortWhile you’re touching this test, consider changing the options object to spread the per‑describe
options(withtype: 'natural') instead of only specifyinggroups,newlinesBetween, andnewlinesInside. Right now it relies on the rule’s default type (alphabetical), so it validates newline handling but not the natural sort configuration in this block.For example:
Suggested tweak
- options: [ - { - groups: ['unknown', 'method'], - newlinesBetween: 1, - newlinesInside: 0, - }, - ], + options: [ + { + ...options, + groups: ['unknown', 'method'], + newlinesBetween: 1, + newlinesInside: 0, + }, + ],
8507-8526: Optional: align line-length “preserves comments” test with itsoptionsobjectSimilar to the natural section, this test’s options only specify
groups,newlinesBetween, andnewlinesInside, so it falls back to the rule’s default alphabetical type. If you want the test to truly live in the “line-length” suite, you could spread the localoptionsconstant here as well.Suggested tweak
- options: [ - { - groups: ['unknown', 'method'], - newlinesBetween: 1, - newlinesInside: 0, - }, - ], + options: [ + { + ...options, + groups: ['unknown', 'method'], + newlinesBetween: 1, + newlinesInside: 0, + }, + ],
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (69)
docs/content/rules/sort-array-includes.mdx(1 hunks)docs/content/rules/sort-classes.mdx(1 hunks)docs/content/rules/sort-decorators.mdx(1 hunks)docs/content/rules/sort-enums.mdx(1 hunks)docs/content/rules/sort-export-attributes.mdx(1 hunks)docs/content/rules/sort-exports.mdx(1 hunks)docs/content/rules/sort-heritage-clauses.mdx(1 hunks)docs/content/rules/sort-import-attributes.mdx(1 hunks)docs/content/rules/sort-imports.mdx(1 hunks)docs/content/rules/sort-interfaces.mdx(1 hunks)docs/content/rules/sort-intersection-types.mdx(1 hunks)docs/content/rules/sort-jsx-props.mdx(1 hunks)docs/content/rules/sort-maps.mdx(1 hunks)docs/content/rules/sort-modules.mdx(1 hunks)docs/content/rules/sort-named-exports.mdx(1 hunks)docs/content/rules/sort-named-imports.mdx(1 hunks)docs/content/rules/sort-object-types.mdx(1 hunks)docs/content/rules/sort-objects.mdx(1 hunks)docs/content/rules/sort-sets.mdx(1 hunks)docs/content/rules/sort-union-types.mdx(1 hunks)docs/content/rules/sort-variable-declarations.mdx(1 hunks)rules/sort-array-includes.ts(1 hunks)rules/sort-classes.ts(1 hunks)rules/sort-decorators.ts(1 hunks)rules/sort-enums.ts(1 hunks)rules/sort-export-attributes.ts(1 hunks)rules/sort-exports.ts(1 hunks)rules/sort-heritage-clauses.ts(1 hunks)rules/sort-import-attributes.ts(1 hunks)rules/sort-imports.ts(1 hunks)rules/sort-intersection-types.ts(1 hunks)rules/sort-jsx-props.ts(1 hunks)rules/sort-maps.ts(1 hunks)rules/sort-modules.ts(1 hunks)rules/sort-named-exports.ts(1 hunks)rules/sort-named-imports.ts(1 hunks)rules/sort-object-types.ts(1 hunks)rules/sort-objects.ts(1 hunks)rules/sort-union-types.ts(1 hunks)rules/sort-variable-declarations.ts(1 hunks)test/rules/sort-array-includes.test.ts(7 hunks)test/rules/sort-classes.test.ts(11 hunks)test/rules/sort-decorators.test.ts(5 hunks)test/rules/sort-enums.test.ts(7 hunks)test/rules/sort-exports.test.ts(9 hunks)test/rules/sort-heritage-clauses.test.ts(5 hunks)test/rules/sort-imports.test.ts(10 hunks)test/rules/sort-interfaces.test.ts(9 hunks)test/rules/sort-intersection-types.test.ts(9 hunks)test/rules/sort-jsx-props.test.ts(10 hunks)test/rules/sort-maps.test.ts(10 hunks)test/rules/sort-modules.test.ts(8 hunks)test/rules/sort-named-exports.test.ts(12 hunks)test/rules/sort-named-imports.test.ts(10 hunks)test/rules/sort-object-types.test.ts(9 hunks)test/rules/sort-objects.test.ts(7 hunks)test/rules/sort-sets.test.ts(7 hunks)test/rules/sort-union-types.test.ts(7 hunks)test/rules/sort-variable-declarations.test.ts(7 hunks)test/utils/get-newlines-between-option.test.ts(5 hunks)test/utils/sort-nodes-by-groups.test.ts(1 hunks)test/utils/validate-newlines-and-partition-configuration.test.ts(1 hunks)types/common-groups-options.ts(4 hunks)utils/get-newlines-between-option.ts(5 hunks)utils/json-schemas/common-groups-json-schemas.ts(2 hunks)utils/make-fixes.ts(1 hunks)utils/report-all-errors.ts(1 hunks)utils/report-errors.ts(2 hunks)utils/validate-newlines-and-partition-configuration.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
utils/report-errors.ts (1)
types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
utils/report-all-errors.ts (1)
types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
utils/validate-newlines-and-partition-configuration.ts (4)
types/common-groups-options.ts (2)
CommonGroupsOptions(75-98)NewlinesInsideOption(207-212)types/common-partition-options.ts (1)
CommonPartitionOptions(47-59)utils/is-group-with-overrides-option.ts (1)
isGroupWithOverridesOption(30-34)utils/is-newlines-between-option.ts (1)
isNewlinesBetweenOption(36-40)
test/utils/get-newlines-between-option.test.ts (2)
utils/get-newlines-between-option.ts (1)
GetNewlinesBetweenOptionParameters(17-26)types/common-groups-options.ts (4)
CustomGroupsOption(35-73)NewlinesBetweenOption(171-182)NewlinesInsideOption(207-212)GroupsOptions(232-237)
test/utils/validate-newlines-and-partition-configuration.test.ts (1)
utils/validate-newlines-and-partition-configuration.ts (1)
validateNewlinesAndPartitionConfiguration(101-121)
types/common-groups-options.ts (1)
rules/sort-imports/types.ts (1)
CustomTypeOption(96-96)
utils/make-fixes.ts (3)
types/sorting-node.ts (1)
SortingNode(18-102)types/common-partition-options.ts (1)
PartitionByCommentOption(30-45)types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
5a31529 to
1caabf9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/utils/validate-newlines-and-partition-configuration.test.ts (1)
90-105: Update test descriptions to mentionnewlinesInside, notnewlinesBetweenThese two
it.eachtests varynewlinesInside(ingroups/customGroups) and assert thenewlinesInsideerror message, but their descriptions still saynewlinesBetween: '%s'. This is misleading for readers and test output.Suggested fix:
Proposed description tweaks
- it.each([1, 0] as const)( - "throws an error when 'partitionByNewline' is enabled and `newlinesBetween: '%s'` exists in 'groups'", + it.each([1, 0] as const)( + "throws an error when 'partitionByNewline' is enabled and `newlinesInside: '%s'` exists in 'groups'", newlinesInside => { @@ - it.each([1, 0] as const)( - "throws an error when 'partitionByNewline' is enabled and `newlinesBetween: '%s'` exists in 'customGroups'", + it.each([1, 0] as const)( + "throws an error when 'partitionByNewline' is enabled and `newlinesInside: '%s'` exists in 'customGroups'", newlinesInside => {The rest of the assertions already line up with
newlinesInside.Also applies to: 107-122
🧹 Nitpick comments (6)
docs/content/rules/sort-imports.mdx (1)
274-305:newlinesInsidedocs and defaults forsort-importslook consistent with the new behaviorThe new
newlinesBetween/newlinesInsidedescriptions and examples (including thenewlinesInside: 0default in the config snippets) correctly reflect the split between between-group and in-group spacing and the special backward-compat default for this rule. As a minor clarity tweak, you might consider updating the earlier “newlines behavior” link under the'unsorted'type description to also reference#newlinesinside, since both options now participate in that behavior.Also applies to: 696-752
docs/content/rules/sort-import-attributes.mdx (1)
146-177:newlinesInsidedocumentation is accurate; consider tightening group typingsThe new
newlinesInsidesection, its default'ignore', and the “only whenpartitionByNewLineis false” note all line up with the intended behavior, and the example configs look good. One minor doc gap: thegroupsunion still documents{ newlinesBetween: number }, but per the broader option surface you also support'ignore'there; it would be clearer to shownewlinesBetween: number | 'ignore'(and to link “newlines behavior” for the'unsorted'type to both#newlinesbetweenand#newlinesinside).Also applies to: 178-222, 250-266, 272-335
test/rules/sort-intersection-types.test.ts (1)
821-903: NewnewlinesInside/newlinesBetweentests correctly encode the intended spacing semanticsThe added
newlinesInside: 0tests (including the explicit “removes newlines inside groups” case and the “handles newlines and comment after fixes” variants across alphabetical/natural/line‑length) look spot on: they enforce only in‑group spacing while leaving between‑group spacing tonewlinesBetween, and the updatednewlinesBetween: 0expectations now correctly preserve blank lines within groups. If you want to polish further, you could rename the newnewlinesInside‑focused tests to consistently say “inside groups” (rather than “between”) to match the option’s semantics, but behavior-wise this suite is in great shape.Also applies to: 1110-1134, 1570-1637, 2440-2480, 3972-4012, 4210-4244
test/rules/sort-variable-declarations.test.ts (1)
1030-1067:newlinesInsidecoverage for variable declarations matches the new modelThe updated tests around
newlinesInside—both via group overrides and the globalnewlinesInside: 0cases—nicely validate that only within-group spacing is affected, while the revisednewlinesBetweentests now focus purely on between-group spacing (with partition and dependency rules still taking precedence). The only nit is the alphabetical test named “removes newlinesInside between groups when newlinesInside is 0”, which is really exercising inside-group spacing; renaming it to “…inside groups…” would better reflect what’s being asserted, but the expectations themselves look correct.Also applies to: 1381-1425, 1428-1702, 3111-3371, 3972-4013, 4610-4677
test/rules/sort-objects.test.ts (1)
2072-2078: ExplicitnewlinesInside: 0keeps legacy intra‑group behavior in “preserves comments” testAdding
newlinesInside: 0alongsidegroups: ['unknown', 'method']andnewlinesBetween: 1preserves the previous behavior where both:
- one blank line is enforced between groups, and
- no blank lines appear inside the method group, while comments stay attached correctly.
One minor robustness thought: this test still relies on the rule’s default
type/orderrather than spreading...optionsas elsewhere. Ifsort-objectsdefaults ever change, this could become brittle; consider adding...optionshere (and in the analogousnatural/line-lengthcases) in a follow‑up for future‑proofing.utils/validate-newlines-and-partition-configuration.ts (1)
17-22: JSDoc and error messaging now slightly out of sync with behaviorThe new
NEWLINES_INSIDE_ERROR_MESSAGE/NEWLINES_BETWEEN_ERROR_MESSAGEcorrectly unify the error text, and validation now also coversnewlinesInside. However:
- The JSDoc describes only
newlinesBetweenconflicts and its examples still talk about a more specific per‑group error (“'newlinesBetween' objects can not be used in 'groups'…”), which no longer matches the thrown message.- There’s no mention that
newlinesInsideis also disallowed whenpartitionByNewLineis enabled (globally, ingroups, and incustomGroups).Consider updating the documentation block to mention
newlinesInsidealongsidenewlinesBetweenand to align the “Throws:” comments with the new shared error messages.Also applies to: 23-65, 166-184
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (69)
docs/content/rules/sort-array-includes.mdx(3 hunks)docs/content/rules/sort-classes.mdx(3 hunks)docs/content/rules/sort-decorators.mdx(3 hunks)docs/content/rules/sort-enums.mdx(3 hunks)docs/content/rules/sort-export-attributes.mdx(3 hunks)docs/content/rules/sort-exports.mdx(3 hunks)docs/content/rules/sort-heritage-clauses.mdx(3 hunks)docs/content/rules/sort-import-attributes.mdx(3 hunks)docs/content/rules/sort-imports.mdx(3 hunks)docs/content/rules/sort-interfaces.mdx(3 hunks)docs/content/rules/sort-intersection-types.mdx(3 hunks)docs/content/rules/sort-jsx-props.mdx(3 hunks)docs/content/rules/sort-maps.mdx(3 hunks)docs/content/rules/sort-modules.mdx(3 hunks)docs/content/rules/sort-named-exports.mdx(3 hunks)docs/content/rules/sort-named-imports.mdx(3 hunks)docs/content/rules/sort-object-types.mdx(3 hunks)docs/content/rules/sort-objects.mdx(3 hunks)docs/content/rules/sort-sets.mdx(3 hunks)docs/content/rules/sort-union-types.mdx(3 hunks)docs/content/rules/sort-variable-declarations.mdx(3 hunks)rules/sort-array-includes.ts(1 hunks)rules/sort-classes.ts(1 hunks)rules/sort-decorators.ts(1 hunks)rules/sort-enums.ts(1 hunks)rules/sort-export-attributes.ts(1 hunks)rules/sort-exports.ts(1 hunks)rules/sort-heritage-clauses.ts(1 hunks)rules/sort-import-attributes.ts(1 hunks)rules/sort-imports.ts(1 hunks)rules/sort-intersection-types.ts(1 hunks)rules/sort-jsx-props.ts(1 hunks)rules/sort-maps.ts(1 hunks)rules/sort-modules.ts(1 hunks)rules/sort-named-exports.ts(1 hunks)rules/sort-named-imports.ts(1 hunks)rules/sort-object-types.ts(1 hunks)rules/sort-objects.ts(1 hunks)rules/sort-union-types.ts(1 hunks)rules/sort-variable-declarations.ts(1 hunks)test/rules/sort-array-includes.test.ts(7 hunks)test/rules/sort-classes.test.ts(11 hunks)test/rules/sort-decorators.test.ts(5 hunks)test/rules/sort-enums.test.ts(7 hunks)test/rules/sort-exports.test.ts(9 hunks)test/rules/sort-heritage-clauses.test.ts(5 hunks)test/rules/sort-imports.test.ts(10 hunks)test/rules/sort-interfaces.test.ts(9 hunks)test/rules/sort-intersection-types.test.ts(9 hunks)test/rules/sort-jsx-props.test.ts(10 hunks)test/rules/sort-maps.test.ts(10 hunks)test/rules/sort-modules.test.ts(8 hunks)test/rules/sort-named-exports.test.ts(12 hunks)test/rules/sort-named-imports.test.ts(10 hunks)test/rules/sort-object-types.test.ts(9 hunks)test/rules/sort-objects.test.ts(7 hunks)test/rules/sort-sets.test.ts(7 hunks)test/rules/sort-union-types.test.ts(7 hunks)test/rules/sort-variable-declarations.test.ts(7 hunks)test/utils/get-newlines-between-option.test.ts(5 hunks)test/utils/sort-nodes-by-groups.test.ts(1 hunks)test/utils/validate-newlines-and-partition-configuration.test.ts(1 hunks)types/common-groups-options.ts(4 hunks)utils/get-newlines-between-option.ts(5 hunks)utils/json-schemas/common-groups-json-schemas.ts(2 hunks)utils/make-fixes.ts(1 hunks)utils/report-all-errors.ts(1 hunks)utils/report-errors.ts(2 hunks)utils/validate-newlines-and-partition-configuration.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/content/rules/sort-classes.mdx
🚧 Files skipped from review as they are similar to previous changes (36)
- rules/sort-array-includes.ts
- docs/content/rules/sort-objects.mdx
- rules/sort-intersection-types.ts
- rules/sort-union-types.ts
- docs/content/rules/sort-exports.mdx
- docs/content/rules/sort-named-exports.mdx
- rules/sort-enums.ts
- rules/sort-maps.ts
- docs/content/rules/sort-export-attributes.mdx
- docs/content/rules/sort-union-types.mdx
- docs/content/rules/sort-variable-declarations.mdx
- docs/content/rules/sort-array-includes.mdx
- rules/sort-modules.ts
- rules/sort-variable-declarations.ts
- test/utils/sort-nodes-by-groups.test.ts
- rules/sort-named-imports.ts
- test/rules/sort-heritage-clauses.test.ts
- rules/sort-imports.ts
- docs/content/rules/sort-modules.mdx
- docs/content/rules/sort-intersection-types.mdx
- rules/sort-decorators.ts
- docs/content/rules/sort-named-imports.mdx
- test/rules/sort-decorators.test.ts
- docs/content/rules/sort-decorators.mdx
- test/rules/sort-array-includes.test.ts
- test/rules/sort-sets.test.ts
- rules/sort-jsx-props.ts
- docs/content/rules/sort-heritage-clauses.mdx
- rules/sort-classes.ts
- docs/content/rules/sort-sets.mdx
- rules/sort-named-exports.ts
- test/rules/sort-named-exports.test.ts
- rules/sort-export-attributes.ts
- rules/sort-object-types.ts
- rules/sort-import-attributes.ts
- utils/report-all-errors.ts
🧰 Additional context used
🧬 Code graph analysis (4)
test/utils/validate-newlines-and-partition-configuration.test.ts (1)
utils/validate-newlines-and-partition-configuration.ts (1)
validateNewlinesAndPartitionConfiguration(101-121)
utils/make-fixes.ts (3)
types/sorting-node.ts (1)
SortingNode(18-102)types/common-partition-options.ts (1)
PartitionByCommentOption(30-45)types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
utils/report-errors.ts (1)
types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
types/common-groups-options.ts (1)
rules/sort-imports/types.ts (1)
CustomTypeOption(96-96)
🔇 Additional comments (76)
rules/sort-exports.ts (1)
73-73: LGTM! Default value correctly set to 'ignore'.The addition of
newlinesInside: 'ignore'to the default options is consistent with the PR objectives, which specify that all rules exceptsort-importsshould default to'ignore'.docs/content/rules/sort-jsx-props.mdx (1)
242-256: LGTM! Documentation clearly explains the new option.The documentation for
newlinesInsideis comprehensive and correctly:
- Specifies the type (
number | 'ignore') and default value ('ignore')- Explains the behavior for different values
- Notes that it's only applicable when
partitionByNewLineisfalse- Cross-references related options for per-group overrides
test/rules/sort-interfaces.test.ts (4)
1786-1827: LGTM! Test correctly validates newlinesInside behavior.This test properly verifies that when
newlinesInside: 0is set:
- Newlines within the same group (the 'unknown' group containing
b,y,z) are removed- Newlines between groups (method → unknown) are preserved since
newlinesBetweenis not setThe test correctly expects both ordering and spacing errors, and the output properly removes intra-group newlines while maintaining inter-group spacing.
1829-1869: LGTM! Test correctly demonstrates newlinesBetween vs newlinesInside separation.This test properly validates that when only
newlinesBetween: 0is set:
- Newlines between groups (method → unknown at line 1834) are removed
- Newlines within the unknown group (between
yandzat line 1857) are preserved becausenewlinesInsidedefaults to'ignore'This is the correct behavior and clearly demonstrates the separation of concerns between the two options.
4369-4409: LGTM! Natural sorting tests correctly mirror the alphabetical test patterns.The test properly validates
newlinesBetween: 0behavior in the natural sorting context, maintaining consistency with the alphabetical tests.
6767-6810: LGTM! Line-length sorting tests correctly mirror the established test patterns.The test properly validates
newlinesBetween: 0behavior in the line-length sorting context, maintaining consistency with other sorting type tests.test/rules/sort-union-types.test.ts (3)
822-903: NewnewlinesInside: 0test and adjustednewlinesBetweenexpectations look consistentThe new test for
newlinesInside: 0plus the nearbynewlinesBetween: 0case correctly exercise the separation of “inside group” vs “between groups” spacing: order andextraSpacingBetweenUnionTypesare both asserted, and the expected output keeps partitioning while normalizing the relevant newlines. No issues spotted here.
1100-1135: UsingnewlinesInside: 0to preserve legacy no‑blank‑lines semantics is appropriateAdding
newlinesInside: 0alongsidenewlinesBetween: 1in these tests keeps the original behavior (“no blank lines inside groups” while still enforcing 1 newline between groups) under the new option model. This is a good, explicit way to avoid regressions after decoupling inside vs between behavior.Also applies to: 2686-2721, 4213-4248
2449-2490: Tweaked outputs fornewlinesBetween: 0are compatible with the new modelThe minor adjustments to the expected outputs in the
newlinesBetween: 0cases correctly reflect that only between‑group spacing is mandated bynewlinesBetween, while any inside‑group normalization is now controlled bynewlinesInside. The resulting formatted unions remain sensible and in line with the rule’s intent.Also applies to: 3976-4016
docs/content/rules/sort-interfaces.mdx (1)
286-317:newlinesInsidedocumentation and defaults align with the new behaviorThe updated
newlinesBetweendescription, the newnewlinesInsidesubsection (including applicability and group/customGroups overrides), and the default config examples withnewlinesInside: 'ignore'accurately describe and reflect the new option semantics for this rule. Looks good.Also applies to: 445-460, 575-583, 777-812
rules/sort-objects.ts (1)
69-85: DefaultnewlinesInside: 'ignore'wiring is correctAdding
newlinesInside: 'ignore'intodefaultOptionscleanly exposes the new option forsort-objectswhile keeping existing behavior unchanged until users opt in. Integration with the rest of the rule (option completion, validation, grouping) is consistent.utils/report-errors.ts (1)
3-6: StandardizingoptionsonCommonGroupsOptionsmatches fix logic needsSwitching the
optionsparameter toCommonGroupsOptions<unknown, unknown, string>givesreportErrors/makeFixesthe shared grouping and newline configuration (includingnewlinesInside) without over‑constraining callers. Given structural typing, passing fuller rule option objects still type‑checks and runtime behavior remains intact.Also applies to: 45-61, 111-147
rules/sort-heritage-clauses.ts (1)
48-62: Heritage‑clauses now correctly exposenewlinesInsidewith a safe defaultAdding
newlinesInside: 'ignore'todefaultOptionsand running it through the existing validation pipeline cleanly enables inside‑group newline control for heritage clauses without changing defaults.docs/content/rules/sort-enums.mdx (1)
215-246:newlinesInsidedocs and examples for enums are coherent with the new APIThe new
newlinesInsidesection, the clarifiednewlinesBetweendescription, and the updated config examples (includingnewlinesInside: 'ignore') accurately reflect the rule’s new spacing controls and their interaction withgroups/customGroupsandpartitionByNewLine.Also applies to: 247-363, 371-402
docs/content/rules/sort-object-types.mdx (1)
251-282: Object‑types docs correctly incorporatenewlinesInsideand its overridesThe updated
newlinesBetweenwording, the newnewlinesInsideoption description, the per‑group/customGroupnewlinesInsidebullets, and the default config examples all line up with the new spacing model for object types. This keeps the rule’s documentation consistent with the shared options surface.Also applies to: 413-551, 618-667, 724-751, 760-783
docs/content/rules/sort-maps.mdx (5)
214-220: Clear documentation ofnewlinesBetweenbehavior and constraints.The updated wording clarifies the option's behavior and correctly documents the constraint that it only applies when
partitionByNewLineisfalse. The cross-reference to the groups section is accurate.
221-235: Well-structured documentation for the newnewlinesInsideoption.The documentation clearly explains the option's purpose, behavior, and constraints. The distinction between "inside groups" vs "between groups" is clear, and the cross-references to override mechanisms are accurate.
294-327: Correct integration ofnewlinesInsideinto groups type definition.The type signature and description properly document the ability to override
newlinesInsidefor specific groups.
368-405: Consistent documentation ofnewlinesInsidein custom groups.The type definitions and attribute descriptions properly document the
newlinesInsideoverride capability for both custom group definition types.
439-440: Usage examples correctly reflect the new defaults.Both configuration examples show the correct default values for
newlinesBetweenandnewlinesInsideas'ignore'.Also applies to: 471-472
test/rules/sort-imports.test.ts (10)
1283-1327: NewnewlinesInside: 0test exercises intra‑group spacing vs inter‑group spacing correctlyThis case nicely captures that with
customGroups+groups: ['a', 'unknown']andnewlinesBetween: 'ignore', only the blank line inside theunknowngroup (z→b) is removed while the inter‑group spacing afterais preserved. The dual assertions (unexpectedImportsOrderandextraSpacingBetweenImports) match that intent.
1734-1739: ExplicitnewlinesInside: 'ignore'keeps newline‑partitioning test focused on orderingFor the “allows partitioning by new lines” (alphabetical) case, adding
newlinesInside: 'ignore'alongsidenewlinesBetween: 'ignore'ensures the test continues to validate only partitioning and ordering, without being affected by the new defaultnewlinesInside: 0for this rule.
3123-3137: Dependencies vs newline partitions (alphabetical) remain well‑specified withnewlinesInside: 'ignore'Here
newlinesBetween: 'ignore'+partitionByNewLine: true+newlinesInside: 'ignore'cleanly isolates the behavior under test: dependency ordering should override partition boundaries, but we don’t want incidental spacing fixes inside partitions. The single dependency error expectation is consistent with that.
3195-3201: Content‑separation‑over‑dependencies test (alphabetical) correctly neutralizes inside‑group spacingAdding
newlinesInside: 'ignore'matches the description: we’re asserting that explicit content separation wins over dependency ordering, not testing newline normalization, so ignoring inside‑group newlines here is appropriate.
5182-5187: Natural sort: partition‑by‑newline test aligned with alphabetical vianewlinesInside: 'ignore'This mirrors the alphabetical test:
newlinesBetween: 'ignore'+newlinesInside: 'ignore'keeps the behavior focused purely on partitioning and ordering, independent of the new default inside‑group newline policy.
6399-6412: Natural sort: dependencies overriding newline partitions with spacing ignored is consistentThe combination of
newlinesBetween: 'ignore',partitionByNewLine: true, andnewlinesInside: 'ignore'again ensures only the dependency ordering is asserted, with a singleunexpectedImportsDependencyOrdererror, matching the test name and intent.
6469-6476: Natural sort: content separation vs dependencies usesnewlinesInside: 'ignore'appropriatelyAs in the alphabetical block, explicitly ignoring inside‑group newlines prevents the new default from altering the partition structure this test is verifying, so expectations stay stable.
8485-8515: Line‑length: newline‑partitioning test now shields inside‑group spacing from the new defaultFor the line‑length variant of “allows partitioning by new lines”, adding
newlinesInside: 'ignore'lets you keep the test focused on partition partitioning and relative order (./atomsvs./shared), while leaving the single blank line between partitions intact under the new defaultnewlinesInside: 0.
9740-9755: Line‑length: dependencies over newline partitions correctly ignore spacing fixesThis matches the earlier sections: with
newlinesBetween: 'ignore',partitionByNewLine: true, andnewlinesInside: 'ignore', only the dependency violation is expected, so the singleunexpectedImportsDependencyOrdererror is appropriate.
9768-9817: Line‑length: content‑separation‑over‑dependencies test neutralizes inside‑group newlinesAgain, explicitly passing
newlinesInside: 'ignore'ensures that content separation vs dependency ordering is what’s being exercised, not the inside‑group newline normalization introduced by the new option. Expectations align with that goal.utils/get-newlines-between-option.ts (4)
42-65: LGTM!The function correctly delegates to
computeNewlinesInsideOptionfor same-group nodes,computeNewlinesBetweenOptionForDifferentGroupsfor non-adjacent groups, and returnsoptions.newlinesBetweendirectly for adjacent different groups. This aligns well with the new options-based resolution approach.
67-111: LGTM!The function correctly handles inline group overrides and falls back to
options.newlinesBetweenwhen no override is specified. The priority logic (max number → ignore → 0) is well-structured.
113-138: LGTM!The priority chain is correctly implemented:
customGroup.newlinesInside→groupOverrideNewlinesInside→options.newlinesInside. This aligns with the PR objective of makingnewlinesInsideconfigurable at multiple scopes (group-level → rule-level → settings-level → default).
165-185: LGTM!The helper function correctly inserts the global
newlinesBetweenoption between adjacent groups that don't have explicit newlines settings.test/rules/sort-named-imports.test.ts (4)
1097-1144: LGTM!The test correctly verifies that
newlinesInside: 0removes newlines within theunknowngroup while preserving the blank lines between groups (sincenewlinesBetweendefaults to'ignore').
1182-1190: Verify expected whitespace behavior in output.The output preserves a blank line between
yandzwithin theunknowngroup. This is consistent withnewlinesInsidedefaulting to'ignore'(intra-group newlines are not enforced). However, please verify this is the intended behavior, as the blank line appears to be residual whitespace carried over during reordering rather than an intentional preservation.
1433-1436: LGTM!Adding explicit
newlinesInside: 0clarifies the test's intent and aligns with the new option's behavior. This pattern is correctly applied across the alphabetical, natural, and line-length test sections.
2568-2598: LGTM!The error expectations are correctly updated to reflect the new error reporting order. The pattern is consistent with similar tests in the file.
utils/make-fixes.ts (2)
125-141: LGTM!The options object is correctly passed to
makeNewlinesBetweenFixes, which will now receivenewlinesInsidealongside other configuration. The fix orchestration logic properly handles the expanded options.
18-23: LGTM! Type simplification improves consistency.Using
CommonGroupsOptions<unknown, unknown, string>directly instead of a Pick is cleaner and automatically includesnewlinesInsidealongsidenewlinesBetween,groups, andcustomGroups. This ensures the options object consistently carries all grouping-related configuration.The removal of the
exportmodifier is appropriate since the interface is only used internally within the utils module.test/rules/sort-objects.test.ts (6)
1746-1787: Good focused coverage fornewlinesInside: 0on grouped configThis new test clearly exercises that
newlinesInside: 0on the rule options (withgroups: ['method', 'unknown']) both:
- triggers an order fix within the
unknowngroup (zvsb), and- reports and fixes extra spacing inside that group while leaving inter‑group spacing alone.
The expectations (errors and
output) match the intended contract of the newnewlinesInsideoption and the PR description.
1789-1820:newlinesBetween: 0test now isolates inter‑group spacing vs intra‑group spacingUpdating the first error’s
datato{ right: 'y', left: 'a' }and changing theoutputto keep a blank line betweenyandzmakes this test accurately reflect the new semantics:
- The only spacing violation comes from the boundary between the
methodandunknowngroups (controlled bynewlinesBetween: 0).- The retained blank line between
yandzis intra‑group and thus ignored under the defaultnewlinesInside: 'ignore'.This nicely documents the new separation of concerns between the two options.
5615-5621: Natural-ordernewlinesBetween: 0output aligns with decouplednewlinesInsideThe updated
outputthat keeps a blank line betweenyandzwhile collapsing spacing betweenaandbmirrors the alphabetical case and matches the new contract:
newlinesBetween: 0only normalizes spacing at group boundaries.- Intra‑group blank lines are preserved because
newlinesInsideisn’t set (defaults to'ignore').This is consistent with the PR’s behavioral description.
5874-5880: Natural-order “preserves comments…” test correctly opts into legacy intra‑group spacingMirroring the alphabetical block, adding
newlinesInside: 0here ensures the method group remains compact (no internal blank lines) while still inserting exactly one blank line between theunknownandmethodgroups, preserving the comment layout.Configuration and expected output look coherent with the new option semantics.
8261-8267: Line-lengthnewlinesBetween: 0test now preserves intra‑group blank lineThe line‑length variant of “removes newlines between groups when newlinesBetween is 0” now retains a blank line in the
unknowngroup while collapsing spacing at the group boundary, just like the alphabetical/natural variants.This ensures all three sorting modes exercise the same separation between
newlinesBetween(inter‑group) and the now‑independentnewlinesInside.
8520-8526: Line-length “preserves comments…” test explicitly constrains intra‑group spacingAdding
newlinesInside: 0alongsidegroups: ['unknown', 'method']andnewlinesBetween: 1in the line‑length suite keeps this test aligned with prior expectations: one newline between groups, none within, and comments preserved.This mirrors the alphabetical and natural blocks and gives consistent coverage across all sort types.
test/rules/sort-maps.test.ts (3)
947-1041:newlinesInsidevsnewlinesBetweensemantics in alphabetic map tests look consistent
- The new
"removes extra newlines inside groups when newlinesInside is 0"test (lines 947–993) correctly:
- Uses a custom group
'a'vs'unknown'andnewlinesInside: 0at rule level.- Asserts both an order error (
unexpectedMapElementsOrderfor'b'vs'z') and an intra‑group spacing error (extraSpacingBetweenMapElementsMembersfor'b'vs'z'), which matches the re‑ordering and newline removal inside the'unknown'group incode→output.- The updated
"removes extra newlines between groups when newlinesBetween is 0"test (lines 996–1041) correctly:
- Relies only on
newlinesBetween: 0with defaultnewlinesInside: 'ignore'.- Expects a spacing error between groups (
extraSpacingBetweenMapElementsMembersfor'y'vs'a') and an order error within the'unknown'group (unexpectedMapElementsOrderfor'b'vs'z'), while preserving an internal blank line inside the group in theoutput, which matches the new behavior that intra‑group newlines are ignored unlessnewlinesInsideis set.- Net: the pair of tests clearly separates responsibilities of
newlinesInsidevsnewlinesBetweenand matches the PR’s described behavior.
1282-1291: AddingnewlinesInside: 0to inline‑comment preservation tests is appropriate
- In the three
"preserves inline comments when reordering elements"tests (alphabetical, natural, line‑length), addingnewlinesInside: 0alongsidenewlinesBetween: 1ensures:
- The fixer cannot “explain away” blank lines by the new default
newlinesInside: 'ignore'; it must normalize intra‑group spacing to zero while still reordering.- The expectations about how inline comments are laid out remain stable across the behavioral change in defaults.
- None of these blocks use
partitionByNewLine, so the configuration is valid under the new constraints.Also applies to: 2620-2632, 3964-3976
2337-2383: Natural and line‑lengthnewlinesBetween: 0tests now assert only inter‑group spacing, which matches the new default
- In both natural and line‑length “removes extra newlines between groups when newlinesBetween is 0” tests, the updated
errorsarrays:
- Keep a single spacing error at the group boundary (
extraSpacingBetweenMapElementsMemberswith left = the last element of the first group, right = first of the next group).- Keep or add a separate order error only where the group’s internal ordering changes (
unexpectedMapElementsOrder), and no longer report spacing inside groups (sincenewlinesInsideis now implicitly'ignore'there).- The
outputstrings preserve a blank line inside the second group while collapsing the inter‑group gap, which aligns precisely with “between‑group only” enforcement whennewlinesInsideis unset.Also applies to: 3678-3727
test/rules/sort-exports.test.ts (3)
955-1042: ExportsnewlinesInside: 0test plusnewlinesBetween: 0regression check look correct
- The new
"removes newlines inside groups when newlinesInside is 0"test (lines 955–998):
- Uses a custom
'a'group and'unknown'group withnewlinesInside: 0.- Correctly expects both an order error and an
extraSpacingBetweenExportserror between'z'and'b', matching the intra‑group newline removal and re‑ordering incode→output.- The adjacent
"removes newlines between groups when newlinesBetween is 0"test (lines 1000–1042) now:
- Only reports
extraSpacingBetweenExportsat the group boundary ('a'/'y') and an order error for'b'vs'z'.- Leaves one blank line inside the
'unknown'group in theoutput, showing that intra‑group spacing is no longer enforced whennewlinesInsideis not set.- Together these tests mirror the semantics shift:
newlinesInsidecontrols inner‑group spacing,newlinesBetweenkeeps handling between‑group spacing.
1280-1291: UsingnewlinesInside: 0in exports inline‑comment tests keeps expectations stable
- In the three
"preserves inline comments when reordering elements"tests across alphabetical, natural, and line‑length modes, the addition ofnewlinesInside: 0:
- Forces the rule to normalize intra‑group blank lines to zero, ensuring that the only variability under test is ordering and comment preservation, not optional internal spacing.
- Prevents the new default (
'ignore') from hiding regressions in how comments are moved when entries are re‑ordered.- Configs don’t combine
newlinesInsidewithpartitionByNewLine, so they remain valid under the new validation rules.Also applies to: 2620-2632, 3964-3976
2338-2380:newlinesBetween: 0exports tests correctly limit spacing assertions to between‑group gaps
- In the alphabetical and natural
"removes newlines between groups when newlinesBetween is 0"tests, the updatederrors:
- Retain a single
extraSpacingBetweenExportserror at the transition from group'a'to group'unknown'.- Drop any spacing assertions inside the
'unknown'group, while keepingunexpectedExportsOrderwhere reordering actually occurs, aligning withnewlinesInsidenow defaulting to'ignore'.- In the line‑length variant (lines 3712–3750), the test only asserts the inter‑group spacing error:
- The unknown‑group members’ lines are all equivalent in length, so leaving them in original order and not emitting an order error is reasonable.
- The
outputcollapses just the group boundary gap and leaves inner spacing as‑is, again matching the new separation of concerns.- Overall these tests give good coverage that
newlinesBetween: 0no longer incidentally enforces intra‑group spacing.Also applies to: 3712-3750
test/rules/sort-modules.test.ts (5)
1794-1839: LGTM! Comprehensive test for newlinesInside removal behavior.The test correctly validates that:
- Elements are reordered according to group hierarchy
- Elements within a group are sorted alphabetically
- Extra spacing within groups is flagged and removed when
newlinesInside: 0The expected output correctly shows no blank lines between functions (within the 'unknown' group), while the blank line before the interface (between groups) is preserved since only
newlinesInsideis configured.
1841-1883: LGTM! Test correctly validates newlinesBetween independently from newlinesInside.With only
newlinesBetween: 0configured andnewlinesInsidedefaulting to'ignore', the test correctly expects:
- 2 errors (group order and element order) - no extra spacing error
- Output preserves existing blank lines within the 'unknown' group
- No blank line between the 'unknown' and 'interface' groups
2188-2206: LGTM! Test correctly validates combined newlinesBetween and newlinesInside behavior.The configuration
newlinesBetween: 1withnewlinesInside: 0correctly produces:
- One blank line between 'function' and 'type' groups
- No blank lines between
type Bandtype C(within the same 'type' group)
4281-4298: Consistent test coverage in the natural sorting section.This test mirrors the alphabetical section test, correctly validating that
newlinesBetween: 0removes between-group spacing while preserving within-group spacing whennewlinesInsidedefaults to'ignore'.
6657-6674: Consistent test coverage in the line-length sorting section.Maintains parity with alphabetical and natural sections for the
newlinesBetween: 0behavior.types/common-groups-options.ts (3)
207-212: Well-designed type that mirrors NewlinesBetweenOption.The
NewlinesInsideOptiontype is consistent with the existingNewlinesBetweenOptiondesign, using'ignore' | numberfor the same semantics. The JSDoc comments clearly distinguish the purpose (elements within a group vs. between groups).
75-98: Clean consolidation of common group options.The new
CommonGroupsOptionsinterface effectively consolidates the four related options (customGroups,groups,newlinesBetween,newlinesInside) into a reusable type. All fields being required is appropriate since rules will provide defaults for these options.
46-48: Consistent optional newlinesInside in group-level configurations.Making
newlinesInsideoptional in bothCustomGroupsOptionandGroupWithOverridesOptioncorrectly allows per-group overrides while falling back to the rule-level setting when not specified. This aligns with the PR's precedence chain: group-level → rule-level → settings-level → default.Also applies to: 110-112
test/rules/sort-enums.test.ts (2)
1094-1141: LGTM! Test correctly validatesnewlinesInside: 0behavior.The new test properly verifies that
newlinesInside: 0removes blank lines within a group while preserving between-group spacing. The error expectations correctly identify both the ordering violation and the extra spacing violation within the 'unknown' group.
1143-1189: Clarify unexpected blank line between elements within the same group in test expectations.The tests "removes newlines between groups when newlinesBetween is 0" across all three sorting types (alphabetical, natural, line-length) show an unexpected pattern: the expected output contains a blank line between
YandZ, even though:
YandZare both in the'unknown'group (same group)- The input code has no blank line between them
newlinesInsideis not set (defaults to'ignore')newlinesBetween: 0should only control spacing between different groupsThis pattern repeats consistently across all three test variants, suggesting intentional implementation behavior rather than a test error. However, it's unclear why elements within the same group would receive a blank line added between them during sorting when no such separation existed in the input.
Please clarify:
- Is this the intended behavior when elements are repositioned during sorting with
newlinesInside: 'ignore'?- Should spacing from the input be preserved or does the implementation have different logic for handling inter-element spacing during reordering?
test/rules/sort-object-types.test.ts (3)
1645-1685: Good separation of inside‑ vs between‑group spacing semanticsThe two new tests cleanly assert that
newlinesInside: 0only removes blank lines within a group whilenewlinesBetween: 0removes extra spacing between groups but preserves intra‑group spacing under the defaultnewlinesInside: 'ignore'. Expectations and diff outputs look consistent with the intended semantics and grouping.Also applies to: 1688-1727
4931-4970: Consistent coverage ofnewlinesBetween: 0across sorting strategiesThe added “removes newlines between groups when newlinesBetween is 0” tests for
naturalandline-lengthmirror the alphabetical behavior with appropriately adjusted element names. They confirm that group reordering and spacing are enforced identically regardless of sort strategy.Also applies to: 7288-7330
1965-2000: Ensuring previous behavior withnewlinesInside: 0in comment-sensitive testsAdding
newlinesInside: 0alongsidenewlinesBetween: 1in the “handles newlines and comment after fixes” tests across all strategies keeps intra‑group spacing strict under the new option model, preserving the original behavior while still validating comment handling. Looks correct.Also applies to: 5207-5243, 7567-7603
test/utils/get-newlines-between-option.test.ts (1)
3-8: Tests correctly encodenewlinesInsidevsnewlinesBetweenprecedenceThe extended suite around
get-newlines-between-optionnow:
- Verifies that for different groups the global
newlinesBetweenis used, and for same groupsnewlinesInsideis returned.- Exercises custom-group
newlinesInsideoverrides vs the global setting.- Uses strongly typed parameters via
NewlinesBetweenOption,NewlinesInsideOption,CustomGroupsOption, andGroupsOptions.- Ensures
buildParametersalways passesnewlinesInside(defaulting to'ignore') and a concretecustomGroupsarray, which matches how the production utility is expected to be called.Behavior and typings look consistent with the intended option model.
Also applies to: 16-80, 120-141, 327-359
test/utils/validate-newlines-and-partition-configuration.test.ts (1)
6-55: Comprehensive coverage of partition vs newline optionsThe new test blocks around
'newlinesBetween'and'newlinesInside'do a good job of pinning down the intended matrix:
- Non‑'ignore'
newlinesBetweenornewlinesInsideis rejected whenpartitionByNewLineis true, both globally and via group objects.- All values are allowed when
partitionByNewLineis false.- The final test confirms that partitions are accepted when all related options — global and per‑group/customGroup — are
'ignore'.These expectations are aligned with the updated validator implementation.
Also applies to: 57-89, 125-139
utils/validate-newlines-and-partition-configuration.ts (1)
123-164: The code is already type-safe. ThecustomGroupsandgroupsproperties are required (non-optional) in theCommonGroupsOptionsinterface, so they cannot beundefinedat the function call site. TypeScript's type system enforces this at compile time, making the for-of loops safe to use without defensive null-coalescing checks.Likely an incorrect or invalid review comment.
test/rules/sort-jsx-props.test.ts (3)
848-895: LGTM! Well-designed test fornewlinesInside: 0behavior.The test correctly validates that
newlinesInside: 0removes newlines within groups while preserving between-group spacing. The error expectations (order + spacing violations) are accurate, and the intentional indentation variations properly test preservation of original formatting.
897-943: LGTM! Correctly updated to reflect new defaultnewlinesInside: 'ignore'behavior.The test now properly validates that
newlinesBetween: 0removes spacing between groups while the defaultnewlinesInside: 'ignore'preserves existing newlines within groups (line 938). Error expectations accurately reflect both between-group and within-group violations.
1205-1205: LGTM! Appropriate addition of explicitnewlinesInside: 0configuration.Adding explicit
newlinesInside: 0to these comment-preservation tests ensures deterministic behavior and prevents reliance on the new 'ignore' default. This makes test expectations clearer and more maintainable.Also applies to: 2455-2455, 3708-3708
test/rules/sort-classes.test.ts (5)
3828-3869:newlinesInside: 0intra‑group behavior is well coveredThis new test cleanly exercises the distinction between sorting (
unexpectedClassesOrder) and intra‑group spacing (extraSpacingBetweenClassMembers) whennewlinesInsideis explicitly0. The expectations for reorderingy/z/band collapsing extra blank lines inside the non‑method group look consistent with the new option semantics.
3871-3911:newlinesBetween: 0tests correctly isolate inter‑group spacing across all sort typesIn all three variants (“alphabetical”, “natural”, “line‑length”), these tests now:
- Flag only spacing between the method group and the following group (
extraSpacingBetweenClassMembers), and- Keep intra‑group blank lines intact, validating that
newlinesBetweenno longer governs within‑group spacing.The error metadata (left/right member names) and the fixed outputs match that intent for each sort type.
Also applies to: 8381-8421, 12835-12871
4185-4204: Comment‑stability withnewlinesInside: 0is explicitly exercisedAdding
newlinesInside: 0alongsidegroups: ['property', 'method']andnewlinesBetween: 1in the “handles newlines and comment after fixes” tests for all three strategies nicely checks that:
- The intra‑group newline normalization doesn’t break association of the trailing comment with
a, and- Inter‑group spacing (one blank line) is preserved.
The scenarios and outputs look coherent with the new configuration semantics.
Also applies to: 8695-8714, 13145-13164
13628-13666: Group‑level override ofnewlinesInsideis validated correctlyThis test now confirms that a group object in
groupscan override:
type/order(alphabetical desc onunknown), andnewlinesInside: 1,while the rule as a whole is
type: 'unsorted'. The dual errors (order + missed spacing) and resulting single blank line betweenbandaare exactly what you’d expect from that override.
14392-14526: Custom‑groupnewlinesInsidebehavior and overload special‑casing are well covered
The “allows to use newlinesInside: 1/0” tests for
methodsWithNewlinesInside/methodsWithoutNewlinesInsidecorrectly verify:
- Insertion of a single blank line between methods when
newlinesInside: 1.- Removal of extra blank lines when
newlinesInside: 0, while leaving the partition betweena(unknown) and the method group untouched.The
it.eachfor overloads withnewlinesInsidein{1, 0}asserts that overload signatures still cannot have blank lines between them (twoextraSpacingBetweenClassMemberserrors), which preserves the existing invariant that overload blocks are always tightly packed regardless of newline configuration.Together these give solid coverage for both regular groups and the overload special case under the new
newlinesInsideoption.
- 👉 Set default option to `ignore`. - ⏳ Set default option to `0`.
- ✅ Set default option to `ignore`. - 👉 Set default option to `0`.
- ℹ️ Hide whitespace changes for this commit.
1caabf9 to
482a8fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (3)
docs/content/rules/sort-named-imports.mdx (3)
276-276: Fix type definition inconsistency.The type definition shows
newlinesInside?: numberbut the option description at line 249 specifies the type asnumber | 'ignore'. This inconsistency will mislead users and cause TypeScript errors when they try to use the'ignore'value at the group level.🔎 Proposed fix
- newlinesInside?: number; + newlinesInside?: number | 'ignore';
366-366: Fix type definition inconsistency.The type definition shows
newlinesInside?: numberbut should benumber | 'ignore'to match the option description at line 249.🔎 Proposed fix
- newlinesInside?: number + newlinesInside?: number | 'ignore'
382-382: Fix type definition inconsistency.The type definition shows
newlinesInside?: numberbut should benumber | 'ignore'to match the option description at line 249.🔎 Proposed fix
- newlinesInside?: number + newlinesInside?: number | 'ignore'
♻️ Duplicate comments (1)
utils/json-schemas/common-groups-json-schemas.ts (1)
28-41: Past review concerns about schema description and per-group type remain unaddressedThe description at line 32 still reads "between groups elements" rather than the clearer "inside groups between elements", and the schema continues to allow
'ignore' | numberfor per-group/customGroup contexts without clarification of whether'ignore'is intentionally supported at those scopes.Also applies to: 175-175, 209-209, 291-291
🧹 Nitpick comments (13)
docs/content/rules/sort-decorators.mdx (3)
261-261: Clarify the allowed number range.The phrase "Any other number" is imprecise. Consider specifying "Any positive number" or explicitly documenting whether negative numbers are valid and how they're handled.
🔎 Suggested wording improvement
-- Any other number — Enforce this number of newlines between each group. +- Any positive number — Enforce this exact number of newlines between each group.
277-277: Clarify the allowed number range.Same issue as line 261: "Any other number" should be more precise. Consider "Any positive number" or explicit validation guidance.
🔎 Suggested wording improvement
-- Any other number — Enforce this number of newlines between each element of the same group. +- Any positive number — Enforce this exact number of newlines between each element of the same group.
268-282: Consider documenting settings-level configuration and precedence.The PR objectives mention that
newlinesInsidecan be configured at the settings level (e.g.,settings.perfectionist.newlinesInside) with precedence: group-level → rule-level → settings-level → default. Consider adding a note about this precedence chain to help users understand how the option is resolved.Example addition after line 271:
If not set at the rule level, the value will fall back to `settings.perfectionist.newlinesInside`, then to the default.docs/content/rules/sort-variable-declarations.mdx (1)
214-228: Consider mentioning settings-level configuration.The
newlinesInsidedocumentation is clear and complete for rule-level usage. However, the PR objectives indicate this option can also be configured globally viasettings.perfectionist.newlinesInside. While settings-level configuration is typically documented in general configuration guides, a brief mention or cross-reference here could help users discover this capability.Optional: Add note about settings-level configuration
After line 217, consider adding:
<sub>default: `'ignore'`</sub> + +This option can also be configured globally via `settings.perfectionist.newlinesInside`. Rule-level settings take precedence over global settings. Specifies how to handle newlines inside groups.test/rules/sort-named-imports.test.ts (1)
2553-2599: Consistent test updates for natural sort type.The changes mirror the alphabetical section's test updates, properly validating
newlinesBetween: 0behavior with the new semantics. The test structure is consistent across sort types.Optional: Consider adding a dedicated "removes newlines inside groups when newlinesInside is 0" test in the natural and line-length sections to match the alphabetical section's coverage, even though the behavior should be identical across sort types. This would make the test structure more uniform and easier to maintain.
utils/report-errors.ts (1)
3-5: Keepoptionstyping coupled tomakeFixesto avoid future driftSwitching
ReportErrorsParameters.optionstoCommonGroupsOptions<unknown, unknown, string>works for current usage (it’s only passed through tomakeFixes), but it breaks the direct link toMakeFixesParameters<T>['options']. IfmakeFixes’soptionstype ever widens or adds required fields beyond the common groups surface, this file won’t get a compiler error even though it forwardsoptionsstraight through.Consider typing
optionsviaMakeFixesParameters<T>['options'](or a shared exported alias) and, if needed, constraining that to extendCommonGroupsOptionsso both places stay in sync.Also applies to: 45-61, 111-145
test/rules/sort-variable-declarations.test.ts (1)
1381-1426: Misleading test name: "newlinesInside" vs "between groups".The test is named
'removes newlinesInside between groups when newlinesInside is 0', butnewlinesInsidecontrols spacing within groups, not between groups. Based on the test setup withgroups: ['a', 'unknown']andnewlinesInside: 0, this test should be validating that newlines inside theunknowngroup are removed.Consider renaming to clarify the intent, e.g.,
'removes newlines inside groups when newlinesInside is 0'.types/common-groups-options.ts (1)
207-213: Consider consolidating duplicate type definitions.
NewlinesInsideOptionis structurally identical toNewlinesBetweenOption(lines 171-182):export type NewlinesBetweenOption = 'ignore' | number export type NewlinesInsideOption = 'ignore' | numberWhile having separate types provides semantic clarity and allows future divergence, you could consider defining a shared base type if these are intended to remain identical.
🔎 Optional: Extract a shared type
// Shared base type export type NewlinesOption = 'ignore' | number // Semantic aliases (allows future divergence if needed) export type NewlinesBetweenOption = NewlinesOption export type NewlinesInsideOption = NewlinesOptiontest/rules/sort-objects.test.ts (2)
1746-1787: Rule-levelnewlinesInside: 0test accurately targets intra‑group spacingThe fixture and expectations correctly assert that:
- properties are reordered within the
unknowngroup (b,y,z) with theunexpectedObjectsOrdererror on{ right: 'b', left: 'z' }, and- extra blank lines inside that group trigger
extraSpacingBetweenObjectMembers, while the double blank between themethodandunknowngroups is left alone (sincenewlinesBetweenis not set).If you want to make this even clearer, you could simplify the input so that the only extra blank lines are between same‑group members (no double blank between groups), which would make the test’s intent more immediately obvious, but the current case is still valid.
2060-2078: AddingnewlinesInside: 0preserves prior “no extra blank lines within groups” behaviorIn the three
"preserves comments and handles newlines after fixes"tests you now pass:options: [ { groups: ['unknown', 'method'], newlinesBetween: 1, newlinesInside: 0, }, ]This is a good adjustment: after decoupling inside‑group spacing from
newlinesBetween, these tests would otherwise stop flagging internal blank lines. WithnewlinesInside: 0, the fixer still collapses intra‑group spacing while keeping exactly one blank line between theunknownandmethodgroups, preserving the original intent of the tests across alphabetical, natural, and line‑length modes.If you want to be ultra‑explicit, you could also include the sort
type/orderhere (e.g. spread...optionsas done in nearby tests), but relying on the defaults is acceptable since the scenarios don’t depend on the specific sort strategy.Also applies to: 5861-5880, 8507-8526
docs/content/rules/sort-interfaces.mdx (1)
286-317: Docs correctly describe newlinesInside/newlinesBetween behavior and constraintsThe updated docs for
newlinesBetweenand the newnewlinesInsidesection line up with the implementation:
- Types and defaults (
number | 'ignore', default'ignore') are accurate.- The “only applicable when
partitionByNewLineis false” note matches the validator’s enforcement.- Cross-links to
groups/customGroupsand the “Newlines between groups” subsection are correct and help users understand per-group overrides.- Config examples including
newlinesInside: 'ignore'keep defaults explicit and consistent.Only optional nit: if you expect users to ever set
newlinesInside: 'ignore'at group/customGroup level, you might mention'ignore'alongsidenumberin those type snippets too, but it’s not strictly necessary.Also applies to: 445-460, 651-699, 769-812
utils/validate-newlines-and-partition-configuration.ts (1)
1-22: Validation logic for partition/newlines options looks correct; JSDoc could be updatedThe new validation flow properly enforces:
- When
partitionByNewLineistrue:
- global
newlinesBetweenandnewlinesInsidemust both be'ignore';- group-level
{ newlinesBetween: ... }must be'ignore'if present;- group/customGroup
newlinesInsideoverrides must beundefinedor'ignore'.- When
partitionByNewLineisfalse, the function leavesnewlinesBetween/newlinesInsidefree for the dedicated newline validators to handle.The use of
CommonGroupsOptionspicks,isNewlinesBetweenOption, andisGroupWithOverridesOptionis sound, and the shared error messages keep behavior consistent.One thing to adjust for clarity: the big JSDoc above
validateNewlinesAndPartitionConfiguration(and the comment about what it “throws”) still only mentionsnewlinesBetweenand even references the old, more specific error message for group-level{ newlinesBetween: ... }. It doesn’t mentionnewlinesInsideat all, even though this function now enforces constraints for it too. I’d recommend:
- Extending the description/examples to cover
newlinesInsideconflicts.- Updating the “Throws:” comments to reflect the unified
NEWLINES_BETWEEN_ERROR_MESSAGEandNEWLINES_INSIDE_ERROR_MESSAGEstrings.Functionally everything looks good; this is just about keeping the documentation in sync with the behavior.
Also applies to: 101-184
test/rules/sort-classes.test.ts (1)
4198-4204: Consider explicitly spreading the mode‑specificoptionsinto these testsThese “handles newlines and comment after fixes” cases now use
groups: ['property', 'method']withnewlinesBetween: 1andnewlinesInside: 0, which is a good scenario to test comment handling plus inside/between spacing. For clarity and future‑proofing, you might want to include...options(the mode‑specific{ type, order }) in these option objects, so the tests are explicitly tied to the enclosingalphabetical/natural/line-lengthconfiguration instead of relying on global defaults.Also applies to: 8698-8714, 13158-13164
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (70)
docs/content/rules/sort-array-includes.mdx(3 hunks)docs/content/rules/sort-classes.mdx(3 hunks)docs/content/rules/sort-decorators.mdx(3 hunks)docs/content/rules/sort-enums.mdx(3 hunks)docs/content/rules/sort-export-attributes.mdx(3 hunks)docs/content/rules/sort-exports.mdx(3 hunks)docs/content/rules/sort-heritage-clauses.mdx(3 hunks)docs/content/rules/sort-import-attributes.mdx(3 hunks)docs/content/rules/sort-imports.mdx(3 hunks)docs/content/rules/sort-interfaces.mdx(3 hunks)docs/content/rules/sort-intersection-types.mdx(3 hunks)docs/content/rules/sort-jsx-props.mdx(3 hunks)docs/content/rules/sort-maps.mdx(3 hunks)docs/content/rules/sort-modules.mdx(3 hunks)docs/content/rules/sort-named-exports.mdx(3 hunks)docs/content/rules/sort-named-imports.mdx(3 hunks)docs/content/rules/sort-object-types.mdx(3 hunks)docs/content/rules/sort-objects.mdx(3 hunks)docs/content/rules/sort-sets.mdx(3 hunks)docs/content/rules/sort-union-types.mdx(3 hunks)docs/content/rules/sort-variable-declarations.mdx(3 hunks)rules/sort-array-includes.ts(1 hunks)rules/sort-classes.ts(1 hunks)rules/sort-decorators.ts(1 hunks)rules/sort-enums.ts(1 hunks)rules/sort-export-attributes.ts(1 hunks)rules/sort-exports.ts(1 hunks)rules/sort-heritage-clauses.ts(1 hunks)rules/sort-import-attributes.ts(1 hunks)rules/sort-imports.ts(1 hunks)rules/sort-intersection-types.ts(1 hunks)rules/sort-jsx-props.ts(1 hunks)rules/sort-maps.ts(1 hunks)rules/sort-modules.ts(1 hunks)rules/sort-named-exports.ts(1 hunks)rules/sort-named-imports.ts(1 hunks)rules/sort-object-types.ts(1 hunks)rules/sort-objects.ts(1 hunks)rules/sort-union-types.ts(1 hunks)rules/sort-variable-declarations.ts(1 hunks)test/rules/sort-array-includes.test.ts(7 hunks)test/rules/sort-classes.test.ts(11 hunks)test/rules/sort-decorators.test.ts(5 hunks)test/rules/sort-enums.test.ts(7 hunks)test/rules/sort-exports.test.ts(9 hunks)test/rules/sort-heritage-clauses.test.ts(5 hunks)test/rules/sort-imports.test.ts(10 hunks)test/rules/sort-interfaces.test.ts(9 hunks)test/rules/sort-intersection-types.test.ts(9 hunks)test/rules/sort-jsx-props.test.ts(10 hunks)test/rules/sort-maps.test.ts(10 hunks)test/rules/sort-modules.test.ts(8 hunks)test/rules/sort-named-exports.test.ts(12 hunks)test/rules/sort-named-imports.test.ts(10 hunks)test/rules/sort-object-types.test.ts(9 hunks)test/rules/sort-objects.test.ts(7 hunks)test/rules/sort-sets.test.ts(7 hunks)test/rules/sort-union-types.test.ts(7 hunks)test/rules/sort-variable-declarations.test.ts(7 hunks)test/utils/get-newlines-between-option.test.ts(5 hunks)test/utils/json-schemas/common-groups-json-schemas.test.ts(2 hunks)test/utils/sort-nodes-by-groups.test.ts(1 hunks)test/utils/validate-newlines-and-partition-configuration.test.ts(1 hunks)types/common-groups-options.ts(4 hunks)utils/get-newlines-between-option.ts(5 hunks)utils/json-schemas/common-groups-json-schemas.ts(2 hunks)utils/make-fixes.ts(1 hunks)utils/report-all-errors.ts(1 hunks)utils/report-errors.ts(2 hunks)utils/validate-newlines-and-partition-configuration.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/content/rules/sort-enums.mdx
🚧 Files skipped from review as they are similar to previous changes (29)
- rules/sort-export-attributes.ts
- rules/sort-maps.ts
- rules/sort-named-imports.ts
- rules/sort-import-attributes.ts
- rules/sort-decorators.ts
- rules/sort-imports.ts
- rules/sort-modules.ts
- rules/sort-array-includes.ts
- test/rules/sort-imports.test.ts
- rules/sort-variable-declarations.ts
- docs/content/rules/sort-array-includes.mdx
- rules/sort-exports.ts
- docs/content/rules/sort-maps.mdx
- test/rules/sort-decorators.test.ts
- rules/sort-classes.ts
- rules/sort-objects.ts
- test/rules/sort-interfaces.test.ts
- rules/sort-named-exports.ts
- test/rules/sort-union-types.test.ts
- docs/content/rules/sort-named-exports.mdx
- rules/sort-jsx-props.ts
- docs/content/rules/sort-imports.mdx
- test/rules/sort-array-includes.test.ts
- test/utils/sort-nodes-by-groups.test.ts
- rules/sort-union-types.ts
- docs/content/rules/sort-exports.mdx
- rules/sort-object-types.ts
- docs/content/rules/sort-sets.mdx
- docs/content/rules/sort-jsx-props.mdx
🧰 Additional context used
🧬 Code graph analysis (6)
utils/validate-newlines-and-partition-configuration.ts (4)
types/common-groups-options.ts (2)
CommonGroupsOptions(75-98)NewlinesInsideOption(207-212)types/common-partition-options.ts (1)
CommonPartitionOptions(47-59)utils/is-group-with-overrides-option.ts (1)
isGroupWithOverridesOption(30-34)utils/is-newlines-between-option.ts (1)
isNewlinesBetweenOption(36-40)
utils/report-errors.ts (1)
types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
test/utils/json-schemas/common-groups-json-schemas.test.ts (1)
utils/json-schemas/common-groups-json-schemas.ts (1)
newlinesInsideJsonSchema(29-41)
utils/report-all-errors.ts (1)
types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
utils/make-fixes.ts (3)
types/sorting-node.ts (1)
SortingNode(18-102)types/common-partition-options.ts (1)
PartitionByCommentOption(30-45)types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
test/utils/validate-newlines-and-partition-configuration.test.ts (1)
utils/validate-newlines-and-partition-configuration.ts (1)
validateNewlinesAndPartitionConfiguration(101-121)
🔇 Additional comments (83)
docs/content/rules/sort-classes.mdx (3)
294-297: LGTM! Clear distinction between group boundaries.The updated text properly clarifies that
newlinesBetweenapplies between groups, and the cross-reference to the groups option is correctly formatted.
299-315: LGTM! Comprehensive documentation for the new option.The
newlinesInsidesection is well-structured:
- Type and default value are clearly specified
- Behavior for different values is explained concisely
- Cross-references to
groupsandcustomGroupsfor per-group overrides are properly linked- Applicability constraint (when
partitionByNewLineisfalse) is documented consistently withnewlinesBetween
831-831: LGTM! Usage examples updated correctly.Both configuration examples (Flat and Legacy) now include
newlinesInside: 'ignore', matching the documented default value and demonstrating the new option's usage.Also applies to: 886-886
docs/content/rules/sort-decorators.mdx (2)
297-297: LGTM! Type definitions and documentation are consistent.The
newlinesInsideoption is properly documented across all relevant type definitions (groups, customGroups) and corresponding descriptions. The documentation clearly explains how to override this option at the group and custom group levels.Also applies to: 326-326, 401-401, 416-416, 433-433
500-500: LGTM! Usage examples are complete and accurate.The
newlinesInside: 'ignore'option is correctly included in both flat and legacy configuration examples with the appropriate default value.Also applies to: 536-536
docs/content/rules/sort-named-imports.mdx (4)
240-246: LGTM!The clarifications about
partitionByNewLineapplicability and the cross-reference to the groups option improve the documentation's clarity.
247-261: LGTM!The new
newlinesInsidesection is well-structured and clearly documents the option's behavior, type, default value, and applicability constraints.
400-400: LGTM!The attribute description accurately explains the purpose of
newlinesInside.
436-436: LGTM!The usage examples correctly demonstrate the
newlinesInsideoption with its default value of'ignore'in both configuration formats.Also applies to: 468-468
docs/content/rules/sort-modules.mdx (3)
325-339: Documentation fornewlinesInsideis clear and well-structured.The new section follows the existing documentation pattern, correctly documents the type (
number | 'ignore'), default value ('ignore'), and provides clear descriptions for each option value. The cross-references togroupsandcustomGroupsfor per-group overrides are appropriate, and thepartitionByNewLineconstraint is correctly noted.
625-625: Usage examples correctly updated with the new option.Both the Flat Config and Legacy Config examples include
newlinesInside: 'ignore', which matches the documented default value and maintains consistency with the existing option ordering.Also applies to: 669-669
350-355: Group-level type definition is intentionally more restrictive.The
newlinesInside?: numbertype in the groups definition (without'ignore') is appropriate. At the group level, overrides should specify a concrete number;'ignore'is only meaningful at the rule level to opt-out of enforcement entirely.docs/content/rules/sort-variable-declarations.mdx (4)
198-212: Documentation accurately reflects the separated concerns.The updated
newlinesBetweendescription now correctly focuses solely on between-group newlines, removing the implicit restriction on within-group newlines. The cross-reference and applicability condition are appropriate.
230-248: Type definitions and group override documentation are accurate.The addition of
newlinesInside?: numberto the group override object type (line 244) and its corresponding documentation (line 273) correctly reflect the new capability to override newline behavior at the group level.Also applies to: 267-282
304-362: Custom group type definitions correctly include newlinesInside.The
newlinesInside?: numberproperty is properly added to bothCustomGroupDefinition(line 320) andCustomGroupAnyOfDefinition(line 336) interfaces, with appropriate documentation in the attributes section (line 354).
364-435: Usage examples correctly demonstrate the new option.Both Flat Config (line 389) and Legacy Config (line 420) examples include
newlinesInside: 'ignore', which correctly reflects the default value and provides users with complete configuration examples.test/rules/sort-named-imports.test.ts (6)
1097-1144: LGTM! Well-structured test for the newnewlinesInsideoption.The test clearly validates that
newlinesInside: 0removes newlines within groups while preserving the inter-group spacing controlled bynewlinesBetween(which defaults to'ignore'). The error expectations correctly include both ordering and spacing violations.
1146-1192: Correct separation of inter-group vs. intra-group newline semantics.The updated test properly validates
newlinesBetween: 0behavior while allowingnewlinesInsideto default to'ignore'. The preserved newline at line 1187 betweenyandz(both in theunknowngroup) is intentional and demonstrates that intra-group newlines are preserved whennewlinesInsideis not explicitly set.
1423-1466: Good coverage of comment preservation with the new option.Adding
newlinesInside: 0at line 1435 ensures this test validates both inter-group and intra-group newline handling while preserving inline comments during reordering. This is an important edge case.
2830-2873: Consistent update for comment preservation test in natural section.Adding
newlinesInside: 0at line 2842 maintains consistency with the alphabetical section and ensures proper validation of newline handling with comment preservation.
3953-4002: Appropriate test updates for line-length sort type.The changes maintain consistency across all sort types (alphabetical, natural, line-length), ensuring the new
newlinesInsidesemantics are properly validated in each context.
4233-4276: Final consistency update completes the coverage.Adding
newlinesInside: 0at line 4245 completes the consistent coverage of comment preservation with newline handling across all three sort type sections (alphabetical, natural, line-length).test/rules/sort-jsx-props.test.ts (4)
848-943: LGTM! Clean separation ofnewlinesInsidevsnewlinesBetweenconcerns.The test refactoring correctly reflects the breaking change in the PR:
- Lines 848-895: Tests
newlinesInside: 0removes spacing within the 'unknown' group (between y, z, b) while preserving between-group spacing (after 'a')- Lines 897-943: Tests
newlinesBetween: 0removes spacing between groups (after 'a') while preserving within-group spacing (blank at line 938)This demonstrates that
newlinesBetweenno longer implicitly controls inside-group newlines, which is the core behavior change described in the PR objectives.
1205-1205: LGTM! Consistent addition ofnewlinesInside: 0for comment-preservation tests.Adding
newlinesInside: 0ensures that tests validating inline comment preservation also verify the new option works correctly alongside comment handling.
2162-2193: LGTM! Natural sorting section mirrors the alphabetical tests correctly.The error expectations and output format for the
newlinesBetween: 0test in the "natural" section match the pattern from the "alphabetical" section, ensuring consistent behavior across sorting modes.
2455-2455: LGTM! Consistent propagation ofnewlinesInside: 0across all sorting modes.The addition of
newlinesInside: 0in the "natural" and "line-length" sections maintains test parity with the "alphabetical" section.Also applies to: 3708-3708
test/rules/sort-modules.test.ts (7)
1794-1839: LGTM!The test correctly validates the
newlinesInside: 0behavior by expecting theextraSpacingBetweenModulesMemberserror for the extra newlines within theunknowngroup. The separation of this test from thenewlinesBetweentest (below) provides clear coverage for both options independently.
1841-1883: LGTM!This test correctly validates that
newlinesBetween: 0only affects newlines between groups, while intra-group newlines are preserved whennewlinesInsidedefaults to'ignore'. The absence ofextraSpacingBetweenModulesMembersin the expected errors confirms the separation of concerns between the two options.
2188-2206: LGTM!The updated test correctly validates the interaction between
newlinesBetween: 1andnewlinesInside: 0. The expected output shows one blank line between thefunctionandtypegroups, while types B and C are adjacent within their group.
4257-4299: LGTM!The natural sorting section consistently mirrors the alphabetical section changes. The test correctly validates
newlinesBetween: 0behavior while respecting the defaultnewlinesInside: 'ignore'setting.
4590-4623: LGTM!Consistent with the alphabetical section - the combined
newlinesBetween: 1andnewlinesInside: 0options are properly tested.
6633-6675: LGTM!The line-length sorting section maintains consistency with the alphabetical and natural sections. The test coverage for
newlinesInsideis comprehensive across all sorting types.
6969-7002: LGTM!Consistent test implementation for the combined options behavior in the line-length sorting context.
rules/sort-heritage-clauses.ts (1)
48-55:newlinesInsidedefault matches the new grouping semanticsSetting
newlinesInside: 'ignore'indefaultOptionskeeps this rule non-opinionated about inner-group newlines unless explicitly configured and stays consistent with the cross‑rule contract described in the PR.test/rules/sort-sets.test.ts (1)
1092-1141: Tests correctly modelnewlinesInsidevsnewlinesBetweenbehaviorThe new
newlinesInside: 0coverage and the adjusted expectations fornewlinesBetween: 0cleanly distinguish inner‑group vs between‑group spacing, including with custom groups and inline comments. This aligns with the intended semantics and gives good regression coverage.Also applies to: 1143-1190, 1437-1449, 2565-2613, 2860-2871, 3987-4037, 4285-4296
test/rules/sort-named-exports.test.ts (1)
961-1008: Updated newline tests reflect the new inside‑vs‑between semantics for named exportsThe added
newlinesInside: 0test and the revisednewlinesBetween: 0expectations (for alphabetical, natural, and line‑length modes) now clearly exercise and differentiate inner‑group spacing from between‑group spacing, while still checking comment and partition handling. The scenarios look consistent with the intended rule behavior.Also applies to: 1010-1055, 1292-1305, 2286-2332, 2562-2611, 3848-3860
test/rules/sort-variable-declarations.test.ts (5)
1703-1744: LGTM!The test correctly validates that inline comments are preserved when reordering elements with
newlinesBetween: 1andnewlinesInside: 0. The configuration and expected output are coherent with the newnewlinesInsidesemantics.
3111-3155: Verify the trailing empty line marker in output.Line 3151 has a
~marker on an empty line within the output block. This appears to indicate a newline is being added after sorting. Ensure this is the intended behavior for thenewlinesBetween: 0test case - typicallynewlinesBetween: 0would remove such spacing.Looking at the test, with
newlinesBetween: 0, the output showsb,followed by an empty line and thenz,. This seems inconsistent with the option's intent.
3432-3473: LGTM!The test correctly validates that inline comments are preserved when reordering elements with the
newlinesInside: 0option in thenaturalsorting context.
4837-4883: Verify the trailing empty line marker in output.Similar to the earlier
naturalsorting test, line 4880 has a~marker on an empty line. WithnewlinesBetween: 0, there should not be extra newlines between groups or within groups. This output shows an empty line betweenyy,andz,which may be unintended.
5164-5205: LGTM!The test correctly validates inline comment preservation with
newlinesInside: 0in theline-lengthsorting context.docs/content/rules/sort-union-types.mdx (2)
230-244: LGTM!The documentation for
newlinesInsideis clear, follows the established pattern, and correctly describes the option's behavior including its interaction withpartitionByNewLineand group/customGroups overrides.
502-510: LGTM!The example configuration correctly includes
newlinesInside: 'ignore'alongside the other options.utils/report-all-errors.ts (1)
108-125: LGTM!The type simplification from a complex intersection type to
CommonGroupsOptions<unknown, unknown, string>is cleaner and aligns with the broader refactor introducingnewlinesInside. This ensures the error reporting utilities have access to the new option.types/common-groups-options.ts (3)
75-98: LGTM!The new
CommonGroupsOptionsinterface cleanly consolidates all group-related options (customGroups,groups,newlinesBetween,newlinesInside) with proper JSDoc documentation. This provides a single source of truth for these options across the codebase.
46-48: LGTM!The
CustomGroupsOptioncorrectly usesNewlinesInsideOptionfor type safety and consistency with the new option structure.
110-113: LGTM!The
GroupWithOverridesOptioncorrectly usesNewlinesInsideOption, maintaining consistency with the custom groups definition.utils/make-fixes.ts (1)
18-24: LGTM!The interface is now internal (not exported), which is appropriate since it's only used within this file. The type simplification to use
CommonGroupsOptions<unknown, unknown, string>directly is consistent with the refactor across other utilities and ensures the newnewlinesInsideoption is accessible during fix generation. Confirmed thatMakeFixesParametersis not imported elsewhere in the codebase.test/rules/sort-maps.test.ts (2)
947-994: LGTM! Well-structured test for inside-group newline enforcement.The test correctly verifies that
newlinesInside: 0removes extra newlines within groups while preserving between-group spacing. The expected output and error messages accurately reflect the intended behavior.
996-1042: LGTM! Clear distinction between inside-group and between-group newline behavior.This test effectively demonstrates that
newlinesBetween: 0removes newlines between groups while leaving inside-group newlines unaffected (defaulting to'ignore'). The separation of concerns between these two tests is well done.rules/sort-enums.ts (1)
59-59: LGTM! Backward-compatible default added.The
newlinesInside: 'ignore'default ensures existing behavior is preserved while enabling users to opt into inside-group newline enforcement when needed.rules/sort-intersection-types.ts (1)
31-31: LGTM! Consistent with other rule defaults.The addition mirrors the pattern used in other rules (e.g.,
sort-enums.ts), maintaining consistency across the plugin's rule set.test/utils/json-schemas/common-groups-json-schemas.test.ts (1)
26-38: LGTM! Comprehensive schema validation.The test suite properly validates the
newlinesInsideJSON schema, ensuring both valid values ('ignore',0,1) are accepted and invalid values are rejected. The structure mirrors the existingnewlinesBetweentests, maintaining consistency.docs/content/rules/sort-heritage-clauses.mdx (1)
181-195: LGTM! Clear and comprehensive documentation.The
newlinesInsideoption is well-documented with clear explanations of its behavior, type constraints, and interactions with other options. The cross-references togroupsandcustomGroupshelp users understand per-group configuration.docs/content/rules/sort-import-attributes.mdx (1)
162-175: LGTM! Consistent documentation across rules.The documentation follows the established pattern from other rule files, ensuring a consistent user experience across the plugin's documentation.
docs/content/rules/sort-intersection-types.mdx (1)
210-223: LGTM! Well-integrated documentation.The
newlinesInsidedocumentation is properly integrated into the existing structure and maintains consistency with other rule documentation in the plugin.docs/content/rules/sort-objects.mdx (1)
291-303: LGTM! Complete documentation coverage.The documentation completes the comprehensive coverage of the
newlinesInsideoption across all relevant rules, maintaining consistency in structure and clarity throughout.utils/get-newlines-between-option.ts (1)
64-64: LGTM! Consolidation to direct options usage simplifies the logicThe removal of global option computation in favor of direct fallback to
options.newlinesBetweenandoptions.newlinesInsideis a clean refactor. The precedence chain (group/customGroup override → rule-level option) is correctly implemented.Also applies to: 81-81, 90-90, 122-122, 136-136
docs/content/rules/sort-export-attributes.mdx (1)
162-176: LGTM! Comprehensive documentation for the new optionThe
newlinesInsideoption is well-documented with clear type information, default value, behavior description, and cross-references to related options. Config examples consistently include the new option.Also applies to: 296-296, 327-327
test/rules/sort-heritage-clauses.test.ts (1)
525-620: LGTM! Tests clearly distinguish inside-group and between-group newline behaviorThe test updates correctly separate
newlinesInside(inside groups) fromnewlinesBetween(between groups) semantics, with appropriate error expectations and output verification.Also applies to: 920-920
test/rules/sort-intersection-types.test.ts (1)
821-903: LGTM! Comprehensive test coverage across sorting typesThe test updates consistently cover
newlinesInsidebehavior across alphabetical, natural, and line-length sorting modes, maintaining clear distinction fromnewlinesBetweensemantics.Also applies to: 1116-1116, 2440-2481, 2694-2694, 3972-4013, 4226-4226
test/rules/sort-exports.test.ts (3)
955-998: LGTM! Test correctly validatesnewlinesInsidebehavior.The test accurately demonstrates that
newlinesInside: 0removes extra blank lines within groups while maintaining group boundaries. The updated description clearly distinguishes inside-group from between-group newline semantics, and the test configuration withcustomGroupsexercises the new option appropriately.
1000-1042: LGTM! Test clearly demonstrates the distinction betweennewlinesBetweenandnewlinesInside.This test effectively contrasts with the previous test, showing that
newlinesBetween: 0removes newlines between groups while preserving inside-group spacing (note line 1038 in the output). The test suite now provides comprehensive coverage of both options working independently.
2338-2380: LGTM! Consistent test coverage across all sorting types.The test updates in the 'natural' and 'line-length' sections mirror the changes in the 'alphabetical' section, ensuring the new
newlinesBetweenandnewlinesInsideoptions work correctly with all sorting types. The consistency in error reporting and expected output demonstrates thorough test coverage.Also applies to: 3712-3750
docs/content/rules/sort-object-types.mdx (2)
260-266: LGTM! Clear documentation improvements fornewlinesBetween.The updated description clarifies the option's behavior and provides helpful cross-references to related configuration options. The note about
partitionByNewLineapplicability helps users understand when the option takes effect.
267-281: LGTM! Comprehensive documentation for the newnewlinesInsideoption.The documentation clearly explains the new option's purpose, valid values, and relationship to other configuration options. The distinction between inside-group and between-group newline control is well-articulated, and the cross-references to
groupsandcustomGroupshelp users understand the full configuration surface. The example configurations appropriately show the default value.Also applies to: 746-746, 779-779
test/rules/sort-objects.test.ts (1)
1789-1820:newlinesBetween: 0tests now cleanly exercise only inter‑group spacingThe three
it('removes newlines between groups when newlinesBetween is 0'...)cases (alphabetical, natural, line‑length):
- Correctly report
extraSpacingBetweenObjectMembersfor the gap between the last member of themethodgroup and the first of theunknowngroup ({ right: 'y', left: 'a' }/{ right: 'yy', left: 'aaaa' }).- Keep a blank line inside the
unknowngroup in the fixed output, which matches the new contract thatnewlinesBetweengoverns only spacing between groups while intra‑group spacing is left tonewlinesInside(defaulting to'ignore'here).This cleanly demonstrates the separation of concerns between the two options across all three sort modes.
Also applies to: 5591-5621, 8234-8267
test/rules/sort-enums.test.ts (1)
1143-1189: Test expectation is correct; no issues found.The blank line between Y and Z in the expected output is intentional and correct. When
newlinesBetweenis set to a number, it enforces that number of newlines between groups and forbids newlines inside groups. However, in this test, the rule's internal logic uses thenewlinesInsideoption (notnewlinesBetween) to control spacing within the same group.Since the test does not override
newlinesInsideand the default value is'ignore', blank lines within groups are preserved as-is. ThenewlinesBetween: 0setting only removes blank lines between the 'a' group (A) and 'unknown' group (Y, Z, B), which the expected output correctly reflects (no blank line between A and B after sorting). The blank line between Y and Z remains because they're in the same group andnewlinesInside: 'ignore'allows intra-group spacing to be preserved.This is confirmed by the preceding test (line 1094) which sets
newlinesInside: 0to explicitly remove blank lines within groups—a separate concern fromnewlinesBetween.test/rules/sort-object-types.test.ts (6)
1645-1727: Good separation of inside-vs-between newline semanticsThe paired tests for
newlinesInside: 0andnewlinesBetween: 0correctly distinguish:
- inside-group spacing (only
extraSpacing…onb/zwhennewlinesInside: 0), vs- between-group spacing (only
extraSpacing…ona/ywhennewlinesBetween: 0andnewlinesInsideleft at default).The expected outputs and error payloads look consistent with the new semantics and grouping (
methodvsunknown). No issues spotted here.
1965-2000: AddingnewlinesInside: 0to comment-fix regression is appropriateExtending the “handles newlines and comment after fixes” case with
newlinesInside: 0is a good guardrail to ensure the fixer doesn’t introduce or retain stray blank lines inside the property/method groups while still respecting the inline comment. Expectations match the described behavior.
4931-4970: Natural sort variant mirrors alphabetical newline behaviorThe natural-
type“removes newlines between groups when newlinesBetween is 0” test mirrors the alphabetical version and focuses solely on spacing between themethodandunknowngroups (no assertions on internal blank lines). This keeps behavior aligned across sort strategies; the errors and output look correct.
5208-5243: Locking innewlinesInside: 0for natural ‘comment after fixes’Same as in the alphabetical block, adding
newlinesInside: 0here ensures that post-fix formatting remains tight within groups while preserving the comment and enforced inter-group newline. Expectations look consistent.
7288-7331: Line-length variant of newlinesBetween=0 keeps behavior consistentThe line-length
typevariant fornewlinesBetween: 0uses a slightly different naming/values (aaaa,yy,bbb) but still asserts the same semantics: collapse extra blank lines between themethodandunknowngroups while maintaining sort order. This symmetry with the other describes is good; no functional issues.
7568-7603: Line-length ‘comment after fixes’ test correctly integratesnewlinesInside: 0Adding
newlinesInside: 0here is consistent with the other sort modes and constrains intra-group blank lines without affecting the comment handling. The expectations match the intent.test/rules/sort-classes.test.ts (2)
3828-3869: Solid coverage for rule‑levelnewlinesInsidewith grouped membersThis new test correctly exercises
newlinesInside: 0at rule‑options level withgroups: ['method', 'unknown'], validating both reordering and removal of intra‑group blank lines while leaving inter‑group spacing behavior unchanged. Fixtures and expected errors look consistent with the intended semantics.
3871-3910: GroupednewlinesBetweentests look consistent across all sort modesUsing
groups: ['method', 'unknown']plusnewlinesBetween: 0for alphabetical, natural, and line‑length modes gives good coverage that between‑group spacing is enforced even when grouping is enabled. The input/output pairs andextraSpacingBetweenClassMembers/unexpectedClassesOrderexpectations align with the described behavior.Also applies to: 8381-8420, 12835-12870
test/utils/validate-newlines-and-partition-configuration.test.ts (3)
6-55: Well-structured test coverage fornewlinesBetweenvalidation.The test cases comprehensively cover:
- Error cases when
partitionByNewLineis enabled with numericnewlinesBetweenvalues- Error cases when groups contain
newlinesBetweenobjects- Success cases when
partitionByNewLineis disabled
57-123: Comprehensive test coverage fornewlinesInsidevalidation.The test cases properly cover:
- Error when
partitionByNewLineis enabled with numericnewlinesInsidevalues- Success when
partitionByNewLineis disabled- Error when
newlinesInsideis set ingroupsorcustomGroupswith partitioning enabledThe test descriptions at lines 91 and 108 now correctly reference
newlinesInsideinstead ofnewlinesBetween.
125-139: Good edge case coverage.This test validates the important scenario where
partitionByNewLinecan coexist withnewlinesInsideandnewlinesBetweenwhen both are set to'ignore', including group-level overrides.test/utils/get-newlines-between-option.test.ts (4)
3-8: Appropriate type imports for the updated test helper.The imported types (
NewlinesBetweenOption,NewlinesInsideOption,CustomGroupsOption,GroupsOptions) are correctly used in thebuildParametershelper function signature.
62-79: Key behavioral test fornewlinesInside.This test validates the core new behavior: when two nodes belong to the same group (
nodeGroupNumber === nextNodeGroupNumber), the function returns thenewlinesInsidevalue instead ofnewlinesBetween. This correctly distinguishes between-group and inside-group newline handling.
120-152: Good coverage for custom groupnewlinesInsideprecedence.Tests correctly verify that when both nodes belong to the same custom group, the custom group's
newlinesInsideoption takes precedence over the global setting. This aligns with the stated priority chain: group-level → rule-level → settings-level → default.
327-359: Well-designed helper function update.The
buildParametershelper correctly:
- Accepts optional
newlinesInsideparameter- Defaults to
'ignore'to maintain backward compatibility with existing tests- Uses proper type annotations from the imported types
482a8fe to
b6b72f6
Compare
b6b72f6 to
a4f5229
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (4)
docs/content/rules/sort-heritage-clauses.mdx (2)
309-316: Type inconsistency:newlinesInsideshould include'ignore'option.The
CustomGroupDefinitioninterface showsnewlinesInside?: numberon line 314, but the documented behavior (lines 183-184, 188) and the type definition intypes/common-groups-options.tsindicate it should benumber | 'ignore'.🔎 Proposed fix
interface CustomGroupDefinition { groupName: string type?: 'alphabetical' | 'natural' | 'line-length' | 'unsorted' order?: 'asc' | 'desc' fallbackSort?: { type: string; order?: 'asc' | 'desc' } - newlinesInside?: number + newlinesInside?: number | 'ignore' elementNamePattern?: string | string[] | { pattern: string; flags?: string } | { pattern: string; flags?: string }[] }
324-335: Same type inconsistency inCustomGroupAnyOfDefinition.Line 329 shows
newlinesInside?: numberbut should benumber | 'ignore'to match the documented behavior.🔎 Proposed fix
interface CustomGroupAnyOfDefinition { groupName: string type?: 'alphabetical' | 'natural' | 'line-length' | 'unsorted' order?: 'asc' | 'desc' fallbackSort?: { type: string; order?: 'asc' | 'desc' } - newlinesInside?: number + newlinesInside?: number | 'ignore' anyOf: Array<{ selector?: string elementNamePattern?: string | string[] | { pattern: string; flags?: string } | { pattern: string; flags?: string }[] }> }docs/content/rules/sort-union-types.mdx (2)
430-438: Type inconsistency:newlinesInsideshould include'ignore'option.Same issue as in
sort-heritage-clauses.mdx: line 435 showsnewlinesInside?: numberbut should benumber | 'ignore'to match the documented behavior at lines 232-233.🔎 Proposed fix
interface CustomGroupDefinition { groupName: string type?: 'alphabetical' | 'natural' | 'line-length' | 'unsorted' order?: 'asc' | 'desc' fallbackSort?: { type: string; order?: 'asc' | 'desc' } - newlinesInside?: number + newlinesInside?: number | 'ignore' selector?: string elementNamePattern?: string | string[] | { pattern: string; flags?: string } | { pattern: string; flags?: string }[] }
446-457: Same type inconsistency inCustomGroupAnyOfDefinition.Line 451 shows
newlinesInside?: numberbut should benumber | 'ignore'.🔎 Proposed fix
interface CustomGroupAnyOfDefinition { groupName: string type?: 'alphabetical' | 'natural' | 'line-length' | 'unsorted' order?: 'asc' | 'desc' fallbackSort?: { type: string; order?: 'asc' | 'desc' } - newlinesInside?: number + newlinesInside?: number | 'ignore' anyOf: Array<{ selector?: string elementNamePattern?: string | string[] | { pattern: string; flags?: string } | { pattern: string; flags?: string }[] }> }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
docs/content/rules/sort-array-includes.mdx(4 hunks)docs/content/rules/sort-classes.mdx(4 hunks)docs/content/rules/sort-decorators.mdx(4 hunks)docs/content/rules/sort-enums.mdx(4 hunks)docs/content/rules/sort-export-attributes.mdx(4 hunks)docs/content/rules/sort-exports.mdx(4 hunks)docs/content/rules/sort-heritage-clauses.mdx(4 hunks)docs/content/rules/sort-import-attributes.mdx(4 hunks)docs/content/rules/sort-imports.mdx(4 hunks)docs/content/rules/sort-interfaces.mdx(4 hunks)docs/content/rules/sort-intersection-types.mdx(4 hunks)docs/content/rules/sort-jsx-props.mdx(4 hunks)docs/content/rules/sort-maps.mdx(4 hunks)docs/content/rules/sort-modules.mdx(4 hunks)docs/content/rules/sort-named-exports.mdx(4 hunks)docs/content/rules/sort-named-imports.mdx(4 hunks)docs/content/rules/sort-object-types.mdx(4 hunks)docs/content/rules/sort-objects.mdx(4 hunks)docs/content/rules/sort-sets.mdx(4 hunks)docs/content/rules/sort-union-types.mdx(4 hunks)docs/content/rules/sort-variable-declarations.mdx(4 hunks)test/utils/get-settings.test.ts(1 hunks)types/common-groups-options.ts(4 hunks)utils/get-settings.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/content/rules/sort-export-attributes.mdx
- docs/content/rules/sort-array-includes.mdx
- docs/content/rules/sort-import-attributes.mdx
- docs/content/rules/sort-variable-declarations.mdx
- docs/content/rules/sort-object-types.mdx
- docs/content/rules/sort-maps.mdx
- docs/content/rules/sort-decorators.mdx
- docs/content/rules/sort-objects.mdx
🧰 Additional context used
🧬 Code graph analysis (2)
utils/get-settings.ts (1)
types/common-groups-options.ts (1)
CommonGroupsOptions(75-98)
types/common-groups-options.ts (1)
rules/sort-imports/types.ts (1)
CustomTypeOption(96-96)
🔇 Additional comments (27)
docs/content/rules/sort-interfaces.mdx (3)
302-316: LGTM! Comprehensive documentation for the newnewlinesInsideoption.The documentation clearly explains:
- The distinction between inside-group and between-group newline handling
- Type and default values
- Applicability constraints (when
partitionByNewLineisfalse)- Cross-references to group-level overrides
The structure mirrors the
newlinesBetweendocumentation, making it easy for users to understand the relationship between the two options.
450-461: Well-structured type definitions with propernewlinesInsideintegration.The type definitions correctly show:
newlinesInside?: number | 'ignore'in group override objects- Inclusion in both
CustomGroupDefinitionandCustomGroupAnyOfDefinitioninterfaces- Consistency with the
newlinesBetweenpatternThis provides clear guidance for TypeScript users on how to configure the option at different levels.
Also applies to: 650-684
778-778: Usage examples properly updated with the new option.Both Flat Config and Legacy Config examples include
newlinesInside: 'ignore', showing users the default configuration and how to specify the option in their ESLint configs.Also applies to: 811-811
test/utils/get-settings.test.ts (1)
21-40: Test coverage appropriately extended fornewlinesInside.The test correctly verifies that
newlinesInsideis recognized as a valid Perfectionist setting at the global level, ensuring users won't encounter false validation errors when configuring this option in theirsettings.perfectionistconfiguration.utils/get-settings.ts (2)
14-21: Settings type correctly extended to includenewlinesInside.The
Picktype properly includes both'newlinesBetween'and'newlinesInside'fromCommonGroupsOptions, enabling users to configure the new option globally viasettings.perfectionist.newlinesInside.
66-84: Validation logic properly updated.The
allowedOptionsSet now includes'newlinesInside'alongside'newlinesBetween', ensuring the validation function correctly recognizes the new setting as valid and provides clear error messages for truly invalid options.docs/content/rules/sort-exports.mdx (2)
211-225: Consistent documentation pattern fornewlinesInside.The documentation for
sort-exportsfollows the same clear structure as other rules, maintaining consistency across the plugin's documentation. The explanation of inside-group newline behavior is clear and the cross-references togroupsandcustomGroupsare helpful.
413-413: Usage examples properly include the new option.Both configuration examples show
newlinesInside: 'ignore', helping users understand the default behavior and how to configure the option.Also applies to: 444-444
docs/content/rules/sort-jsx-props.mdx (2)
242-256: Well-documentednewlinesInsideoption for JSX props.The documentation clearly explains how to control newlines within groups for JSX props, maintaining consistency with other rules while being specific to the JSX props context.
339-353: Type definitions properly extended.Group and CustomGroup interfaces correctly include
newlinesInside?: number | 'ignore', enabling per-group configuration of inside-group newline behavior.Also applies to: 456-487
docs/content/rules/sort-named-imports.mdx (2)
247-261: Clear documentation for named imports.The
newlinesInsidedocumentation forsort-named-importsmaintains the consistent pattern across rules while being contextually appropriate for named imports.
360-388: CustomGroup definitions properly extended.Both
CustomGroupDefinitionandCustomGroupAnyOfDefinitioninterfaces includenewlinesInside?: number, enabling fine-grained control over newline behavior within custom groups.docs/content/rules/sort-sets.mdx (2)
266-280: ComprehensivenewlinesInsidedocumentation for Set elements.The documentation clearly explains how to enforce or ignore newlines within groups of Set elements, with proper cross-references to group-level configuration options.
336-351: Type definitions and CustomGroups properly extended.The groups type definition and CustomGroup interfaces correctly include
newlinesInside, enabling consistent configuration across all levels (global, rule, group, and custom group).Also applies to: 420-448
docs/content/rules/sort-enums.mdx (2)
231-245: Well-structurednewlinesInsidedocumentation for enums.The documentation maintains consistency with other rules while being appropriate for the enum context. The explanation of inside-group newline behavior is clear and actionable.
324-350: CustomGroup definitions properly supportnewlinesInside.Both
CustomGroupDefinitionandCustomGroupAnyOfDefinitioninterfaces includenewlinesInside?: number, completing the support for per-group newline configuration in custom groups.docs/content/rules/sort-intersection-types.mdx (1)
210-224: LGTM!The
newlinesInsidedocumentation section is well-structured and consistent with the existingnewlinesBetweensection. The default value of'ignore'aligns with the PR objectives for non-sort-imports rules, and the cross-references togroupsandcustomGroupssections are correctly linked.docs/content/rules/sort-modules.mdx (1)
325-339: LGTM!The
newlinesInsidesection is correctly documented with the expected type, default value, and behavior descriptions. The applicability note regardingpartitionByNewLineis properly included.docs/content/rules/sort-imports.mdx (1)
290-304: LGTM! Correct default for sort-imports.The
newlinesInsidedefault of0forsort-importsis intentional and correctly documented. This preserves backwards compatibility since the previousnewlinesBetween: 1behavior implicitly disallowed newlines within groups. The documentation clearly reflects this special case compared to other rules that default to'ignore'.docs/content/rules/sort-classes.mdx (1)
301-315: LGTM!The
newlinesInsidedocumentation is consistent with the other rule documentation files. The section correctly describes the option's behavior, default value, and applicability constraints.docs/content/rules/sort-named-exports.mdx (1)
248-262: LGTM!The
newlinesInsidedocumentation follows the established pattern across all rule documentation files. The cross-references togroupsandcustomGroupssections are correctly linked.docs/content/rules/sort-heritage-clauses.mdx (1)
181-195: Documentation fornewlinesInsidelooks complete and consistent.The new section properly documents the type, default, behavior options, and cross-references to group/customGroup overrides. The applicability condition regarding
partitionByNewLineis correctly noted.types/common-groups-options.ts (3)
75-98: Well-structured consolidation of group options.The
CommonGroupsOptionsinterface cleanly groups related options (customGroups,groups,newlinesBetween,newlinesInside) with proper generics for type safety. The JSDoc comments clearly describe each property's purpose.
207-212:NewlinesInsideOptiontype is correctly defined.The type mirrors the structure of
NewlinesBetweenOption(lines 171-182), maintaining consistency in the API design.
46-48: LGTM!The
newlinesInsideproperty is properly added with the correct type and documentation.docs/content/rules/sort-union-types.mdx (2)
230-244: Documentation fornewlinesInsideis complete and well-structured.The section properly documents the type, default value, available options, and cross-references to group/customGroup override capabilities.
504-506: Example configuration correctly includesnewlinesInside.The example demonstrates the default
'ignore'value, which aligns with the documented default.
There was a problem hiding this 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 adds a new newlinesInside option to all rules that support grouping (all rules except sort-switch-case). This option controls how newlines are handled within groups, independently from the existing newlinesBetween option which controls newlines between different groups.
Key Changes:
- Adds
newlinesInsideoption (default:'ignore', exceptsort-importswhere it defaults to0) - Updates
newlinesBetweenbehavior to only control newlines between groups (previously also affected within groups) - Updates all rule default options, JSON schemas, types, validation logic, and documentation
Reviewed changes
Copilot reviewed 72 out of 72 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
types/common-groups-options.ts |
Adds NewlinesInsideOption type and updates CommonGroupsOptions interface |
utils/validate-newlines-and-partition-configuration.ts |
Adds validation for newlinesInside option |
utils/get-newlines-between-option.ts |
Refactors to separate newlinesInside from newlinesBetween logic |
utils/json-schemas/common-groups-json-schemas.ts |
Exports newlinesInsideJsonSchema for validation |
utils/get-settings.ts |
Adds newlinesInside to settings extraction |
utils/report-errors.ts |
Updates type definitions for consistency |
utils/report-all-errors.ts |
Updates type definitions for consistency |
utils/make-fixes.ts |
Simplifies type definitions |
rules/*.ts |
Adds newlinesInside: 'ignore' to default options (except sort-imports which uses 0) |
docs/content/rules/*.mdx |
Documents the new newlinesInside option |
test/**/*.test.ts |
Updates tests to include newlinesInside parameter |
Comments suppressed due to low confidence (2)
utils/validate-newlines-and-partition-configuration.ts:1
- The validation logic for
newlinesBetweenin groups filters forgroup.newlinesBetween !== 'ignore', but the validation fornewlinesInsidein groups at line 160 allows'ignore'and returns early. These should use consistent validation patterns. Consider extracting the filtering logic into a shared helper function to ensure both validations follow the same pattern.
utils/get-newlines-between-option.ts:1 - When a group is not found at the specified
groupIndex, the function falls back tooptions.newlinesInside. However, this might not be the correct behavior if the group index is invalid or out of bounds. Consider adding validation or a warning when accessing an undefined group, as this could mask configuration errors.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | string | ||
| | string[] | ||
| | { newlinesBetween: number } | ||
| | { newlinesBetween: number | 'ignore' } |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type annotation shows newlinesBetween: number | 'ignore' but according to the implementation and other documentation, valid numeric values should be non-negative numbers (0 or greater). The documentation should specify this constraint, for example: number (≥0) | 'ignore'.
| | { newlinesBetween: number | 'ignore' } | |
| | { newlinesBetween: number /* >= 0 */ | 'ignore' } |
|
Thanks for the feature! It looks useful. However, this changes the default behavior: with Would it be possible to preserve backward compatibility by making Users who want the new behavior could then explicitly set |
- Support deprecated behavior.
- Update rules and tests.
- Update documentation.
newlinesInside at rule options and settings levelnewlinesInside at rule options and settings level
|
I think that the breaking change was minor considering that the release was delivered very recently and that the breaking change only "removes an enforced behavior" (and doesn't enforce an other). But we can support both. The default value for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/rules/sort-array-includes.test.ts (1)
1291-1342: Fix options format to use array instead of plain objectLine 1356 passes
optionsas a plain object, which violates the RuleTester API. It should be wrapped in an array to match ESLint's rule options format and align with other test cases in this file:options: [ { ...options, customGroups: [ { elementNamePattern: 'a', groupName: 'a', }, ], groups: ['a', 'unknown'], newlinesInside: 0, }, ],
🧹 Nitpick comments (11)
test/rules/sort-named-imports.test.ts (1)
1097-1144: LGTM! Well-structured test fornewlinesInside: 0behavior.The test correctly verifies that
newlinesInside: 0removes blank lines within groups while preserving between-group spacing. The expected errors and output align with the feature's intended behavior.Optional: Consider explicitly setting `newlinesBetween` for clarity
For improved test clarity and explicit intent, consider adding
newlinesBetween: 'ignore'(or another explicit value) to the options. This would make it immediately clear that the test focuses solely on inner-group newline behavior:options: [ { ...options, customGroups: [ { elementNamePattern: 'a', groupName: 'a', }, ], groups: ['a', 'unknown'], newlinesInside: 0, + newlinesBetween: 'ignore', }, ],This is purely for readability—the current test is correct.
utils/json-schemas/common-groups-json-schemas.ts (1)
28-41: Consider clarifying the description wording.The description "Specifies how to handle newlines between groups elements" on line 32 could be clearer. Consider rewording to "Specifies how to handle newlines between elements inside a group" to better distinguish this from
newlinesBetweenwhich handles spacing between groups.Suggested wording improvement
export let newlinesInsideJsonSchema: JSONSchema4 = { oneOf: [ { - description: 'Specifies how to handle newlines between groups elements', + description: 'Specifies how to handle newlines between elements inside a group', enum: ['ignore'], type: 'string', },docs/content/rules/sort-maps.mdx (1)
223-240: Clarify the relationship between default and effective behavior.The documentation states the default is
'newlinesBetween'(line 228), but the usage examples shownewlinesInside: 'ignore'(lines 444-445, 476-477). While technically correct (the effective default is'ignore'whennewlinesBetweendefaults to'ignore'), this may confuse users.Consider adding a note to clarify:
### newlinesInside <sub> type: `number | 'ignore' | 'newlinesBetween'` </sub> <sub>default: `'newlinesBetween'`</sub> **Note**: When using the default `'newlinesBetween'` value, the effective behavior depends on your `newlinesBetween` setting. With the default `newlinesBetween: 'ignore'`, this effectively behaves as `newlinesInside: 'ignore'`.This would help users understand why the usage examples show
'ignore'even though the documented default is'newlinesBetween'.docs/content/rules/sort-array-includes.mdx (1)
248-281: Align documentednewlinesInsidedefault with the usage examplesThe updated descriptions and type snippets for
newlinesBetween/newlinesInside,groups, andcustomGroupsare consistent with the new options model, including the'newlinesBetween'bridge mode. However, the section documentsnewlinesInside’s default as'newlinesBetween'while the configuration examples later in the page explicitly setnewlinesInside: 'ignore'. It would be less confusing to either:
- show
'newlinesBetween'in the examples, or- omit
newlinesInsidefrom the example config and rely on the documented default.Also applies to: 339-353, 383-389, 422-462, 488-500
utils/validate-newlines-and-partition-configuration.ts (1)
23-52: Update JSDoc to reflectnewlinesInsidehandling and unified error messagesThe JSDoc still:
- talks only about conflicts between
partitionByNewLineandnewlinesBetween, and- shows an example error string (
'newlinesBetween' objects can not be used in 'groups' alongside 'partitionByNewLine') that no longer matches the newNEWLINES_BETWEEN_ERROR_MESSAGE.Given the new
newlinesInsidechecks and shared error wording, it would be clearer to:
- mention both
newlinesBetweenandnewlinesInsidein the high‑level description and@throws, and- update the example comments to match the actual error messages.
Also applies to: 54-65, 98-100
docs/content/rules/sort-export-attributes.mdx (1)
148-181: ClarifynewlinesInsidedefault vs the documented example configThe updated section correctly introduces
newlinesBetween/newlinesInsideand extendsgroups/customGroupssignatures, but—as in other rule docs—the text saysnewlinesInsidedefaults to'newlinesBetween'while the Flat/Legacy config examples explicitly usenewlinesInside: 'ignore'.To avoid confusion, consider either updating the examples to show
'newlinesBetween'(or omittingnewlinesInside) or adjusting the stated default to match how you expect users to configure this rule.Also applies to: 185-201, 223-225, 260-271, 288-304, 319-335
test/rules/sort-objects.test.ts (1)
1746-1787: Alphabetical: inside-vs-between newline tests look correct; consider adding default-behavior coverage
- The new
"removes newlines inside groups when newlinesInside is 0"case plus the updated"removes newlines between groups when newlinesBetween is 0"(withnewlinesInside: 'ignore') and the comment-preservation test withnewlinesInside: 0accurately exercise:
- strict removal of intra‑group blank lines,
- independent control of inter‑group spacing,
- and safe handling of inline comments during fixing.
- To fully lock in the BC behavior requested in the PR discussion, it would be valuable to add at least one case where
newlinesInsideis omitted and:
newlinesBetweenis numeric (expect effective inside value0), andnewlinesBetweenis'ignore'(expect effective inside value'ignore').Also applies to: 1789-1829, 2061-2079
docs/content/rules/sort-imports.mdx (1)
276-309: Import docs correctly capture newline semantics; consider mentioning the deprecated sentinel if public
- The
newlinesBetween/newlinesInsidedescriptions, updated group/customGroup signatures, and default config (newlinesBetween: 1,newlinesInside: 0) all line up with the intended BC for imports.- If
'newlinesInside': 'newlinesBetween'is still accepted via the shared options schema for this rule (as it is forsort-objects), you might optionally add a brief “[DEPRECATED]” note here too for consistency; if it’s intentionally hidden for imports, this is fine as-is.Also applies to: 337-347, 605-606, 621-622, 711-713, 754-756
utils/get-newlines-between-option.ts (1)
2-6:getNewlinesBetweenOptionnow correctly derivesnewlinesInsidefromnewlinesBetween
- Same-group vs different-group branching is still clear, and using
options.newlinesBetweendirectly for adjacent groups is equivalent to the old global computation.computeGlobalNewlinesInsideOption’s mapping:
'newlinesBetween'→'ignore'or0based onoptions.newlinesBetween,'ignore'/number passthrough otherwise,
aligns with the requested BC semantics and the docs.- The precedence chain
customGroup.newlinesInside ?? groupOverride.newlinesInside ?? globalDerivedInsideis sensible and consistent with other per-group overrides.If not already covered elsewhere, it would be worth adding a focused test that leaves
newlinesInsideunset and asserts:
- numeric
newlinesBetweenyields inside=0, and'ignore'yields inside='ignore',to guard this logic against future regressions.
Also applies to: 20-25, 70-71, 82-88, 96-97, 126-156
types/common-groups-options.ts (2)
46-48: Consider aligning the JSDoc comment with other similar fields.The comment for
newlinesInsidesays "between elements of groups" but should say "between elements inside a group" to better distinguish fromnewlinesBetweenwhich handles spacing between groups.🔎 Suggested refinement
- /** Specify the exact number of newlines required between elements of groups. */ + /** Specify the exact number of newlines required between elements inside a group. */ newlinesInside?: NewlinesInsideOption
80-81: Same comment refinement for consistency.🔎 Suggested refinement
- /** Specify the exact number of newlines required between elements of groups. */ + /** Specify the exact number of newlines required between elements inside a group. */ newlinesInside:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (65)
docs/content/rules/sort-array-includes.mdxdocs/content/rules/sort-classes.mdxdocs/content/rules/sort-decorators.mdxdocs/content/rules/sort-enums.mdxdocs/content/rules/sort-export-attributes.mdxdocs/content/rules/sort-exports.mdxdocs/content/rules/sort-heritage-clauses.mdxdocs/content/rules/sort-import-attributes.mdxdocs/content/rules/sort-imports.mdxdocs/content/rules/sort-interfaces.mdxdocs/content/rules/sort-intersection-types.mdxdocs/content/rules/sort-jsx-props.mdxdocs/content/rules/sort-maps.mdxdocs/content/rules/sort-modules.mdxdocs/content/rules/sort-named-exports.mdxdocs/content/rules/sort-named-imports.mdxdocs/content/rules/sort-object-types.mdxdocs/content/rules/sort-objects.mdxdocs/content/rules/sort-sets.mdxdocs/content/rules/sort-union-types.mdxdocs/content/rules/sort-variable-declarations.mdxrules/sort-array-includes.tsrules/sort-classes.tsrules/sort-decorators.tsrules/sort-enums.tsrules/sort-export-attributes.tsrules/sort-exports.tsrules/sort-heritage-clauses.tsrules/sort-import-attributes.tsrules/sort-intersection-types.tsrules/sort-jsx-props.tsrules/sort-maps.tsrules/sort-modules.tsrules/sort-named-exports.tsrules/sort-named-imports.tsrules/sort-object-types.tsrules/sort-objects.tsrules/sort-union-types.tsrules/sort-variable-declarations.tstest/rules/sort-array-includes.test.tstest/rules/sort-classes.test.tstest/rules/sort-decorators.test.tstest/rules/sort-enums.test.tstest/rules/sort-exports.test.tstest/rules/sort-heritage-clauses.test.tstest/rules/sort-interfaces.test.tstest/rules/sort-intersection-types.test.tstest/rules/sort-jsx-props.test.tstest/rules/sort-maps.test.tstest/rules/sort-modules.test.tstest/rules/sort-named-exports.test.tstest/rules/sort-named-imports.test.tstest/rules/sort-object-types.test.tstest/rules/sort-objects.test.tstest/rules/sort-sets.test.tstest/rules/sort-union-types.test.tstest/rules/sort-variable-declarations.test.tstest/utils/get-newlines-between-option.test.tstest/utils/get-settings.test.tstest/utils/validate-newlines-and-partition-configuration.test.tstypes/common-groups-options.tsutils/get-newlines-between-option.tsutils/get-settings.tsutils/json-schemas/common-groups-json-schemas.tsutils/validate-newlines-and-partition-configuration.ts
🚧 Files skipped from review as they are similar to previous changes (25)
- rules/sort-modules.ts
- rules/sort-jsx-props.ts
- rules/sort-classes.ts
- test/utils/get-settings.test.ts
- rules/sort-array-includes.ts
- utils/get-settings.ts
- rules/sort-variable-declarations.ts
- rules/sort-import-attributes.ts
- rules/sort-heritage-clauses.ts
- rules/sort-export-attributes.ts
- rules/sort-enums.ts
- test/rules/sort-classes.test.ts
- docs/content/rules/sort-modules.mdx
- test/rules/sort-union-types.test.ts
- rules/sort-decorators.ts
- rules/sort-named-imports.ts
- docs/content/rules/sort-named-imports.mdx
- test/rules/sort-interfaces.test.ts
- rules/sort-named-exports.ts
- test/rules/sort-sets.test.ts
- docs/content/rules/sort-variable-declarations.mdx
- docs/content/rules/sort-import-attributes.mdx
- rules/sort-intersection-types.ts
- docs/content/rules/sort-object-types.mdx
- rules/sort-union-types.ts
🧰 Additional context used
🧬 Code graph analysis (5)
utils/get-newlines-between-option.ts (1)
types/common-groups-options.ts (2)
CommonGroupsOptions(75-104)NewlinesInsideOption(213-218)
test/utils/validate-newlines-and-partition-configuration.test.ts (1)
utils/validate-newlines-and-partition-configuration.ts (1)
validateNewlinesAndPartitionConfiguration(101-121)
utils/validate-newlines-and-partition-configuration.ts (4)
types/common-groups-options.ts (2)
CommonGroupsOptions(75-104)NewlinesInsideOption(213-218)types/common-partition-options.ts (1)
CommonPartitionOptions(47-59)utils/is-group-with-overrides-option.ts (1)
isGroupWithOverridesOption(30-34)utils/is-newlines-between-option.ts (1)
isNewlinesBetweenOption(36-40)
test/utils/get-newlines-between-option.test.ts (2)
utils/get-newlines-between-option.ts (2)
getNewlinesBetweenOption(48-71)GetNewlinesBetweenOptionParameters(18-32)types/common-groups-options.ts (4)
CustomGroupsOption(35-73)NewlinesBetweenOption(177-188)NewlinesInsideOption(213-218)GroupsOptions(238-243)
types/common-groups-options.ts (1)
rules/sort-imports/types.ts (1)
CustomTypeOption(96-96)
🔇 Additional comments (57)
test/rules/sort-decorators.test.ts (3)
914-959: Verify unusual indentation in test code/output strings.The test properly covers the new
newlinesInside: 0functionality. However, the code and output strings contain unusual indentation (e.g., line 940:@yand line 946:@bwith leading spaces). This inconsistent formatting doesn't appear in other tests in the file.Is this intentional to test that the rule handles unusual formatting, or is it a copy-paste artifact?
Expected formatting (if unintentional)
code: dedent` @a - @y + @y @z - @b + @b class Class {} `, output: dedent` @a - @b + @b @y - @z + @z class Class {} `,
961-1006: LGTM! Clarifies test intent withnewlinesInside: 'ignore'.Adding
newlinesInside: 'ignore'at line 973 properly isolates this test to verifynewlinesBetween: 0behavior without conflating inside-group newline handling. The preserved blank line at line 1001 (between @y and @z in the same group) correctly reflects the'ignore'setting.
1295-1295: LGTM! Expands test coverage for inline comment interaction.Adding
newlinesInside: 0here ensures the new option is tested alongside inline comments, verifying that comment preservation works correctly with inside-group newline removal.test/rules/sort-variable-declarations.test.ts (2)
1406-1414: Verify test output expectations for newlinesInside: 0Please verify that the expected output correctly reflects the behavior of
newlinesInside: 0. Specifically:
- Lines 1409-1410 show double newlines between
a,(group 'a') andb,(group 'unknown'). This is between-group spacing, which should be controlled bynewlinesBetween(not set explicitly here, so using default).- Lines 1411-1413 show
b, y, zwith no blank lines between them. All three are in the 'unknown' group, sonewlinesInside: 0should indeed remove blank lines between them. ✓The output looks potentially correct, but given the complexity of interactions between
newlinesInside,newlinesBetween, and defaults, it would be good to manually verify this test case produces the expected behavior.
3147-3154: Verify test output with newlinesInside: 'ignore'Please verify this output is correct when
newlinesInside: 'ignore'is set (line 3123).The output shows:
a,(group 'a')b,(group 'unknown')y,(group 'unknown')- (blank line at 3152)
z,(group 'unknown')With
newlinesInside: 'ignore', blank lines inside the 'unknown' group (b, y, z) should be ignored/allowed. However, line 3152 adds a blank line betweenyandzthat doesn't appear to correspond to any spacing in the original input. Please confirm this is the intended behavior of the 'ignore' setting.test/rules/sort-named-imports.test.ts (6)
1146-1193: Excellent update to maintain test focus!Adding
newlinesInside: 'ignore'ensures this test specifically validatesnewlinesBetween: 0behavior without interference from inner-group newline enforcement. This change correctly reflects the PR's breaking change wherenewlinesBetweenandnewlinesInsideare now independently configurable.The updated expectations (errors and output) properly demonstrate that:
- Between-group newlines are removed per
newlinesBetween: 0- Inner-group newlines are preserved per
newlinesInside: 'ignore'
1436-1436: Good defensive configuration.Explicitly setting
newlinesInside: 0ensures this comment-preservation test remains focused and unaffected by default newline behavior changes.Also applies to: 1436-1436
2566-2566: Consistent pattern applied correctly.Setting
newlinesInside: 'ignore'maintains this test's focus onnewlinesBetween: 0behavior across the natural sorting section, matching the pattern established in the alphabetical section.Also applies to: 2566-2566
2844-2844: Appropriate configuration for comment-preservation test.The
newlinesInside: 0setting keeps this test simple and focused on inline comment handling during reordering.Also applies to: 2844-2844
3967-3967: Maintains test isolation effectively.The
newlinesInside: 'ignore'addition ensures this between-group newline test in the line-length section remains properly isolated from inner-group newline enforcement.Also applies to: 3967-3967
4248-4248: Final instance follows the established pattern.Consistent application of
newlinesInside: 0for comment-preservation tests across all sort type sections.Also applies to: 4248-4248
rules/sort-maps.ts (1)
51-66: LGTM! Default aligns with backward compatibility strategy.The
'newlinesBetween'literal value enables the inheritance model wherenewlinesInsidederives its behavior fromnewlinesBetweenwhen not explicitly set, preserving backward compatibility as requested in the PR discussion.rules/sort-objects.ts (1)
69-85: LGTM! Consistent with other rule defaults.rules/sort-object-types.ts (1)
78-94: LGTM! Consistent default option added.test/rules/sort-heritage-clauses.test.ts (4)
525-572: LGTM! Good test coverage for inside-group newline removal.The test correctly verifies that
newlinesInside: 0removes newlines within groups while preserving the blank lines between groups (lines 553-554 in output), as the 'a' and 'unknown' custom groups are separate.
574-621: LGTM! Correctly isolates between-group behavior testing.Setting
newlinesInside: 'ignore'allows the test to focus solely onnewlinesBetween: 0behavior. The preserved blank line at line 616 is expected since it's inside the 'unknown' group andnewlinesInside: 'ignore'doesn't enforce removal.
919-923: LGTM! Explicit newlinesInside setting for clearer test intent.Adding
newlinesInside: 0makes the test's expectations about inside-group spacing explicit.
999-1070: LGTM! Good coverage for per-group newlinesInside overrides.These tests validate both
newlinesInside: 1(enforce spacing) andnewlinesInside: 0(remove spacing) at the custom group level, ensuring the feature works at all configuration scopes.utils/json-schemas/common-groups-json-schemas.ts (1)
205-210: LGTM! Schema correctly allows inheritance value at rule level.The
oneOfstructure appropriately allows'newlinesBetween'as a special value at the rule-options level for inheritance, while the per-group schema (line 175, 296) only exposesnewlinesInsideJsonSchemawithout the inheritance option—which is correct since inheritance fromnewlinesBetweenonly makes sense at the top level.test/rules/sort-intersection-types.test.ts (2)
821-904: Inside- vs between-group newline behavior is well coveredThe two tests around Lines 821–904 cleanly separate responsibilities:
newlinesInside: 0removes extra blank lines only within the group.newlinesBetween: 0withnewlinesInside: 'ignore'removes extra blank lines only between groups and preserves inside‑group spacing.The error expectations and outputs match those semantics and align with the new option’s intent.
1100-1135: Comment + newline handling withnewlinesInside: 0looks correctIn all three “handles newlines and comment after fixes” variants, adding
newlinesInside: 0together withnewlinesBetween: 1ensures:
- A single blank line between groups.
- No extra blank lines inside a group.
- Inline comments stay attached to the correct element.
The before/after code and error lists are consistent with that behavior.
Also applies to: 2680-2714, 4212-4247
test/rules/sort-jsx-props.test.ts (3)
848-895: JSX tests cleanly distinguish inside-group vs between-group newline controlThe new JSX tests correctly exercise:
newlinesInside: 0to collapse extra blank lines only inside a group, leaving group boundaries untouched.newlinesBetween: 0withnewlinesInside: 'ignore'to collapse blank lines only between groups, preserving inside‑group spacing.The reported errors and transformed outputs match these expectations and are consistent with the intersection-types suite.
Also applies to: 897-944
2148-2195: Consistent “between groups” coverage across JSX sort modesIn the natural and line-length describe blocks, the “removes newlines between groups when newlinesBetween is 0” tests:
- Use
newlinesInside: 'ignore'so only between-group spacing is enforced.- Keep intra-group blank lines in the output, validating the ignore semantics.
- Share the same shape as the alphabetical tests, giving good cross-mode coverage.
The options, error messages, and outputs look coherent.
Also applies to: 3396-3446, 3396-3446, 3399-3444, 3396-3446, 3998-4016
1194-1236: Inline comment preservation withnewlinesInside: 0is well-specifiedAdding
newlinesInside: 0alongsidenewlinesBetween: 1in the “preserves inline comments when reordering props” tests ensures:
- No surplus blank lines are allowed within a group.
- Exactly one blank line is kept between groups.
- The trailing inline comment stays attached to the intended prop.
The before/after JSX snippets confirm these invariants for alphabetical, natural, and line-length modes.
Also applies to: 2445-2488, 3699-3742
docs/content/rules/sort-decorators.mdx (1)
254-288: Docs accurately reflect the expanded newline options and override surfaceThe updates to
newlinesBetween, the newnewlinesInsideoption (including the'newlinesBetween'compatibility sentinel), and the addednewlinesInsidefields ingroups/customGroupsall line up with the described behavior and constraints (especially thepartitionByNewLinecaveat). The configuration examples usingnewlinesInside: 'ignore'are consistent and helpful.Also applies to: 291-305, 329-333, 401-422, 438-439, 505-542
rules/sort-exports.ts (1)
67-81: DefaultnewlinesInside: 'newlinesBetween'aligns sort-exports with the BC storyWiring
newlinesInside: 'newlinesBetween'intodefaultOptionsensures this rule:
- Derives its inside-group newline behavior from
newlinesBetweenwhen the user does not explicitly setnewlinesInside, preserving pre-existing configs.- Still allows explicit overrides at rule/options/group level per the shared utilities.
The change is localized and consistent with the broader newline configuration model.
test/rules/sort-modules.test.ts (3)
1794-1839: LGTM! Good test coverage fornewlinesInside: 0behavior.The test correctly validates that
newlinesInside: 0removes extra newlines within the same group while preserving group boundaries. The expected output properly shows the newline between theunknownandinterfacegroups is maintained while intra-group newlines are removed.
1841-1884: Good defensive test update.Adding
newlinesInside: 'ignore'ensures this test continues to focus solely onnewlinesBetweenbehavior without being affected by the newnewlinesInsidefeature. The expected output correctly preserves the internal double newlines (lines 1876-1878) while removing newlines between groups.
1794-1884: DefaultnewlinesInsidebehavior is already tested.The implementation confirms backward-compatible defaults are in place:
newlinesInsideis set to the special value'newlinesBetween'in defaultOptions (line 87 of rules/sort-modules.ts). This same pattern appears across all sorting rules. Tests that usenewlinesBetweenwithout explicitly settingnewlinesInside—such as "maintains single newline between groups when newlinesBetween is 1" (line 1886)—implicitly validate that the default behavior works correctly, ensuring backward compatibility is maintained.test/rules/sort-exports.test.ts (4)
955-998: LGTM! Test correctly validates inside-group newline removal.The test properly exercises the new
newlinesInside: 0option, confirming that newlines within groups are removed while between-group spacing remains untouched. The separation from the between-group test (next test) cleanly validates the orthogonal behavior of the two options.
2339-2382: Consistent test pattern for 'natural' sorting type.The test changes in the 'natural' block mirror the pattern from the 'alphabetical' block, correctly validating
newlinesBetween: 0withnewlinesInside: 'ignore'. The preserved newline at line 2378 is correct for inside-group behavior.Note: Unlike the 'alphabetical' block, there's no separate "removes newlines inside groups when newlinesInside is 0" test in the 'natural' block. This may be intentional to reduce duplication, but verify this is consistent with the test strategy for the codebase.
3714-3753: Consistent test pattern for 'line-length' sorting type.The test changes in the 'line-length' block maintain consistency with the 'alphabetical' and 'natural' blocks, correctly validating the interaction between
newlinesBetween: 0andnewlinesInside: 'ignore'. The preserved newline at line 3743 correctly demonstrates that inside-group newlines are untouched whennewlinesInside: 'ignore'.
1000-1043: The test correctly validates the override behavior wherenewlinesInside: 'ignore'is explicitly set withnewlinesBetween: 0. Default behavior coverage is already provided by other tests in the file (e.g., lines 1045, 1281) that omitnewlinesInsideand rely on the defaultnewlinesInside: 'newlinesBetween'behavior, so no coverage gap exists.test/rules/sort-object-types.test.ts (1)
1688-1729: Good separation of concerns for newline enforcement.The test correctly demonstrates that
newlinesInside: 'ignore'preserves inner-group newlines whilenewlinesBetween: 0removes between-group newlines. The output on line 1724 shows a blank line within the 'unknown' group is preserved as expected.docs/content/rules/sort-maps.mdx (2)
228-234: Consider whether marking the default value as deprecated is the intended design.Line 233 marks the
'newlinesBetween'value as[DEPRECATED], yet line 228 documents it as the default. This creates an unusual situation where the default value is deprecated.Typically, deprecated values are legacy options maintained for backward compatibility, with a non-deprecated value as the default. Was the intent to:
- Keep
'newlinesBetween'as default for backward compatibility (current approach), or- Change the default to
'ignore'and keep'newlinesBetween'only as a deprecated option?Based on the PR objectives noting this is a breaking change and the maintainer's feedback requesting backward compatibility, the current approach appears intentional. However, you might want to rephrase the deprecation notice to make this clearer:
- `'newlinesBetween'` — **For backward compatibility**. If [`newlinesBetween`](#newlinesbetween) is `'ignore'`, then `'ignore'`, otherwise `0`. Consider explicitly setting `newlinesInside` to `'ignore'` or a number for clarity.
207-222: Documentation clearly describes the updated newlinesBetween behavior.The updated documentation correctly explains that
newlinesBetweennow acceptsnumber | 'ignore'and clarifies that non-zero values enforce that many newlines between groups (line 216). The note about overriding via the groups option (lines 218-219) is helpful.test/rules/sort-enums.test.ts (1)
1094-1141: New newline-spacing enum tests look consistent and correctly scopedThe added cases for
newlinesInsidevsnewlinesBetween(including group/customGroup setups and messageIds) accurately exercise intra‑ vs inter‑group spacing and are mirrored acrossalphabetical,natural, andline-lengthmodes without obvious edge‑case gaps.Also applies to: 1143-1190, 2564-2611, 3953-4001
test/rules/sort-maps.test.ts (1)
947-1043: Map newline behavior tests cover inside vs between groups thoroughlyThe new
newlinesInside/newlinesBetweenscenarios (including the dedicated “inside groups” tests and the updated inline‑comment cases) correctly distinguish intra‑group from inter‑group spacing and match the intended error IDs. Coverage acrossalphabetical,natural, andline-lengthlooks comprehensive.Also applies to: 996-1042, 1279-1292, 2338-2385, 2387-2440, 2622-2634, 3680-3729, 3732-3784, 3967-3979
utils/validate-newlines-and-partition-configuration.ts (1)
101-121: Partition/newlines validation logic correctly guards conflicting configurationsThe refactor to delegate into
validateNewlinesBetweenConfigurationandvalidateNewlinesInsideConfigurationcleanly enforces:
partitionByNewLine === true⇒ top‑levelnewlinesBetweenmust be'ignore', and per‑groupnewlinesBetweenobjects must also use'ignore'.- Under partitioning,
newlinesInsideat the top level may only be'ignore'or the bridge'newlinesBetween'token, and group/custom‑group overrides may only useundefined | 'ignore'.This keeps the partition mode “read‑only” with respect to blank lines while still allowing the
'newlinesBetween'sentinel for default derivation when needed.Also applies to: 123-168, 170-188
test/utils/validate-newlines-and-partition-configuration.test.ts (1)
6-55: Validator tests correctly encode the new mutual-exclusion rulesThe new
newlinesBetween/newlinesInsidesuites (including group and customGroup cases plus the combined “all ignore” scenario) map cleanly onto the validator’s behavior and assert the exact error messages. This provides solid coverage for the partition vs newline‑management constraints.Also applies to: 57-138, 140-152
test/rules/sort-objects.test.ts (2)
5593-5632: Natural: mirrors alphabetical semantics; no issues spottedThese natural-order tests mirror the alphabetical ones (including
newlinesInside: 'ignore'for between‑group checks andnewlinesInside: 0for comment‑preservation), so behavior stays consistent across sort types. No changes needed here.Also applies to: 5863-5899
8237-8279: Line-length: consistent separation of inside vs between newlinesThe line-length variants of the “remove newlines between groups” and “preserve comments with
newlinesInside: 0” tests are consistent with the alphabetical/natural sections and correctly validate that spacing behavior is independent of sort strategy.Also applies to: 8510-8529
docs/content/rules/sort-objects.mdx (1)
275-310: Docs fornewlinesBetween/newlinesInsideand group overrides match runtime behaviorThe updated option docs (types, defaults, deprecation note for
'newlinesBetween', and the addednewlinesInsidefield ongroups/customGroups) align with the new semantics in the utilities and tests, including the “derive fromnewlinesBetweenwhen unset” behavior. Looks good.Also applies to: 503-512, 684-685, 702-703, 801-803, 834-836
docs/content/rules/sort-exports.mdx (2)
197-231: Documentation fornewlinesInsidelooks good.The new
newlinesInsideoption is well-documented with clear descriptions of all possible values. The deprecation note for the'newlinesBetween'value is appropriately marked. The cross-references togroupsandcustomGroupsfor per-group overrides are helpful.
240-247: Type definitions are consistent with the new API.The
groupstype properly reflects the updatednewlinesBetween: number | 'ignore'and the newnewlinesInside?: number | 'ignore'options.docs/content/rules/sort-classes.mdx (2)
303-321:newlinesInsidedocumentation is complete and consistent.The documentation follows the same structure as other rule docs, with the deprecation note for
'newlinesBetween'value properly included.
707-749: Custom group interfaces properly updated.Both
CustomGroupDefinitionandCustomGroupAnyOfDefinitioninclude the newnewlinesInside?: number | 'ignore'field, and the attribute description correctly explains its purpose.docs/content/rules/sort-intersection-types.mdx (1)
212-230: Documentation is consistent with other rule docs.The
newlinesInsideoption documentation matches the pattern established in other rule docs, with proper deprecation notes and cross-references.types/common-groups-options.ts (2)
75-104:CommonGroupsOptionsinterface is well-structured with proper deprecation notice.The interface correctly includes the deprecated
'newlinesBetween'value for backward compatibility while documenting its planned removal in V6. The JSDoc comments are clear and helpful.
213-218:NewlinesInsideOptiontype is correctly defined.The type definition is concise and the inline comments clearly explain each option's behavior.
docs/content/rules/sort-jsx-props.mdx (2)
244-261: Documentation is complete and follows the established pattern.The
newlinesInsideoption is properly documented with all values explained and appropriate cross-references included.
350-358: Groups type definition is correct.The type properly includes both
newlinesBetweenandnewlinesInsideoptions with their updated union types.test/utils/get-newlines-between-option.test.ts (2)
62-79: LGTM! Test coverage fornewlinesInsidewhen nodes are in the same group.The test correctly verifies that when
nodeGroupIndex === nextNodeGroupIndex, the function returns thenewlinesInsidevalue. This ensures the inside-group newline behavior is properly enforced.
153-188: The test coverage for'newlinesBetween'value semantics is correct.Lines 154-168 verify that when
newlinesBetween: 'ignore', the function returns'ignore'. Lines 170-187 verify that whennewlinesBetweenis a number (0 or 1), the function returns0. The implementation inutils/get-newlines-between-option.ts(line 150) confirms this behavior:return options.newlinesBetween === 'ignore' ? 'ignore' : 0, which aligns perfectly with the documented deprecation semantics.test/rules/sort-named-exports.test.ts (1)
961-1008:newlinesInside/newlinesBetweentest matrix looks solidThe added and adjusted tests:
Distinguish three behaviors cleanly:
newlinesBetween: 0only, with nonewlinesInside→ removes both inner and inter-group blank lines (back-compat path).- Explicit
newlinesInside: 0→ only inner-group blank lines are removed.newlinesInside: 'ignore'+newlinesBetween: 0→ only between-group blank lines are normalized.Cover this across alphabetical, natural, and line-length variants and also in the “handles newlines and comment after fixes” scenarios, so inline comments and partitions remain stable under the new option.
The expectations (errors, group assignments, and before/after code) are internally consistent with those semantics; no changes needed here.
Also applies to: 1010-1057, 1293-1306, 2287-2334, 2570-2604, 3564-3613, 3850-3883
test/rules/sort-array-includes.test.ts (2)
3073-3122: Natural suite:newlinesInsideinteraction tests look correctFor the natural-order tests:
newlinesInside: 'ignore'+newlinesBetween: 0(3073–3122) cleanly exercises “between-only” spacing fixes, with messages keyed toextraSpacingBetweenArrayIncludesMembersfor the inter-group gap andunexpectedArrayIncludesOrderfor the reordering.- Adding
newlinesInside: 0in the comment-preservation test (3427–3438) makes sure inner-group blank lines are normalized without upsetting the inline comment layout.The configurations, error data, and expected outputs are consistent; behavior matches the intended separation of inside vs between semantics.
Also applies to: 3427-3438
4739-4791: Line-length suite: consistentnewlinesInsidesemantics across sort typesIn the line-length suite:
- The
newlinesInside: 'ignore'+newlinesBetween: 0case (4739–4791) mirrors the alphabetical/natural behavior: only inter-group spacing is tightened while internal blank lines remain allowed.- The comment-preservation test with
newlinesInside: 0(5099–5110) ensures that, even for line-length sorting, inner-group spacing is normalized and inline comments are preserved.This keeps
newlinesInsidebehavior type-agnostic and in line with the rest of the rule set. No issues from the test side.Also applies to: 5099-5110
| <sub> | ||
| type: `number | 'ignore' | 'newlinesBetween'` | ||
| </sub> | ||
| <sub>default: `'newlinesBetween'`</sub> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the sort-enums.mdx file to verify the line numbers and content
fd "sort-enums.mdx" --type fRepository: azat-io/eslint-plugin-perfectionist
Length of output: 112
🏁 Script executed:
# Also search for the other mentioned files to understand the scope
fd "sort-sets.mdx|sort-union-types.mdx|sort-named-exports.mdx|sort-interfaces.mdx|sort-heritage-clauses.mdx" --type fRepository: azat-io/eslint-plugin-perfectionist
Length of output: 277
🌐 Web query:
ESLint plugin backward compatible options migration deprecated default
💡 Result:
Here’s a short, practical migration checklist and a safe pattern to change/deprecate plugin rule options while remaining backward‑compatible and preserving defaults.
Key ESLint references
- Rule deprecation metadata (meta.deprecated / replacedBy). [1]
- Rule author guide / meta, schema, defaultOptions, deprecated flag. [2]
- ESLint migration notes showing how defaults and option changes were handled historically. [3]
Recommended migration steps
- Add explicit metadata
- Set meta.deprecated (boolean or object) and meta.deprecated.replacedBy when the whole rule is deprecated; use meta.deprecated.message/url/deprecatedSince for guidance. [1]
- Keep old behavior by default
- If you change a default option, keep the previous default behavior for at least one minor/patch cycle (or document exact removal version) and provide a migration path in docs. See ESLint migrations for examples. [3]
- Accept both old and new option names
- Update rule schema to accept both shapes (old and new). In create(), detect old option and map it to the new normalized config so runtime uses a single internal shape.
- Emit a clear deprecation warning when old options are used
- When the rule runs, check supplied options and call context.report (or include a message in rule meta.docs/deprecated) to indicate the option is deprecated and point to replacement docs. Prefer a single lint-time report per config use to avoid noisy output. [2][1]
- Provide defaultOptions and schema
- Use meta.defaultOptions so user‑provided options are merged onto known defaults; validate old/new shapes in schema to avoid config-errors. [2]
- Document migration and timeline
- In docs include: why changed, code examples for old → new, semver/version where old will be removed. Use meta.deprecated.deprecatedSince and availableUntil when applicable. [1]
- Remove old option only in a major release
- Follow semver: deprecate in minor, remove in next major; leave a “grave” doc page pointing to replacements. [1][3]
Minimal code pattern (concept)
-
Normalize options and warn once:
function create(context) {
const userOptions = context.options && context.options[0] || {};
let normalized;
if ('oldOption' in userOptions) {
normalized = mapOldToNew(userOptions);
context.report({
loc: { line: 1, column: 0 },
message: "TheoldOptionconfig is deprecated; usenewOption(see docs)."
});
} else {
normalized = mergeDefaults(userOptions);
}
// rule logic usesnormalized
}
Notes
- Use meta.schema to accept both option shapes so ESLint’s config validation doesn’t break users. [2]
- For fully deprecated rules (not just options), prefer meta.deprecated + replacedBy so tooling/IDEs surface replacement info. [1]
- Give an exact removal version in docs (e.g., “deprecated in vX.Y.Z, removed in vX+1.0.0”) to set clear expectations. [1][3]
If you want, I can:
- Draft a precise schema + create() implementation for your actual rule/options, or
- Write the deprecation text and changelog snippet to include in docs.
🏁 Script executed:
# Get the file size first
wc -l docs/content/rules/sort-enums.mdxRepository: azat-io/eslint-plugin-perfectionist
Length of output: 116
🏁 Script executed:
# Read specific sections: around line 238 (lines 230-250)
sed -n '230,250p' docs/content/rules/sort-enums.mdx | cat -nRepository: azat-io/eslint-plugin-perfectionist
Length of output: 1055
🏁 Script executed:
# Read sections around lines 402 and 434
sed -n '395,410p' docs/content/rules/sort-enums.mdx | cat -nRepository: azat-io/eslint-plugin-perfectionist
Length of output: 724
🏁 Script executed:
# Read around line 434
sed -n '427,445p' docs/content/rules/sort-enums.mdx | cat -nRepository: azat-io/eslint-plugin-perfectionist
Length of output: 772
🏁 Script executed:
# Check around line 243 for deprecated marker
sed -n '240,250p' docs/content/rules/sort-enums.mdx | cat -nRepository: azat-io/eslint-plugin-perfectionist
Length of output: 769
🏁 Script executed:
# Check the pattern in other documentation files mentioned
# sort-sets.mdx
grep -n "newlinesInside" docs/content/rules/sort-sets.mdx | head -20Repository: azat-io/eslint-plugin-perfectionist
Length of output: 513
🏁 Script executed:
# Check one more file to see if the same pattern exists
grep -n "newlinesInside" docs/content/rules/sort-union-types.mdx | head -20Repository: azat-io/eslint-plugin-perfectionist
Length of output: 513
🏁 Script executed:
# Let's see if there are actual usage examples with newlinesInside in sort-enums.mdx
grep -n "newlinesInside:" docs/content/rules/sort-enums.mdxRepository: azat-io/eslint-plugin-perfectionist
Length of output: 172
Add explanatory comments to usage examples clarifying backward-compatible defaults.
Line 238 documents newlinesInside default as 'newlinesBetween', which is marked [DEPRECATED] at line 243. However, lines 402 and 434 explicitly set newlinesInside: 'ignore' in configuration examples without explanation. This pattern appears across all documentation files (sort-sets.mdx, sort-union-types.mdx, sort-named-exports.mdx, sort-interfaces.mdx, sort-heritage-clauses.mdx).
Add a comment in examples explaining that 'newlinesBetween' is the default for backward compatibility, but 'ignore' is the recommended modern usage; or omit the option to demonstrate default behavior if backward compatibility is not the intent of the example.
🤖 Prompt for AI Agents
In docs/content/rules/sort-enums.mdx around line 238 (and update the examples at
~lines 402 and 434), the examples set newlinesInside: 'ignore' without
explaining that the documented default is 'newlinesBetween' (deprecated) and
that 'ignore' is the recommended modern choice; update the examples to either
add a short inline comment before or next to newlinesInside explaining:
"'newlinesBetween' is the backward-compatible default (deprecated), prefer
'ignore' for modern usage" or, if the example intends to show default behavior,
remove the explicit newlinesInside option to rely on the default instead.
| it('removes newlines inside groups when newlinesInside is 0', async () => { | ||
| await invalid({ | ||
| errors: [ | ||
| { | ||
| messageId: 'unexpectedObjectTypesOrder', | ||
| data: { right: 'b', left: 'z' }, | ||
| }, | ||
| { | ||
| messageId: 'extraSpacingBetweenObjectTypeMembers', | ||
| data: { right: 'b', left: 'z' }, | ||
| }, | ||
| ], | ||
| output: dedent` | ||
| type Type = { | ||
| a: () => null, | ||
| b: "b", | ||
| y: "y", | ||
| z: "z", | ||
| } | ||
| `, | ||
| code: dedent` | ||
| type Type = { | ||
| a: () => null, | ||
| y: "y", | ||
| z: "z", | ||
| b: "b", | ||
| } | ||
| `, | ||
| options: [ | ||
| { | ||
| ...options, | ||
| groups: ['method', 'unknown'], | ||
| newlinesInside: 0, | ||
| }, | ||
| ], | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cd /repo && head -2000 test/rules/sort-object-types.test.ts | tail -500 | grep -n "newlinesBetween\|newlinesInside" | head -30Repository: azat-io/eslint-plugin-perfectionist
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Search for tests that use newlinesBetween without explicitly setting newlinesInside
rg -n "newlinesBetween.*\d" test/rules/sort-object-types.test.ts -A 15 | head -200Repository: azat-io/eslint-plugin-perfectionist
Length of output: 6295
🏁 Script executed:
#!/bin/bash
# Find all test cases with newlinesBetween option that DO NOT have newlinesInside
rg -n "newlinesBetween" test/rules/sort-object-types.test.ts | head -50Repository: azat-io/eslint-plugin-perfectionist
Length of output: 2949
🏁 Script executed:
#!/bin/bash
# Check the implementation to understand what newlinesInside default is
find . -type f -name "*.ts" | xargs rg "newlinesInside.*default\|newlinesBetween.*default" 2>/dev/null | head -20Repository: azat-io/eslint-plugin-perfectionist
Length of output: 61
🏁 Script executed:
#!/bin/bash
# 1. Search for tests with newlinesBetween set to 0 or 1
echo "=== Tests with newlinesBetween (last 1650 lines of test file) ==="
rg -n "newlinesBetween" test/rules/sort-object-types.test.ts | tail -50
echo ""
echo "=== Count of newlinesBetween occurrences ==="
rg -c "newlinesBetween" test/rules/sort-object-types.test.ts
echo ""
echo "=== Count of newlinesInside occurrences ==="
rg -c "newlinesInside" test/rules/sort-object-types.test.ts
echo ""
echo "=== Tests that have newlinesBetween but NOT newlinesInside nearby ==="
# Look for patterns like newlinesBetween without newlinesInside in 20-line chunks
rg -B5 -A15 "newlinesBetween.*[0-9]" test/rules/sort-object-types.test.ts | grep -v "newlinesInside" | head -80Repository: azat-io/eslint-plugin-perfectionist
Length of output: 5209
🏁 Script executed:
#!/bin/bash
# Search for newlinesBetween usage in tests
echo "Lines with newlinesBetween in sort-object-types.test.ts:"
rg -n "newlinesBetween" test/rules/sort-object-types.test.ts
echo ""
echo "=== Checking lines 1645-1686 context ==="
sed -n '1630,1700p' test/rules/sort-object-types.test.ts | head -80Repository: azat-io/eslint-plugin-perfectionist
Length of output: 5616
🏁 Script executed:
rg -n "newlinesBetween" test/rules/sort-object-types.test.ts | wc -lRepository: azat-io/eslint-plugin-perfectionist
Length of output: 81
🏁 Script executed:
rg -B10 -A10 "newlinesBetween: [0-9]" test/rules/sort-object-types.test.ts | grep -c "newlinesInside"Repository: azat-io/eslint-plugin-perfectionist
Length of output: 80
🏁 Script executed:
find . -type f -name "*.ts" -not -path "*/node_modules/*" | xargs rg -l "newlinesInside.*default|newlinesBetween.*newlinesInside" 2>/dev/null | head -10Repository: azat-io/eslint-plugin-perfectionist
Length of output: 208
🏁 Script executed:
rg -n "newlinesBetween|newlinesInside" --type ts rules/sort-object-types.ts | head -40Repository: azat-io/eslint-plugin-perfectionist
Length of output: 150
🏁 Script executed:
rg -B5 -A5 "newlinesInside.*newlinesBetween\|newlinesBetween.*newlinesInside" . --type ts 2>/dev/null | head -80Repository: azat-io/eslint-plugin-perfectionist
Length of output: 61
🏁 Script executed:
sed -n '70,95p' rules/sort-object-types.tsRepository: azat-io/eslint-plugin-perfectionist
Length of output: 737
🏁 Script executed:
rg -B10 -A10 "newlinesInside.*newlinesBetween" rules/sort-object-types.tsRepository: azat-io/eslint-plugin-perfectionist
Length of output: 688
🏁 Script executed:
rg -B5 -A10 "newlinesInside\|newlinesBetween" utils/validate-newlines-and-partition-configuration.ts | head -60Repository: azat-io/eslint-plugin-perfectionist
Length of output: 61
🏁 Script executed:
rg -B10 -A10 "newlinesInside.*newlinesBetween" utils/validate-newlines-and-partition-configuration.tsRepository: azat-io/eslint-plugin-perfectionist
Length of output: 61
Add test verifying backward compatibility when only newlinesBetween is configured.
The implementation correctly defaults newlinesInside to 'newlinesBetween', enabling backward compatibility for users upgrading without changing their configs. However, no test explicitly verifies this implicit default resolution. Add a test case that sets only newlinesBetween without newlinesInside to ensure the default mechanism works:
it('applies newlinesBetween as default for newlinesInside when not explicitly set', async () => {
await valid({
code: dedent`
type Type = {
a: () => null,
b: "b",
}
`,
options: [
{
...options,
groups: ['method', 'unknown'],
newlinesBetween: 1,
// newlinesInside intentionally omitted to test default resolution
},
],
})
})This ensures users upgrading with only newlinesBetween set receive the expected behavior without configuration changes.
🤖 Prompt for AI Agents
In test/rules/sort-object-types.test.ts around lines 1645 to 1686, add a new
valid test that ensures backward compatibility by verifying that when
newlinesInside is not provided the rule uses newlinesBetween as the default;
specifically, add a test case that supplies options with groups:
['method','unknown'] and only newlinesBetween set (e.g. 1) and omits
newlinesInside, then assert the code snippet with one newline between members is
considered valid. Ensure the test uses the existing valid(...) helper, dedent
for the code block, and mirrors the style of nearby tests.
|
Should we also update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/rules/sort-variable-declarations.test.ts (1)
1381-1426: Test output doesn't clearly demonstratenewlinesInside: 0behavior.The test name correctly states "removes newlines inside groups when newlinesInside is 0" (addressing the past review comment), but the expected output doesn't clearly demonstrate this. The prominent blank lines in the output (lines 1409-1410) are between groups ('a' and 'unknown'), which should be controlled by
newlinesBetween(notnewlinesInside).The actual inside-group newline removal (the blank line between
zandbin the original 'unknown' group) is hard to verify in the output format. Consider restructuring this test to more clearly demonstrate thenewlinesInside: 0behavior, perhaps by:
- Using a test case where newlines inside a group are more prominent
- Setting
newlinesBetweenexplicitly to avoid confusion- Or splitting into separate tests for reordering vs. newline removal
Good catch 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/content/guide/getting-started.mdx (1)
153-153: Good addition—consider mentioning the default behavior.The documentation clearly describes the new
newlinesInsideoption and its valid values. This addresses the maintainer's request to update documentation.To improve user understanding, consider mentioning that when
newlinesInsideis not explicitly set, its default value derives fromnewlinesBetween(numericnewlinesBetween→newlinesInside: 0;newlinesBetween === 'ignore'→newlinesInside: 'ignore').Additionally, the example configurations (lines 160-206) could be enhanced by demonstrating
newlinesInsideusage, though this is optional.💡 Suggested enhancement to mention default behavior
-- `newlinesInside` — Specifies how to handle newlines between elements of each group. Values can be `'ignore'`, `'newlinesBetween'` or a number. +- `newlinesInside` — Specifies how to handle newlines between elements of each group. Values can be `'ignore'`, `'newlinesBetween'` or a number. When not explicitly set, defaults to `'newlinesBetween'`.
azat-io
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
test/rules/sort-variable-declarations.test.ts (1)
1381-1428: Consider clarifying "by default" in test name.The test name "removes newlines between and inside groups by default when 'newlinesBetween' is 0" uses "by default" to mean "when
newlinesInsideis not explicitly set and derives its default fromnewlinesBetween". This could be misread as "by default configuration" (no options set). Consider rewording for clarity:
- "removes newlines between and inside groups when 'newlinesBetween' is 0 and 'newlinesInside' uses derived default"
- "removes newlines between and inside groups when only 'newlinesBetween' is 0"
The current name is technically accurate but slightly ambiguous.
test/rules/sort-classes.test.ts (1)
8427-8467: Natural mode mirrors the alphabetical behavior appropriately.
- The
newlinesInside: 'ignore', newlinesBetween: 0test correctly verifies that only between-group blank lines (betweena()andy) are removed, while intra-group spacing (e.g. beforez) is left untouched.- The updated options with
newlinesInside: 0in the “handles newlines and comment after fixes” test keeps the same semantics as in the alphabetical block and still preserves the inline comment association.These ensure the new configuration behaves consistently under
type: 'natural'. Looks good.If this rule doesn’t already have it elsewhere, you might optionally add a small “default derivation” test for
newlinesBetween: 'ignore'→newlinesInside: 'ignore'under natural mode, analogous to thenewlinesBetween: 0case, just to guard against regressions in that branch.Also applies to: 8755-8761
test/rules/sort-object-types.test.ts (2)
1645-1688: Add test for positive backward compatibility case withnewlinesBetween: 1.The test at line 1645 verifies backward compatibility when
newlinesBetween: 0is set withoutnewlinesInside, correctly expecting both inside and between newlines to be removed. However, there's no test for the positive case wherenewlinesBetween: 1(or another positive number) is set withoutnewlinesInsideto verify that newlines are enforced/preserved inside groups by default.Add a test that verifies users upgrading with only
newlinesBetween: 1set (without specifyingnewlinesInside) receive the expected behavior:it('enforces newlines between and inside groups by default when newlinesBetween is 1', async () => { await valid({ code: dedent` type Type = { a: () => null, b: "b", y: "y", } `, options: [ { ...options, groups: ['method', 'unknown'], newlinesBetween: 1, // newlinesInside intentionally omitted to test default resolution }, ], }) })This ensures complete backward compatibility coverage for users upgrading without configuration changes.
Based on past review comments.
4977-5018: Consider adding positive backward compatibility tests for natural and line-length sections.Similar to the alphabetical section (lines 1645-1688), the natural and line-length sections should also include tests verifying positive backward compatibility when
newlinesBetween: 1is set withoutnewlinesInside. This ensures consistent coverage across all sorting type sections.Based on similar concern for alphabetical section.
Also applies to: 7335-7379
test/rules/sort-objects.test.ts (1)
2118-2124: Make sort type explicit in comment‑preservation tests by spreadingoptions(optional)These three tests now exercise
newlinesInside: 0, but they no longer spread the mode‑specificoptionsobject, so they rely on the rule’s default type/order instead of explicitly using'alphabetical' | 'natural' | 'line-length'. To keep coverage tied to each mode and future‑proof against default changes, consider:Example diff for the alphabetical block (apply similarly to natural / line‑length)
- options: [ - { - groups: ['unknown', 'method'], - newlinesBetween: 1, - newlinesInside: 0, - }, - ], + options: [ + { + ...options, + groups: ['unknown', 'method'], + newlinesBetween: 1, + newlinesInside: 0, + }, + ],Also applies to: 5921-5927, 8568-8574
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
test/rules/sort-array-includes.test.tstest/rules/sort-classes.test.tstest/rules/sort-decorators.test.tstest/rules/sort-enums.test.tstest/rules/sort-exports.test.tstest/rules/sort-heritage-clauses.test.tstest/rules/sort-interfaces.test.tstest/rules/sort-intersection-types.test.tstest/rules/sort-jsx-props.test.tstest/rules/sort-maps.test.tstest/rules/sort-modules.test.tstest/rules/sort-named-exports.test.tstest/rules/sort-named-imports.test.tstest/rules/sort-object-types.test.tstest/rules/sort-objects.test.tstest/rules/sort-sets.test.tstest/rules/sort-union-types.test.tstest/rules/sort-variable-declarations.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/rules/sort-modules.test.ts
- test/rules/sort-enums.test.ts
- test/rules/sort-exports.test.ts
- test/rules/sort-intersection-types.test.ts
🔇 Additional comments (43)
test/rules/sort-array-includes.test.ts (1)
1291-1340: Excellent test coverage for the newnewlinesInsideoption!The test additions comprehensively verify the new functionality:
Lines 1291-1340: Correctly tests the derived default behavior where
newlinesBetween: 0impliesnewlinesInside: 0, ensuring backward compatibility.Lines 1342-1387: Effectively isolates
newlinesInside: 0behavior, verifying that intra-group newlines are removed while inter-group spacing is preserved (note the preserved double newline between groups in the expected output).Strategic
newlinesInside: 'ignore'additions (lines 1400, 1753, etc.): These updates ensure existingnewlinesBetweentests remain focused on inter-group behavior without interference from the new option.The pattern is correctly replicated across all sort types (alphabetical, natural, line-length), and the expected outputs accurately reflect the intended semantics.
Also applies to: 1342-1387, 1400-1400, 1753-1753
test/rules/sort-sets.test.ts (4)
1092-1143: LGTM! Default derivation behavior is correctly tested.This test properly verifies that when
newlinesBetween: 0is set without explicitnewlinesInside, the defaultnewlinesInsideis derived as 0, resulting in removal of both between-group and inside-group newlines. The error expectations and output align with this behavior.
1145-1194: LGTM! Independent control of inside-group newlines is correctly tested.This test properly demonstrates that
newlinesInside: 0independently controls inside-group spacing without affecting between-group spacing. The preserved double newline in the output (lines 1174-1175) and error expectations correctly reflect this behavior.
1196-1245: LGTM! Independent control of between-group newlines is correctly tested.This test properly demonstrates that
newlinesBetween: 0withnewlinesInside: 'ignore'removes only between-group spacing while preserving inside-group newlines. The preserved newline at line 1239 correctly reflects this behavior.
1502-1502: LGTM! Backward compatibility is properly maintained.The explicit
newlinesInsidesettings in existing tests correctly maintain backward compatibility. Tests at lines 1502, 2925, and 4351 explicitly setnewlinesInside: 0to preserve the original behavior, while tests at lines 2630 and 4053 usenewlinesInside: 'ignore'to demonstrate independent control. The updated outputs at lines 2662 and 4088 correctly reflect preserved inside-group newlines.Also applies to: 2630-2630, 2662-2662, 2925-2925, 4053-4053, 4088-4088, 4351-4351
test/rules/sort-variable-declarations.test.ts (2)
1430-1475: Well-structured test fornewlinesInsideisolation.This test effectively demonstrates that
newlinesInside: 0only controls inside-group newlines while preserving between-group spacing (the double newline after'a'in the output). The error expectations correctly focus on inside-group spacing violations without flagging between-group newlines.
3172-3203: Good use ofnewlinesInside: 'ignore'to isolate test concerns.Adding
newlinesInside: 'ignore'to these "removes newlines between groups" tests properly focuses them on between-group behavior only. The preserved inside-group newlines in the expected outputs (lines 3201, 4931) and reduced error arrays confirm the option works as intended. This pattern is consistently applied across the natural and line-length test suites.Also applies to: 4912-4934
test/rules/sort-decorators.test.ts (4)
914-961: Excellent test coverage for default behavior.This test correctly validates that when
newlinesBetween: 0is set without explicitly settingnewlinesInside, the default behavior removes newlines both between and inside groups. This aligns with the backward compatibility approach described in the PR objectives, wherenewlinesInsidederives fromnewlinesBetweenwhen not explicitly set.
963-1008: Well-designed test for explicitnewlinesInside: 0.This test validates that explicitly setting
newlinesInside: 0removes newlines within groups (lines 1002-1004:@b,@y,@zwith no spacing) while preserving the between-group spacing (blank lines 1000-1001 between@aand the unknown group). This demonstrates proper isolation between thenewlinesInsideandnewlinesBetweenoptions.
1010-1055: Good test coverage for combined options.This test correctly validates the interaction between
newlinesBetween: 0andnewlinesInside: 'ignore'. The output shows between-group spacing is removed (line 1047-1048:@adirectly followed by@b) while inside-group spacing is preserved (lines 1050-1051: blank line between@yand@zwithin the 'unknown' group). This demonstrates the options work independently as intended.
1344-1344: Good addition to validate inline comment handling.Adding
newlinesInside: 0to the inline comments test ensures the new option doesn't interfere with comment preservation logic. This is a sensible edge case to cover.test/rules/sort-named-exports.test.ts (3)
961-1108: LGTM! Comprehensive test coverage fornewlinesInsideoption.The new test cases clearly validate the interaction between
newlinesInsideandnewlinesBetween:
- Line 961: Updated description accurately reflects default behavior when
newlinesBetweenis numeric- Lines 1012-1059: Correctly tests that
newlinesInside: 0removes newlines within groups while preserving between-group spacing- Lines 1061-1108: Correctly tests that
newlinesInside: 'ignore'preserves within-group newlines whilenewlinesBetween: 0removes between-group spacingThe expected outputs and error assertions align with the PR's objective of making
newlinesInsidederive fromnewlinesBetweenby default.
1356-1356: Good practice: Explicit configuration in existing tests.Adding explicit
newlinesInside: 0to these tests that handle newlines and comments makes the test intent clearer and ensures consistent behavior across all sorting type sections.Also applies to: 2633-2633, 3913-3913
2338-2385: Excellent consistency across sorting types.The tests for
naturalandline-lengthsorting types mirror thealphabeticalsection's test structure, ensuring thenewlinesInsideoption works consistently regardless of sort method. The expected outputs correctly show that withnewlinesInside: 'ignore'andnewlinesBetween: 0:
- Newlines between different groups are removed (e.g., between 'a' and 'unknown' groups)
- Newlines within the same group are preserved (e.g., between 'y' and 'z' in the 'unknown' group)
Also applies to: 3615-3665
test/rules/sort-heritage-clauses.test.ts (1)
525-623: Excellent test coverage for the newnewlinesInsideoption!The test additions comprehensively validate the new feature:
Line 525: The renamed test clearly documents that
newlinesBetween: 0removes both between-group and inside-group newlines by default (sincenewlinesInsidederives fromnewlinesBetween).Lines 576-623: New test validates
newlinesInside: 0explicitly, confirming that inside-group newlines are removed while between-group spacing is preserved whennewlinesBetweenis not set.Lines 625-672: New test demonstrates that
newlinesInsideandnewlinesBetweencan be configured independently—newlinesBetween: 0removes inter-group spacing whilenewlinesInside: 'ignore'preserves intra-group newlines.Line 972: Addition ensures the feature works correctly with inline comments.
The error assertions and output expectations are accurate, and the test names clearly document the expected behavior.
Also applies to: 625-672, 972-972
test/rules/sort-maps.test.ts (4)
947-996: LGTM! Clear test for default behavior.The test name and implementation correctly validate that when
newlinesBetween: 0is set without an explicitnewlinesInside, the default behavior removes newlines both between and inside groups. This aligns with the PR's design wherenewlinesInsideinherits fromnewlinesBetweenwhen unspecified.
998-1045: LGTM! Excellent test for isolatednewlinesInsidebehavior.This test effectively validates that
newlinesInside: 0removes extra newlines inside groups while preserving between-group spacing. The expected output correctly shows two blank lines between group 'a' and 'unknown' (inherited from defaultnewlinesBetween), while removing the blank line within the 'unknown' group.
1047-1094: LGTM! Well-designed test for independent control.This test nicely demonstrates the separation between
newlinesBetweenandnewlinesInside. SettingnewlinesBetween: 0removes between-group spacing whilenewlinesInside: 'ignore'preserves the existing newline within the 'unknown' group. The expected output and error assertions correctly reflect this independent control.
1342-1342: LGTM! Comprehensive test coverage across sorting types.The additions of
newlinesInsideto existing tests and the new test cases provide excellent coverage of the feature. The consistent replication across 'alphabetical', 'natural', and 'line-length' sorting types ensures the feature works uniformly. Key scenarios covered include:
- Default behavior (inheritance from
newlinesBetween)- Explicit
newlinesInside: 0(removes inside-group spacing)- Explicit
newlinesInside: 'ignore'(preserves inside-group spacing)- Combinations with various
newlinesBetweenvalues- Interaction with custom groups and other features
Also applies to: 2389-2436, 2684-2684, 3731-3781, 4029-4029
test/rules/sort-jsx-props.test.ts (6)
848-897: LGTM! Test correctly validates defaultnewlinesInsidederivation behavior.The test title accurately describes that when
newlinesBetween: 0is set without an explicitnewlinesInsidevalue, the default behavior removes both between-group and inside-group newlines (sincenewlinesInsidedefaults to0whennewlinesBetweenis numeric). The expected errors and output correctly validate this behavior.
899-946: LGTM! New test correctly validates explicitnewlinesInside: 0behavior.This test properly verifies that when
newlinesInside: 0is explicitly set, newlines are removed inside groups (betweeny,z,bin theunknowngroup) while preserving newlines between groups (betweenagroup andunknowngroup). The expected errors and output are correct.
948-995: LGTM! New test correctly validatesnewlinesInside: 'ignore'withnewlinesBetween: 0.This test properly demonstrates the interaction between
newlinesInside: 'ignore'andnewlinesBetween: 0:
- Between-group newlines are removed (between
agroup andunknowngroup)- Inside-group newlines are preserved (between
yandzwithin theunknowngroup)The expected errors and output correctly reflect this behavior.
1257-1257: LGTM! ExplicitnewlinesInside: 0correctly configures the test.This addition ensures that while newlines are added between groups (
newlinesBetween: 1), no newlines are added inside theb|cgroup. Without this explicit setting,newlinesInsidewould default to1(derived fromnewlinesBetween: 1), which would add unwanted newlines betweenbandc.
2199-2246: LGTM! Test modification correctly demonstratesnewlinesInside: 'ignore'behavior.The addition of
newlinesInside: 'ignore'(line 2211) clarifies that this test is specifically validating that inside-group newlines are preserved while between-group newlines are removed (withnewlinesBetween: 0). The updated errors and output correctly reflect this behavior, consistent with the alphabetical section's equivalent test.
3447-3497: LGTM! Test modification correctly demonstratesnewlinesInside: 'ignore'behavior.The addition of
newlinesInside: 'ignore'(line 3459) clarifies that this test validates inside-group newline preservation while removing between-group newlines (withnewlinesBetween: 0). The updated errors and output are correct and consistent with the equivalent tests in the alphabetical and natural sections.test/rules/sort-interfaces.test.ts (3)
1786-1915: LGTM! Comprehensive test coverage for the newnewlinesInsideoption.The three new/modified tests provide excellent coverage for the
newlinesInsidefeature:
- Default behavior (line 1786): Verifies that when only
newlinesBetween: 0is set,newlinesInsidecorrectly derives to0, removing both between-group and inside-group newlines.- Explicit inside control (line 1831): Confirms that
newlinesInside: 0removes inside-group newlines while preserving between-group spacing.- Independent control (line 1874): Demonstrates that
newlinesBetween: 0withnewlinesInside: 'ignore'removes only between-group newlines, preserving inside-group newlines.The test assertions correctly verify both the output code and the specific error messages, ensuring the feature works as designed per the PR objectives.
2179-2179: Good addition ofnewlinesInside: 0to existing tests.These additions ensure that the "handles newlines and comment after fixes" tests correctly verify that no inside-group newlines are introduced during sorting, which is important for maintaining the expected output format across all three sorting types (alphabetical, natural, line-length).
Also applies to: 4720-4720, 7122-7122
4415-4456: Consistent cross-sorting-type validation.The modifications to the natural and line-length sorting tests ensure that the
newlinesInsideoption works consistently across all sorting algorithms, not just alphabetical. The tests correctly verify thatnewlinesInside: 'ignore'preserves inside-group newlines whilenewlinesBetween: 0removes between-group newlines.Also applies to: 6814-6857
test/rules/sort-union-types.test.ts (2)
822-951: Excellent test coverage for the newnewlinesInsideoption.The three new/updated tests clearly demonstrate the different behaviors:
- Line 822: Tests the default derived behavior where
newlinesInsideinherits fromnewlinesBetweenwhen not explicitly set- Lines 868-907: Verifies that explicit
newlinesInside: 0removes inside-group newlines while preserving between-group spacing- Lines 909-951: Confirms that
newlinesInside: 'ignore'preserves inside-group newlines whilenewlinesBetween: 0removes between-group spacingThis clearly validates the backward compatibility approach described in the PR objectives, where the default for
newlinesInsidederives fromnewlinesBetweento maintain existing behavior.
1164-1164: Good addition of explicitnewlinesInsidesettings to existing tests.These additions ensure that existing test scenarios are explicit about their newline behavior expectations, preventing ambiguity now that
newlinesInsidecan be configured independently. This improves test clarity and maintainability.Also applies to: 2515-2515, 2751-2751, 4043-4043, 4279-4279
test/rules/sort-named-imports.test.ts (5)
1097-1146: Excellent test for default derivation behavior.This test correctly verifies that when
newlinesBetween: 0is set without explicitly settingnewlinesInside, the default fornewlinesInsideis derived as0, removing both inter-group and intra-group newlines. The three expected errors appropriately cover spacing between groups, element ordering, and spacing inside groups.
1148-1244: Well-structured tests for independent newline control.These two tests excellently demonstrate the independence of
newlinesInsideandnewlinesBetween:
- Lines 1148-1195: Setting only
newlinesInside: 0removes newlines inside groups while preserving those between groups- Lines 1197-1244: Setting
newlinesBetween: 0withnewlinesInside: 'ignore'does the oppositeThe error expectations correctly distinguish between inter-group and intra-group spacing violations, validating the core functionality of this feature.
1487-1487: Good practice: explicit option values in existing tests.Adding explicit
newlinesInsidevalues to existing tests improves clarity and maintainability. These modifications ensure the tests aren't relying on implicit defaults and make the expected behavior explicit, particularly in tests that verify comment preservation and newline handling.Also applies to: 2617-2617, 2895-2895, 4018-4018, 4299-4299
1097-1146: Comprehensive test coverage across sort types.The similar test patterns across alphabetical, natural, and line-length sort types ensure the
newlinesInsidefeature works consistently across all sorting modes. This duplication is intentional and represents thorough test coverage.Also applies to: 2605-2652, 4006-4056
2621-2630: Correctly updated error expectations and outputs.The modified error arrays and outputs correctly reflect the
newlinesInside: 'ignore'behavior:
- No errors are raised for intra-group spacing (as expected when ignoring)
- Errors correctly identify inter-group spacing violations and ordering issues
- Outputs show preserved newlines inside groups while enforcing inter-group rules
These changes accurately validate the independent control of inter-group vs intra-group newline behavior.
Also applies to: 2647-2647, 4022-4034, 4051-4051
test/rules/sort-classes.test.ts (3)
3828-3956: Well-structured coverage ofnewlinesBetweenvsnewlinesInsidesemantics (alphabetical mode).The three tests here nicely pin down the intended behavior:
newlinesBetween: 0with nonewlinesInsideset → both between-group and in-group blank lines are removed (default derivation).- Explicit
newlinesInside: 0only → in-group blank lines are removed while inter-group spacing is left alone.newlinesInside: 'ignore'withnewlinesBetween: 0→ only between-group spacing is normalized; in-group spacing is preserved.The error expectations and outputs all align with those cases, including correct grouping (
methodvsunknown) and ordering. No changes needed.
4244-4250: ExplicitnewlinesInside: 0in “handles newlines and comment after fixes” (alphabetical) looks correct.Adding
newlinesInside: 0alongsidenewlinesBetween: 1andgroups: ['property', 'method']is consistent with the new option semantics and ensures method overloads within themethodgroup have no internal blank lines while keeping a single blank line between thepropertyandmethodgroups. The expected output also preserves the trailing comment onacorrectly. No issues here.
12882-12918: Line-length mode spacing tests are consistent with the new options.
- The
newlinesInside: 'ignore', newlinesBetween: 0test fortype: 'line-length'correctly reports only the extra spacing between groups (a→b) and leaves intra-group blank lines intact, matching the intended separation of concerns.- Updating the “handles newlines and comment after fixes” test to include
newlinesInside: 0matches the alphabetical/natural variants and keeps both the spacing and the trailing comment behavior correct.No functional or expectation issues found in these hunks.
Also applies to: 13206-13212
test/rules/sort-object-types.test.ts (2)
1690-1774: LGTM! Clear separation of inside vs between group newline control.These tests effectively demonstrate the independent control of
newlinesInsideandnewlinesBetween:
- Line 1690: Setting only
newlinesInside: 0removes only inside-group newlines while preserving between-group spacing.- Line 1733: Setting
newlinesInside: 'ignore'withnewlinesBetween: 0removes only between-group newlines while preserving inside-group spacing.This clear distinction validates that the two options work independently as intended.
2043-2043: LGTM! ExplicitnewlinesInsideimproves test clarity.Adding explicit
newlinesInside: 0to the "handles newlines and comment after fixes" tests across all sorting type sections makes the test expectations clearer and ensures the tests remain deterministic with the new option.Also applies to: 5287-5287, 7648-7648
test/rules/sort-objects.test.ts (3)
1746-1875: Well-structured coverage of default vs explicitnewlinesInside/newlinesBetweenbehaviorThe three tests here nicely separate: (1) default
newlinesInsidederived fromnewlinesBetween: 0, (2) explicitnewlinesInside: 0only, and (3)newlinesInside: 'ignore'withnewlinesBetween: 0. Code, options, and expected outputs are consistent with the intended precedence and grouping semantics.
5637-5673: Natural sort:newlinesBetween/newlinesInsideinteraction looks correctOverriding
newlinesInside: 'ignore'while keepingnewlinesBetween: 0keeps this test focused on between‑group spacing, and the expected output correctly preserves the intra‑group blank line while removing the one between the method and unknown groups.
8281-8323: Line-length sort: consistent handling of between‑group vs intra‑group newlinesThis mirrors the natural/alpha tests:
newlinesBetween: 0plusnewlinesInside: 'ignore'removes only the group boundary blank lines while retaining the intra‑group blank. The error expectations and output match that intent.
newLinesInsideat rule options and settings levels #652.Description
This PR adds
newlinesInsideto all rules that support grouping (all exceptsort-switch-case).Previously,
newlinesBetweendifferent thanignoremeant that no newline was tolerated by default within groups, as per the documented:Now, as
newlinesInsideisignoreby default for all rules butsort-imports(which hasnewlinesInside: 0to be isofunctional with its defaultnewlinesBetween: 1), errors won't be raised anymore if a newline exist within groups.This means that users will have to manually add
newlinesInside: 0to their rule options.What is the purpose of this pull request?