Skip to content
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

feat(biome_js_analyzer): useJsxSortProps #2652

Merged
merged 6 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 17 additions & 3 deletions crates/biome_configuration/src/analyzer/assists/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,29 +99,42 @@ impl Actions {
#[serde(rename_all = "camelCase", default, deny_unknown_fields)]
#[doc = r" A list of rules that belong to this group"]
pub struct Source {
#[doc = "Enforce props sorting in JSX elements."]
#[serde(skip_serializing_if = "Option::is_none")]
pub sort_jsx_props: Option<RuleAssistConfiguration>,
#[doc = "Sorts the keys of a JSON object in natural order"]
#[serde(skip_serializing_if = "Option::is_none")]
pub use_sorted_keys: Option<RuleAssistConfiguration>,
}
impl Source {
const GROUP_NAME: &'static str = "source";
pub(crate) const GROUP_RULES: &'static [&'static str] = &["useSortedKeys"];
pub(crate) const GROUP_RULES: &'static [&'static str] = &["sortJsxProps", "useSortedKeys"];
pub(crate) fn get_enabled_rules(&self) -> FxHashSet<RuleFilter<'static>> {
let mut index_set = FxHashSet::default();
if let Some(rule) = self.use_sorted_keys.as_ref() {
if let Some(rule) = self.sort_jsx_props.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]));
}
}
if let Some(rule) = self.use_sorted_keys.as_ref() {
if rule.is_enabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]));
}
}
index_set
}
pub(crate) fn get_disabled_rules(&self) -> FxHashSet<RuleFilter<'static>> {
let mut index_set = FxHashSet::default();
if let Some(rule) = self.use_sorted_keys.as_ref() {
if let Some(rule) = self.sort_jsx_props.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]));
}
}
if let Some(rule) = self.use_sorted_keys.as_ref() {
if rule.is_disabled() {
index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]));
}
}
index_set
}
#[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"]
Expand All @@ -133,6 +146,7 @@ impl Source {
rule_name: &str,
) -> Option<RuleAssistConfiguration> {
match rule_name {
"sortJsxProps" => self.sort_jsx_props.as_ref().copied(),
"useSortedKeys" => self.use_sorted_keys.as_ref().copied(),
_ => None,
}
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/assists/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
use biome_analyze::declare_assists_group;

pub mod organize_imports;
pub mod sort_jsx_props;

declare_assists_group! {
pub Source {
name : "source" ,
rules : [
self :: organize_imports :: OrganizeImports ,
self :: sort_jsx_props :: SortJsxProps ,
]
}
}
138 changes: 138 additions & 0 deletions crates/biome_js_analyze/src/assists/source/sort_jsx_props.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
use std::{borrow::Cow, cmp::Ordering, iter::zip};

use biome_analyze::{
context::RuleContext, declare_source_rule, ActionCategory, Ast, Rule, RuleAction, RuleSource,
RuleSourceKind, SourceActionKind,
};
use biome_console::markup;
use biome_diagnostics::Applicability;
use biome_js_syntax::{AnyJsxAttribute, JsxAttribute, JsxAttributeList};
use biome_rowan::{AstNode, BatchMutationExt};

use crate::JsRuleAction;

declare_source_rule! {
/// Enforce props sorting in JSX elements.
///
/// This rule checks if the JSX props are sorted in a consistent way.
/// Props are sorted alphabetically.
/// This rule will not consider spread props as sortable.
/// Instead, whenever it encounters a spread prop, it will sort all the
/// previous non spread props up until the nearest spread prop, if one
/// exist.
/// This prevents breaking the override of certain props using spread
/// props.
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// <Hello lastName="Smith" firstName="John" />;
/// ```
Comment on lines +30 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It definitely makes sense to add an example with spread props, so users can understand how it works. Maybe two spread props should be enough

///
/// ### Valid
///
/// ```js
/// <Hello firstName="John" lastName="Smith" />;
/// <Hello lastName="Smith" {...this.props} firstName="John" />;
/// <Hello tel={5555555} {...this.props} firstName="John" {...another.props} lastName="Smith" />;
/// ```
///
pub SortJsxProps {
version: "next",
name: "sortJsxProps",
language: "js",
recommended: false,
sources: &[RuleSource::EslintReact("jsx-sort-props")],
source_kind: RuleSourceKind::SameLogic,
}
}

impl Rule for SortJsxProps {
type Query = Ast<JsxAttributeList>;
type State = PropGroup;
type Signals = Vec<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let props = ctx.query().clone();
let mut current_prop_group = PropGroup::default();
let mut prop_groups = Vec::new();
for prop in props.clone() {
match prop {
AnyJsxAttribute::JsxAttribute(attr) => {
current_prop_group.props.push(PropElement { prop: attr });
}
// spread prop reset sort order
AnyJsxAttribute::JsxSpreadAttribute(_) => {
prop_groups.push(current_prop_group);
current_prop_group = PropGroup::default();
}
}
}
prop_groups.push(current_prop_group);
prop_groups
Comment on lines +59 to +75
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really looks cleaner after being split to groups

}

fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
if state.is_sorted() {
return None;
}
let mut mutation = ctx.root().begin();

for (PropElement { prop }, PropElement { prop: sorted_prop }) in
zip(state.props.clone(), state.get_sorted_props())
{
mutation.replace_node(prop, sorted_prop);
}

Some(RuleAction::new(
rule_action_category!(),
Applicability::Always,
markup! { "Sort the JSX props." },
mutation,
))
}
}

