From 5e972405cbdba60de0653f6a736580390e28d653 Mon Sep 17 00:00:00 2001
From: Vo Hoang Long <longvh@amazon.com>
Date: Sun, 25 Aug 2024 11:03:04 +0900
Subject: [PATCH] refactor(js_analyze): split props to group

Separated by spread props
---
 .../src/assists/source/sort_jsx_props.rs      | 93 +++++++++++--------
 .../source/sortJsxProps/unsorted.jsx.snap     | 10 +-
 2 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/crates/biome_js_analyze/src/assists/source/sort_jsx_props.rs b/crates/biome_js_analyze/src/assists/source/sort_jsx_props.rs
index e5085c58d46f..8bb539b6eb49 100644
--- a/crates/biome_js_analyze/src/assists/source/sort_jsx_props.rs
+++ b/crates/biome_js_analyze/src/assists/source/sort_jsx_props.rs
@@ -1,4 +1,4 @@
-use std::{borrow::Cow, cmp::Ordering};
+use std::{borrow::Cow, cmp::Ordering, iter::zip};
 
 use biome_analyze::{
     context::RuleContext, declare_source_rule, ActionCategory, Ast, Rule, RuleAction, RuleSource,
@@ -6,7 +6,6 @@ use biome_analyze::{
 };
 use biome_console::markup;
 use biome_diagnostics::Applicability;
-use biome_js_factory::make::jsx_attribute_list;
 use biome_js_syntax::{AnyJsxAttribute, JsxAttribute, JsxAttributeList};
 use biome_rowan::{AstNode, BatchMutationExt};
 
@@ -46,51 +45,41 @@ declare_source_rule! {
 
 impl Rule for SortJsxProps {
     type Query = Ast<JsxAttributeList>;
-    type State = ();
-    type Signals = Option<Self::State>;
+    type State = PropGroup;
+    type Signals = Vec<Self::State>;
     type Options = ();
 
-    fn run(_ctx: &RuleContext<Self>) -> Self::Signals {
-        Some(())
-    }
-
-    fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<JsRuleAction> {
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
         let props = ctx.query().clone();
-        let mut non_spread_props = Vec::new();
-        let mut new_props = Vec::new();
+        let mut current_prop_group = PropGroup::default();
+        let mut prop_groups = Vec::new();
         for prop in props.clone() {
             match prop {
                 AnyJsxAttribute::JsxAttribute(attr) => {
-                    non_spread_props.push(attr);
+                    current_prop_group.props.push(PropElement { prop: attr });
                 }
+                // spread prop reset sort order
                 AnyJsxAttribute::JsxSpreadAttribute(_) => {
-                    if !non_spread_props.is_empty() {
-                        non_spread_props.sort_by(compare_props());
-                        new_props.extend(
-                            non_spread_props
-                                .clone()
-                                .into_iter()
-                                .map(AnyJsxAttribute::JsxAttribute),
-                        );
-                    }
-                    non_spread_props.clear();
-                    new_props.push(prop);
+                    prop_groups.push(current_prop_group);
+                    current_prop_group = PropGroup::default();
                 }
             }
         }
-        if !non_spread_props.is_empty() {
-            non_spread_props.sort_by(compare_props());
-            new_props.extend(
-                non_spread_props
-                    .into_iter()
-                    .map(AnyJsxAttribute::JsxAttribute),
-            );
-        }
-        if new_props == props.clone().into_iter().collect::<Vec<_>>() {
+        prop_groups.push(current_prop_group);
+        prop_groups
+    }
+
+    fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
+        if state.is_sorted() {
             return None;
         }
         let mut mutation = ctx.root().begin();
-        mutation.replace_node(props, jsx_attribute_list(new_props));
+
+        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!(),
@@ -101,13 +90,43 @@ impl Rule for SortJsxProps {
     }
 }
 
-fn compare_props() -> impl FnMut(&JsxAttribute, &JsxAttribute) -> Ordering {
-    |a: &JsxAttribute, b: &JsxAttribute| -> Ordering {
-        let (Ok(a_name), Ok(b_name)) = (a.name(), b.name()) else {
+#[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) = (a_name.text(), b_name.text());
+        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
+    }
+}
diff --git a/crates/biome_js_analyze/tests/specs/source/sortJsxProps/unsorted.jsx.snap b/crates/biome_js_analyze/tests/specs/source/sortJsxProps/unsorted.jsx.snap
index 7a02458a8379..335cfe7108cd 100644
--- a/crates/biome_js_analyze/tests/specs/source/sortJsxProps/unsorted.jsx.snap
+++ b/crates/biome_js_analyze/tests/specs/source/sortJsxProps/unsorted.jsx.snap
@@ -22,6 +22,14 @@ expression: unsorted.jsx
 @@ -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} firstName="John" lastName="Smith" />;
++<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" />;
 
 ```