feat(graphql_analyze): implement useSortedSelectionSet#9853
Merged
Netail merged 3 commits intobiomejs:nextfrom Apr 11, 2026
Merged
feat(graphql_analyze): implement useSortedSelectionSet#9853Netail merged 3 commits intobiomejs:nextfrom
Netail merged 3 commits intobiomejs:nextfrom
Conversation
🦋 Changeset detectedLatest commit: dc24c3a 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 |
6 tasks
Merging this PR will not alter performance
Comparing Footnotes
|
Contributor
WalkthroughAdds a new GraphQL assist action Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/new-bikes-buy.md:
- Around line 1-3: The changeset currently lists "@biomejs/biome": patch which
understates the release; update the changeset to use "minor" instead of "patch"
for the "@biomejs/biome" entry so the new user-facing assist on next is released
as a minor version.
In `@crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs`:
- Around line 218-275: Add a test fixture pair under
tests/specs/source/useSortedSelectionSet/ that exercises the underscore, digit,
and mixed-case branches of the comparator so regressions in
locale_compare/compare_primary/primary_char_key/compare_tertiary are caught:
create one "invalid" input where fields are intentionally ordered to trigger
ordering differences between '_' vs digits vs letters and mixed-case
tie-breakers (e.g., names with leading '_' , names starting with digits, and
same letters with differing case), and a corresponding "valid" file with the
correct order per the comparator; ensure the spec asserts the rule flags the
invalid file and accepts the valid one.
- Around line 45-52: The rule metadata declares UseSortedSelectionSet with
FixKind::Safe but reordering selection fields is observable under GraphQL, so
change the metadata on UseSortedSelectionSet from FixKind::Safe to
FixKind::Unsafe; also add/expand unit tests for the comparator logic (exercise
underscores, digits, and mixed-case tie-breakers) to the existing test suite
that validates the selection ordering comparator to ensure those cases are
covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a6077ffb-fbdd-403d-aa3d-23a866574972
⛔ Files ignored due to path filters (5)
crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/invalid.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/valid.graphql.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
.changeset/new-bikes-buy.mdcrates/biome_configuration/src/analyzer/assist/actions.rscrates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rscrates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/valid.graphqlcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_sorted_selection_set.rs
37e0ee4 to
810c10b
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs (1)
100-102: Tiny tidy-up: inline the one-line comparator helper.
comparatorcurrently just forwards toComparableToken::ascii_nat_cmp; inlining would reduce indirection without changing behaviour.♻️ Minimal refactor
-fn comparator(a: &ComparableToken, b: &ComparableToken) -> std::cmp::Ordering { - ComparableToken::ascii_nat_cmp(a, b) -} @@ - comparator(&a, &b) + ComparableToken::ascii_nat_cmp(&a, &b)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs` around lines 100 - 102, The helper function comparator that simply calls ComparableToken::ascii_nat_cmp should be inlined: replace uses of comparator (e.g., sort_by(comparator) or any place passing comparator) with a direct closure or function pointer calling ComparableToken::ascii_nat_cmp (for example sort_by(|a, b| ComparableToken::ascii_nat_cmp(a, b))) and then remove the now-unused fn comparator definition to eliminate the trivial indirection while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs`:
- Around line 100-102: The helper function comparator that simply calls
ComparableToken::ascii_nat_cmp should be inlined: replace uses of comparator
(e.g., sort_by(comparator) or any place passing comparator) with a direct
closure or function pointer calling ComparableToken::ascii_nat_cmp (for example
sort_by(|a, b| ComparableToken::ascii_nat_cmp(a, b))) and then remove the
now-unused fn comparator definition to eliminate the trivial indirection while
preserving behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a83176d-5da0-4db3-8de8-23390fe94265
⛔ Files ignored due to path filters (5)
crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/invalid.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/valid.graphql.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
.changeset/new-bikes-buy.mdcrates/biome_configuration/src/analyzer/assist/actions.rscrates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rscrates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/valid.graphqlcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_sorted_selection_set.rs
✅ Files skipped from review due to trivial changes (3)
- .changeset/new-bikes-buy.md
- crates/biome_rule_options/src/lib.rs
- crates/biome_rule_options/src/use_sorted_selection_set.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/invalid.graphql
- crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/valid.graphql
- crates/biome_configuration/src/analyzer/assist/actions.rs
810c10b to
ee4154c
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs (1)
106-109: Tiny naming nit:fieldsis actually a selection list.This reads cleaner if
fieldsis renamed toselections(future-you will thank present-you).💡 Suggested rename
- let fields = node.selections(); + let selections = node.selections(); let mut mutation = ctx.root().begin(); - sort_selection_list(&fields, &mut mutation)?; + sort_selection_list(&selections, &mut mutation)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs` around lines 106 - 109, In the action function (fn action) the local variable named fields actually holds node.selections(); rename fields to selections to make intent clear: change the declaration let fields = node.selections() to let selections = node.selections() and update all subsequent uses in the function (and any lambda/closure if present) to reference selections instead of fields (e.g., where fields.iter() or fields.len() might be used); this is purely a local rename inside action and requires no API changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs`:
- Around line 106-109: In the action function (fn action) the local variable
named fields actually holds node.selections(); rename fields to selections to
make intent clear: change the declaration let fields = node.selections() to let
selections = node.selections() and update all subsequent uses in the function
(and any lambda/closure if present) to reference selections instead of fields
(e.g., where fields.iter() or fields.len() might be used); this is purely a
local rename inside action and requires no API changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 920a7242-f24b-4ffd-9b76-a1d0ed0d433a
⛔ Files ignored due to path filters (5)
crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/invalid.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/valid.graphql.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
.changeset/new-bikes-buy.mdcrates/biome_configuration/src/analyzer/assist/actions.rscrates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rscrates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/valid.graphqlcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_sorted_selection_set.rs
✅ Files skipped from review due to trivial changes (6)
- .changeset/new-bikes-buy.md
- crates/biome_rule_options/src/lib.rs
- crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/valid.graphql
- crates/biome_graphql_analyze/tests/specs/source/useSortedSelectionSet/invalid.graphql
- crates/biome_configuration/src/analyzer/assist/actions.rs
- crates/biome_rule_options/src/use_sorted_selection_set.rs
dyc3
approved these changes
Apr 10, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs (1)
106-119: Nitpick: Variable naming.Line 108 uses
fieldsbut it holds all selections (fields, spreads, and fragments). Consider renaming toselectionsfor consistency with the rest of the file.♻️ Suggested fix
fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<GraphqlRuleAction> { let node = ctx.query(); - let fields = node.selections(); + let selections = node.selections(); let mut mutation = ctx.root().begin(); - sort_selection_list(&fields, &mut mutation)?; + sort_selection_list(&selections, &mut mutation)?; Some(RuleAction::new(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs` around lines 106 - 119, In the action function, the local variable named `fields` actually holds all selection items (fields, spreads, fragments); rename `fields` to `selections` to match naming used elsewhere and improve clarity. Update the declaration that assigns `node.selections()` and any subsequent usages (e.g., the call to `sort_selection_list(&fields, &mut mutation)?;`) to use `selections`, ensuring the function `action`, the `node.selections()` call, and the `sort_selection_list` invocation are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs`:
- Around line 106-119: In the action function, the local variable named `fields`
actually holds all selection items (fields, spreads, fragments); rename `fields`
to `selections` to match naming used elsewhere and improve clarity. Update the
declaration that assigns `node.selections()` and any subsequent usages (e.g.,
the call to `sort_selection_list(&fields, &mut mutation)?;`) to use
`selections`, ensuring the function `action`, the `node.selections()` call, and
the `sort_selection_list` invocation are updated accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: baddf7ef-0789-4612-b529-a3e964e28e3d
⛔ Files ignored due to path filters (2)
crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (1)
crates/biome_graphql_analyze/src/assist/source/use_sorted_selection_set.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement SelectionSet sorting for graphql, based on Eslint Graphql's alphabetize
Related #9137
Test Plan
Unit tests
Docs