Address comments

Author: Overload119

lib/rules/jsx-sort-props.js

@@ -40,56 +40,49 @@ function isReservedPropName(name, list) {
   return list.indexOf(name) >= 0;
 }
 
-
 function alphabeticalCompare(a, b, ignoreCase) {
   if (ignoreCase) {
     a = a.toLowerCase();
     b = b.toLowerCase();
   }
-
-  if (a < b) {
-    return -1;
-  } else if (a > b) {
-    return 1;
-  }
-
-  return 0;
+  return a.localeCompare(b);
 }
 
 function generateFixerFunction(node, context) {
   var sourceCode = context.getSourceCode();
-  var sortedAttributes = node.attributes.slice(0);
+  var attributes = node.attributes.slice(0);
   var configuration = context.options[0] || {};
   var ignoreCase = configuration.ignoreCase || false;
 
   // Sort props according to the context. Only supports ignoreCase.
-  sortedAttributes.sort(function(a, b) {
-    // Put spread attributes at the end.
-    if (a.type === 'JSXSpreadAttribute' && b.type !== 'JSXSpreadAttribute') {
-      return 1;
-    } else if (a.type !== 'JSXSpreadAttribute' && b.type === 'JSXSpreadAttribute') {
-      return -1;
-    } else if (a.type === 'JSXSpreadAttribute' && b.type === 'JSXSpreadAttribute') {
-      return alphabeticalCompare(
-        sourceCode.getText(a),
-        sourceCode.getText(b),
-        ignoreCase
-      );
-    }
-
-    var propNameA = propName(a);
-    var propNameB = propName(b);
+  // Since we cannot safely move JSXSpreadAttributes (due to potential variable overrides),
+  // we only consider the sortable attributes.
+  const sortableAttributes = attributes.filter(function(attr) {
+    return attr.type !== 'JSXSpreadAttribute';
+  });
 
-    return alphabeticalCompare(propNameA, propNameB, ignoreCase);
+  sortableAttributes.sort(function(a, b) {
+    return alphabeticalCompare(propName(a), propName(b), ignoreCase);
   });
 
   return function(fixer) {
     // Replace each unsorted attribute with the sorted one.
-    return node.attributes.map(function(attr, index) {
-      var sortedAttr = sortedAttributes[index];
+    const fixers = [];
+    let skipIndex = 0; // skip over spread attributes
+
+    node.attributes.forEach(function(attr, index) {
+      if (attr.type === 'JSXSpreadAttribute') {
+        skipIndex++;
+        return;
+      }
+
+      var sortedAttr = sortableAttributes[index - skipIndex];
       var sortedAttrText = sourceCode.getText(sortedAttr);
-      return fixer.replaceTextRange([attr.start, attr.end], sortedAttrText);
+      fixers.push(
+        fixer.replaceTextRange([attr.start, attr.end], sortedAttrText)
+      );
     });
+    return fixers;
   };
 }
 

tests/lib/rules/jsx-sort-props.js

@@ -161,17 +161,17 @@ ruleTester.run('jsx-sort-props', rule, {
     {
       code: '<App {...this.props} b a />;',
       errors: [expectedError],
-      output: '<App a b {...this.props} />;'
+      output: '<App {...this.props} a b />;'
     },
     {
       code: '<App c {...this.props} b a />;',
       errors: [expectedError],
-      output: '<App a b c {...this.props} />;'
+      output: '<App a {...this.props} b c />;'
     },
     {
       code: '<App a A />;',
       errors: [expectedError],
-      output: '<App A a />;'
+      output: '<App a A />;'
     },
     {
       code: '<App B a />;',
@@ -192,12 +192,12 @@ ruleTester.run('jsx-sort-props', rule, {
     },
     {
       code: '<App {...this.props} c="a" a="c" b="b" />;',
-      output: '<App a="c" b="b" c="a" {...this.props} />;',
+      output: '<App {...this.props} a="c" b="b" c="a" />;',
       errors: 2
     },
     {
       code: '<App d="d" b="b" {...this.props} c="a" a="c" />;',
-      output: '<App a="c" b="b" c="a" d="d" {...this.props} />;',
+      output: '<App a="c" b="b" {...this.props} c="a" d="d" />;',
       errors: 2
     },
     {
@@ -221,9 +221,9 @@ ruleTester.run('jsx-sort-props', rule, {
         'b={false}',
         'onHandle={function(){}}',
         'r',
+        '{...this.props}',
         'z',
-        '{...otherProps}',
-        '{...this.props}>',
+        '{...otherProps}>',
         '  {test}',
         '</App>'
       ].join('\n'),