Skip to content

📝 Add docstrings to feat/use-sorted-keys-group-by-nesting #7800

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.

Sign up for GitHub

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

Closed
coderabbitai wants to merge 1 commit into from
Closed

📝 Add docstrings to feat/use-sorted-keys-group-by-nesting #7800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 171 additions & 17 deletions crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use biome_console::markup;
use biome_deserialize::TextRange;
use biome_diagnostics::{Applicability, category};
use biome_js_factory::make;
use biome_js_syntax::{JsObjectExpression, JsObjectMemberList, T};
use biome_js_syntax::{
AnyJsExpression, AnyJsObjectMember, JsObjectExpression, JsObjectMemberList, T,
};
use biome_rowan::{AstNode, BatchMutationExt, TriviaPieceKind};
use biome_rule_options::use_sorted_keys::{SortOrder, UseSortedKeysOptions};
use biome_string_case::comparable_token::ComparableToken;
Expand Down Expand Up @@ -129,6 +131,26 @@ declare_source_rule! {
/// };
/// ```
///
/// ### `groupByNesting`
/// When enabled, groups object keys by their value's nesting depth before sorting alphabetically.
///
/// ```json,options
/// {
/// "options": {
/// "groupByNesting": true
/// }
/// }
/// ```
/// ```js,use_options,expect_diagnostic
/// const obj = {
/// name: "Sample",
/// details: {
/// description: "nested"
/// },
/// id: "123"
/// };
/// ```
///
pub UseSortedKeys {
version: "2.0.0",
name: "useSortedKeys",
Expand All @@ -139,12 +161,78 @@ declare_source_rule! {
}
}

/// Compute a simple nesting depth indicator for a JavaScript expression used when grouping object members.
///
/// The function returns `1` for object expressions and for array expressions that span multiple lines;
/// it returns `0` for single-line arrays and all other expression kinds.
///
/// # Examples
///
/// ```rust,ignore
/// // object literal -> depth 1
/// let obj_expr: AnyJsExpression = parse_expression("({ a: 1 })");
/// assert_eq!(get_nesting_depth_js(&obj_expr), 1);
///
/// // single-line array -> depth 0
/// let arr_expr: AnyJsExpression = parse_expression("[1, 2, 3]");
/// assert_eq!(get_nesting_depth_js(&arr_expr), 0);
///
/// // multi-line array -> depth 1
/// let multi_arr_expr: AnyJsExpression = parse_expression("[\n 1,\n 2\n]");
/// assert_eq!(get_nesting_depth_js(&multi_arr_expr), 1);
/// ```
///
/// # Returns
///
/// `1` for object expressions and for arrays that contain a newline (span multiple lines), `0` otherwise.
fn get_nesting_depth_js(value: &AnyJsExpression) -> u8 {
match value {
AnyJsExpression::JsObjectExpression(_) => 1,
AnyJsExpression::JsArrayExpression(array) => {
// Check if array spans multiple lines by looking for newlines
if array.syntax().text_trimmed().contains_char('\n') {
1
} else {
0
}
}
_ => 0,
}
}

/// Extracts the value expression from an object member
fn get_member_value(node: &AnyJsObjectMember) -> Option<AnyJsExpression> {
match node {
AnyJsObjectMember::JsPropertyObjectMember(prop) => prop.value().ok(),
_ => None, // Getters, setters, methods, etc. treated as non-nested
}
}

impl Rule for UseSortedKeys {
type Query = Ast<JsObjectMemberList>;
type State = ();
type Signals = Option<Self::State>;
type Options = UseSortedKeysOptions;

/// Determines whether the object member list in the query is sorted according to the rule options.
///
/// When `group_by_nesting` is enabled, members are compared by a pair (nesting depth, property name),
/// where nesting depth is computed from the member's value and property name is the member key token;
/// otherwise members are compared by property name alone. The comparator is chosen from `sort_order`
/// (natural or lexicographic).
///
/// # Returns
///
/// `Some(())` if the members are not sorted according to the configured options, `None` otherwise.
///
/// # Examples
///
/// ```no_run
/// # use crate::{RuleContext, UseSortedKeys};
/// # fn example(ctx: &RuleContext<UseSortedKeys>) {
/// let signal = UseSortedKeys::run(ctx);
/// # }
/// ```
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let options = ctx.options();
let sort_order = options.sort_order;
Expand All @@ -153,16 +241,50 @@ impl Rule for UseSortedKeys {
SortOrder::Lexicographic => ComparableToken::lexicographic_cmp,
};

is_separated_list_sorted_by(
ctx.query(),
|node| node.name().map(ComparableToken::new),
comparator,
)
.ok()?
.not()
.then_some(())
if options.group_by_nesting {
is_separated_list_sorted_by(
ctx.query(),
|node| {
let value = get_member_value(node)?;
let depth = get_nesting_depth_js(&value);
let name = node.name().map(ComparableToken::new)?;
Some((depth, name))
},
|(d1, n1), (d2, n2)| d1.cmp(d2).then_with(|| comparator(n1, n2)),
)
.ok()?
.not()
.then_some(())
} else {
is_separated_list_sorted_by(
ctx.query(),
|node| node.name().map(ComparableToken::new),
comparator,
)
.ok()?
.not()
.then_some(())
}
}

/// Produces a diagnostic that marks an object whose properties are not sorted by key.
///
/// The diagnostic uses the category `assist/source/useSortedKeys` and targets the query range
/// with the message "The object properties are not sorted by key."
///
/// # Examples
///
/// ```
/// // In a rule implementation, `diagnostic(ctx, &state)` is used to create the diagnostic
/// // shown below. `ctx` is provided by the rule runner.
/// use biome_diagnostics::RuleDiagnostic;
///
/// let expected = RuleDiagnostic::new(
/// category!("assist/source/useSortedKeys"),
/// /* range */ Default::default(),
/// "The object properties are not sorted by key.",
/// );
/// ```
fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
Some(RuleDiagnostic::new(
category!("assist/source/useSortedKeys"),
Expand All @@ -181,6 +303,23 @@ impl Rule for UseSortedKeys {
.map(|object| object.range())
}

/// Builds a rule action that replaces the object property list with a sorted version according to the rule options.
///
/// When `group_by_nesting` is enabled the list is sorted by a tuple of (nesting depth of the member's value, property key);
/// otherwise the list is sorted by property key only. The sorting uses either natural or lexicographic ordering depending
/// on the configured `sort_order`. If sorting or list construction fails, the function returns `None`.
///
/// # Returns
///
/// `Some(JsRuleAction)` containing a mutation that replaces the original object member list with the sorted list if sorting
/// succeeded, `None` otherwise.
///
/// # Examples
///
/// ```
/// // Given a rule context `ctx` and state `state`, produce the optional fix action:
/// let action = action(&ctx, &state);
/// ```
fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
let list = ctx.query();
let options = ctx.options();
Expand All @@ -190,13 +329,28 @@ impl Rule for UseSortedKeys {
SortOrder::Lexicographic => ComparableToken::lexicographic_cmp,
};

let new_list = sorted_separated_list_by(
list,
|node| node.name().map(ComparableToken::new),
|| make::token(T![,]).with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]),
comparator,
)
.ok()?;
let new_list = if options.group_by_nesting {
sorted_separated_list_by(
list,
|node| {
let value = get_member_value(node)?;
let depth = get_nesting_depth_js(&value);
let name = node.name().map(ComparableToken::new)?;
Some((depth, name))
},
|| make::token(T![,]).with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]),
|(d1, n1), (d2, n2)| d1.cmp(d2).then_with(|| comparator(n1, n2)),
)
.ok()?
} else {
sorted_separated_list_by(
list,
|node| node.name().map(ComparableToken::new),
|| make::token(T![,]).with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]),
comparator,
)
.ok()?
};

let mut mutation = ctx.root().begin();
mutation.replace_node_discard_trivia(list.clone(), new_list);
Expand All @@ -208,4 +362,4 @@ impl Rule for UseSortedKeys {
mutation,
))
}
}
}
Loading
Loading