feat(assist): use sorted type fields 9137#9275
Conversation
🦋 Changeset detectedLatest commit: f91741a 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 GraphQL assist action called 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)
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.
🧹 Nitpick comments (1)
crates/biome_graphql_analyze/src/assist/source/use_sorted_type_fields.rs (1)
79-131: Consider extracting common pattern.The six match arms follow identical structure. A helper method could reduce duplication, though current readability is fine.
♻️ Optional: Extract helper for common pattern
fn check_fields<T, F>(fields: Option<T>, check_sorted: F) -> Option<UseSortedTypeFieldsState> where F: FnOnce(&T) -> bool, T: Into<UseSortedTypeFieldsState>, { let fields = fields?; if check_sorted(&fields) { None } else { Some(fields.into()) } }This would require implementing
Into<UseSortedTypeFieldsState>for the field types, which may be more boilerplate than it's worth.🤖 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_type_fields.rs` around lines 79 - 131, The six match arms in run (handling UseSortedTypeFieldsQuery::{GraphqlObjectTypeDefinition, GraphqlObjectTypeExtension, GraphqlInterfaceTypeDefinition, GraphqlInterfaceTypeExtension, GraphqlInputObjectTypeDefinition, GraphqlInputObjectTypeExtension}) duplicate the same pattern; extract a helper like check_fields that takes the optional fields, a predicate (is_field_definition_list_sorted or is_input_field_list_sorted) and a closure or an Into/From conversion to produce the appropriate UseSortedTypeFieldsState (UseSortedTypeFieldsState::TypeFields or ::InputFields) so each arm reduces to calling check_fields(fields_option, predicate, state_mapper). Ensure helper handles the Option early-return (fields?) and returns Some(state) or None matching current behavior; reference functions is_field_definition_list_sorted, is_input_field_list_sorted and enum UseSortedTypeFieldsState for integration.
🤖 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_type_fields.rs`:
- Around line 79-131: The six match arms in run (handling
UseSortedTypeFieldsQuery::{GraphqlObjectTypeDefinition,
GraphqlObjectTypeExtension, GraphqlInterfaceTypeDefinition,
GraphqlInterfaceTypeExtension, GraphqlInputObjectTypeDefinition,
GraphqlInputObjectTypeExtension}) duplicate the same pattern; extract a helper
like check_fields that takes the optional fields, a predicate
(is_field_definition_list_sorted or is_input_field_list_sorted) and a closure or
an Into/From conversion to produce the appropriate UseSortedTypeFieldsState
(UseSortedTypeFieldsState::TypeFields or ::InputFields) so each arm reduces to
calling check_fields(fields_option, predicate, state_mapper). Ensure helper
handles the Option early-return (fields?) and returns Some(state) or None
matching current behavior; reference functions is_field_definition_list_sorted,
is_input_field_list_sorted and enum UseSortedTypeFieldsState for integration.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_graphql_analyze/tests/specs/source/useSortedTypeFields/invalid.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_analyze/tests/specs/source/useSortedTypeFields/valid.graphql.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (13)
.changeset/afraid-humans-worry.mdcrates/biome_configuration/src/analyzer/assist/actions.rscrates/biome_graphql_analyze/build.rscrates/biome_graphql_analyze/src/assist.rscrates/biome_graphql_analyze/src/assist/source.rscrates/biome_graphql_analyze/src/assist/source/use_sorted_type_fields.rscrates/biome_graphql_analyze/src/lib.rscrates/biome_graphql_analyze/src/registry.rscrates/biome_graphql_analyze/tests/specs/source/useSortedTypeFields/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/source/useSortedTypeFields/valid.graphqlcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_sorted_type_fields.rsxtask/codegen/src/generate_analyzer.rs
Merging this PR will not alter performance
Comparing Footnotes
|
Summary
Implements the
useSortedTypeFieldsassist action for GraphQL as part of #9137.This action alphabetically sorts fields of:
ObjectTypeDefinitionInterfaceTypeDefinitionInputObjectTypeDefinitionThe ordering matches JavaScript’s
localeCompare()behavior for GraphQL identifiers ([_A-Za-z0-9]):_< digits < letters (case-insensitive)aFieldbeforeAField)This PR also introduces the assist category infrastructure for
biome_graphql_analyze, which previously only supported lint rules.Why this is implemented as an assist
Sorting fields is a non-semantic, style-only transformation with a clear and safe auto-fix. It does not indicate a bug or problematic pattern — it only improves structural consistency and readability.
For that reason, I implemented it as an assist action rather than a lint rule. If this classification is incorrect (for example, if it should follow thenursery rule policy or be modeled as a lint rule instead), please let me know. I’m happy to adjust the implementation accordingly.
Branch Target
This PR currently targets
next, since adding a new assist action is a user-facing feature (minor change).If new assist actions are expected to target
maininstead (e.g. if they follow the same release policy as nursery rules), I’m happy to retarget the branch.Test Plan
Snapshot tests cover:
ObjectTypeDefinitionInterfaceTypeDefinitionInputObjectTypeDefinitionObjectTypeExtensionInterfaceTypeExtensionInputObjectTypeExtensionapplebeforeZoo)aFieldbeforeAField)localeComparecharacter class ordering (a_beforea1)Documentation
Inline rustdoc documentation is provided in
use_sorted_type_fields.rs,including
expect_diffexamples demonstrating the before/after transformation.