#[derive(PartialEq, Eq, Clone)]
pub struct PropElement {
prop: JsxAttribute,
}

impl Ord for PropElement {
fn cmp(&self, other: &Self) -> Ordering {
let (Ok(self_name), Ok(other_name)) = (self.prop.name(), other.prop.name()) else {
return Ordering::Equal;
};
let (a_name, b_name) = (self_name.text(), other_name.text());

a_name.cmp(&b_name)
}
}

impl PartialOrd for PropElement {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}

#[derive(Clone, Default)]
pub struct PropGroup {
props: Vec<PropElement>,
}

impl PropGroup {
fn is_sorted(&self) -> bool {
let mut new_props = self.props.clone();
new_props.sort();
new_props == self.props
}

fn get_sorted_props(&self) -> Vec<PropElement> {
let mut new_props = self.props.clone();
new_props.sort();
new_props
}
}
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/options.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<Hello firstName="John" lastName="Smith" />;
<Hello tel={5555555} {...this.props} firstName="John" lastName="Smith" />;
<Hello onClick={this._handleClick} tel={5555555} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: sorted.jsx
---
# Input
```jsx
<Hello firstName="John" lastName="Smith" />;
<Hello tel={5555555} {...this.props} firstName="John" lastName="Smith" />;
<Hello onClick={this._handleClick} tel={5555555} />

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<Hello lastName="Smith" firstName="John" />;
<Hello tel={5555555} address="NY" {...this.props} lastName="Smith" firstName="John" />;
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: unsorted.jsx
---
# Input
```jsx
<Hello lastName="Smith" firstName="John" />;
<Hello tel={5555555} address="NY" {...this.props} lastName="Smith" firstName="John" />;

```

# Actions
```diff
@@ -1,2 +1,2 @@
-<Hello lastName="Smith" firstName="John" />;
+<Hello firstName="John" lastName="Smith" />;
<Hello tel={5555555} address="NY" {...this.props} lastName="Smith" firstName="John" />;

```

```diff
@@ -1,2 +1,2 @@
<Hello lastName="Smith" firstName="John" />;
-<Hello tel={5555555} address="NY" {...this.props} lastName="Smith" firstName="John" />;
+<Hello address="NY" tel={5555555} {...this.props} lastName="Smith" firstName="John" />;

```

```diff
@@ -1,2 +1,2 @@
<Hello lastName="Smith" firstName="John" />;
-<Hello tel={5555555} address="NY" {...this.props} lastName="Smith" firstName="John" />;
+<Hello tel={5555555} address="NY" {...this.props} firstName="John" lastName="Smith" />;

```
4 changes: 4 additions & 0 deletions packages/@biomejs/backend-jsonrpc/src/workspace.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions packages/@biomejs/biome/configuration_schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading