-
-
Notifications
You must be signed in to change notification settings - Fork 724
feat(useSortedKeys): add groupByNesting option #7799
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
base: next
Are you sure you want to change the base?
feat(useSortedKeys): add groupByNesting option #7799
Conversation
Implements a new `groupByNesting` option for the `useSortedKeys` assist that groups object keys by their value's nesting depth before applying alphabetical sorting. Nesting depth classification: - Depth 0 (simple): primitives, single-line arrays - Depth 1 (nested): objects, multi-line arrays When enabled, simple values are sorted alphabetically first, followed by nested values sorted alphabetically. This addresses the common use case where developers want scalar properties at the top of objects, with complex nested structures appearing at the bottom. Implementation details: - Added `group_by_nesting: bool` field to `UseSortedKeysOptions` - Implemented `get_nesting_depth_js()` for JavaScript AST nodes - Implemented `get_nesting_depth()` for JSON AST nodes - Updated sorting comparators to use tuple (depth, name) ordering - Works with both natural and lexicographic sort orders Test coverage: - 4 JS test cases (natural + lexicographic × grouping on/off) - 4 JSON test cases (natural + lexicographic × grouping on/off) - All snapshot tests generated and validated Configuration: ```json { "assist": { "actions": { "source": { "useSortedKeys": { "level": "on", "options": { "groupByNesting": true } } } } } } ```
🦋 Changeset detectedLatest commit: 092b620 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis pull request introduces a new Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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
🧹 Nitpick comments (5)
.changeset/group-by-nesting-feature.md (1)
5-7
: Polish the changeset text for house style.
- End sentences with a period; rephrase “Will be sorted as:” to a full sentence.
- Add a link to the assist page when mentioning
useSortedKeys
.Example tweak:
-Added `groupByNesting` option to the `useSortedKeys` assist. When enabled, object keys are grouped by their value's nesting depth before sorting alphabetically. +Added `groupByNesting` option to the `useSortedKeys` assist. When enabled, object keys are grouped by their value's nesting depth, then sorted using the configured order (natural or lexicographic). @@ -#### Example +#### Example. @@ -Will be sorted as: +The object will be sorted as follows.Also applies to: 9-37
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs (3)
134-153
: Doc nit: tie wording to configured sort order.“before sorting alphabetically” isn’t precise given natural vs lexicographic. Prefer: “before sorting by the configured key order (natural or lexicographic).”
164-179
: Depth classification misses function/class expressions.Values like
foo: () => {}
orfoo: function() {}
andfoo: class {}
are currently treated as simple (depth 0). If the intent is “non‑primitives as nested”, mark these as depth 1 (and perhaps only if multi‑line). Happy to draft a follow‑up patch.Do you want these treated as nested? If yes, I can extend
get_nesting_depth_js
to handleJsArrowFunctionExpression
,JsFunctionExpression
, andJsClassExpression
.
246-287
: Action message could mention grouping when enabled.Minor: when
groupByNesting
is on, consider “Sort the object properties by group (nesting) and key.” for clarity.crates/biome_json_analyze/src/assist/source/use_sorted_keys.rs (1)
108-124
: Avoidto_string()
allocation; mirror JS detection.Use
array.syntax().text_trimmed().contains_char('\n')
for multi‑line detection to reduce allocations and keep parity with the JS path.- AnyJsonValue::JsonArrayValue(array) => { - // Check if 8000 array spans multiple lines by looking for newlines - if array.to_string().contains('\n') { + AnyJsonValue::JsonArrayValue(array) => { + // Multi-line if the text contains a newline. + if array.syntax().text_trimmed().contains_char('\n') { 1 } else { 0 } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.js.snap
is excluded by!**/*.snap
and included by**
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.js.snap
is excluded by!**/*.snap
and included by**
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.json.snap
is excluded by!**/*.snap
and included by**
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.json.snap
is excluded by!**/*.snap
and included by**
packages/@biomejs/backend-jsonrpc/src/workspace.ts
is excluded by!**/backend-jsonrpc/src/workspace.ts
and included by**
packages/@biomejs/biome/configuration_schema.json
is excluded by!**/configuration_schema.json
and included by**
📒 Files selected for processing (12)
.changeset/group-by-nesting-feature.md
(1 hunks)crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
(5 hunks)crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.js
(1 hunks)crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json
(1 hunks)crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.js
(1 hunks)crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json
(1 hunks)crates/biome_json_analyze/src/assist/source/use_sorted_keys.rs
(5 hunks)crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.json
(1 hunks)crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json
(1 hunks)crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.json
(1 hunks)crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json
(1 hunks)crates/biome_rule_options/src/use_sorted_keys.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_rule_options/src/use_sorted_keys.rs
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.js
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.js
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.json
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.json
crates/biome_json_analyze/src/assist/source/use_sorted_keys.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs
: Format Rust files before committing (e.g., viajust f
which formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_rule_options/src/use_sorted_keys.rs
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
crates/biome_json_analyze/src/assist/source/use_sorted_keys.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.js
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.js
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.json
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.json
crates/biome_json_analyze/src/assist/source/use_sorted_keys.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.js
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.js
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.json
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.json
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md
: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period
Files:
.changeset/group-by-nesting-feature.md
🧬 Code graph analysis (4)
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.js (1)
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.js (1)
obj
(1-19)
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs (2)
crates/biome_analyze/src/utils.rs (2)
is_separated_list_sorted_by
(13-43)sorted_separated_list_by
(53-107)crates/biome_string_case/src/comparable_token.rs (1)
new
(13-15)
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.js (1)
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.js (1)
obj
(1-19)
crates/biome_json_analyze/src/assist/source/use_sorted_keys.rs (1)
crates/biome_analyze/src/utils.rs (2)
is_separated_list_sorted_by
(13-43)sorted_separated_list_by
(53-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Validate PR title
- GitHub Check: Validate PR title
- GitHub Check: Validate PR title
- GitHub Check: triage
🔇 Additional comments (12)
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json (1)
1-16
: Configuration looks correct.The test configuration properly enables the
useSortedKeys
assist with lexicographic ordering and the newgroupByNesting
option.crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.js (1)
1-19
: Test fixture correctly structured.The test data provides a good mix of nesting levels (primitives, single-line arrays, nested objects, and multi-line arrays) to exercise the groupByNesting feature. Keys are intentionally unsorted as this is an input fixture.
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.json (1)
1-19
: JSON test fixture looks good.The structure correctly mirrors the JavaScript variants, providing consistency across language tests.
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.js (1)
1-19
: Test data structure is sound.Identical to the lexicographic variant, which allows testing both sort orders with the same data structure.
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json (1)
1-16
: Configuration is correct.Properly configured for testing natural sort order with grouping by nesting enabled.
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.json (1)
1-19
: JSON fixture correctly structured.Provides appropriate test data for lexicographic sorting with grouping by nesting.
crates/biome_rule_options/src/use_sorted_keys.rs (1)
10-13
: Documentation is clear and implementation looks good.The documentation concisely explains the grouping behaviour and what constitutes simple vs. nested values. The field is properly integrated into the options struct.
crates/biome_json_analyze/tests/specs/source/useSortedKeys/group-by-nesting-natural.options.json (1)
1-16
: Configuration looks proper.The test configuration correctly enables natural sorting with grouping by nesting for JSON analysis.
crates/biome_js_analyze/tests/specs/source/useSortedKeys/group-by-nesting-lexicographic.options.json (1)
1-16
: Looks good.Config keys match the new option and the test intent. Nothing to change.
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs (1)
1-16
: Imports look tidy.New syntax variants are correctly referenced via
AnyJsObjectMember
/AnyJsExpression
; no extra imports needed for the pattern matches above.crates/biome_json_analyze/src/assist/source/use_sorted_keys.rs (2)
78-99
: Docs are clear and match behaviour.Nicely explains grouping and examples. Parity with JS docs looks good.
140-174
: Comparator wiring and chunking logic look correct.Composite key
(depth, name)
withthen_with
is spot‑on, and you don’t introduce unintended chunk splits. Good job.Also applies to: 204-236
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @PaulRBerg. * #7799 (comment) The following files were modified: * `crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs` * `crates/biome_json_analyze/src/assist/source/use_sorted_keys.rs`
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)
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs (1)
134-152
: Consider enhancing the documentation example.The example demonstrates an unsorted object but doesn't show the expected output after grouping. Consider adding a comment showing how simple values (
id
,name
) would be grouped before nested values (details
), or useexpect_diff
to show the transformation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs
: Format Rust files before committing (e.g., viajust f
which formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs (2)
crates/biome_analyze/src/utils.rs (2)
is_separated_list_sorted_by
(13-43)sorted_separated_list_by
(53-107)crates/biome_string_case/src/comparable_token.rs (1)
new
(13-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: triage
- GitHub Check: Validate PR title
🔇 Additional comments (4)
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs (4)
164-182
: LGTM! Solid nesting depth logic.The implementation correctly classifies expressions by depth. The multiline array detection using
contains_char('\n')
is a pragmatic approach that should work reliably in practice.
184-199
: Excellent—addresses the previous review's regression concern.This implementation correctly treats methods, getters, and setters as sortable with depth 1, preventing them from becoming chunk delimiters. The logic matches the suggested fix from the previous review.
Based on learnings
215-237
: LGTM! Clean conditional sorting logic.The tuple-based comparison
d1.cmp(d2).then_with(|| comparator(n1, n2))
correctly prioritizes depth before name. The conditional structure clearly separates grouped vs ungrouped behaviour.
267-287
: LGTM! Action correctly mirrors the run logic.The sorting behaviour in the action path precisely matches the detection logic in
run()
, ensuring consistency. The separator construction and error handling are appropriate.
Please point this PR to |
Implements a new
groupByNesting
option for theuseSortedKeys
assist that groups object keys by their value's nesting depth before applying alphabetical sorting.Nesting depth classification:
When enabled, simple values are sorted alphabetically first, followed by nested values sorted alphabetically. This addresses the common use case where developers want scalar properties at the top of objects, with complex nested structures appearing at the bottom.
Implementation details:
group_by_nesting: bool
field toUseSortedKeysOptions
get_nesting_depth_js()
for JavaScript AST nodesget_nesting_depth()
for JSON AST nodesConfiguration:
Summary
Discussed in #7331
Test Plan