From 279c85019a9459f17a8334376fafbe2072661f05 Mon Sep 17 00:00:00 2001
From: George Guliashvili <george.giorgi.guliashvili@gmail.com>
Date: Thu, 25 Apr 2019 00:31:31 +0100
Subject: [PATCH 1/2] Make prop sort lints fixable

Not all the prop sort lints were fixable.
There were cases when fix would mess other things up.
For example, in case when normal props were not sorted but callback functions were in the end(As they are supposed to be), sorting lint fix would mix oncallback props in between the other props.

Updated the tests accordingly.
---
 lib/rules/jsx-sort-props.js       | 69 ++++++++++++++++++++++++-------
 tests/lib/rules/jsx-sort-props.js | 46 ++++++++++++++-------
 2 files changed, 87 insertions(+), 28 deletions(-)

diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js
index 7528c1905c..5255093ae6 100644
--- a/lib/rules/jsx-sort-props.js
+++ b/lib/rules/jsx-sort-props.js
@@ -27,22 +27,54 @@ function isReservedPropName(name, list) {
   return list.indexOf(name) >= 0;
 }
 
-function propNameCompare(a, b, options) {
-  if (options.ignoreCase) {
-    a = a.toLowerCase();
-    b = b.toLowerCase();
-  }
+function contextCompare(a, b, options) {
+  let aProp = propName(a);
+  let bProp = propName(b);
+
   if (options.reservedFirst) {
-    const aIsReserved = isReservedPropName(a, options.reservedList);
-    const bIsReserved = isReservedPropName(b, options.reservedList);
+    const aIsReserved = isReservedPropName(aProp, options.reservedList);
+    const bIsReserved = isReservedPropName(bProp, options.reservedList);
     if ((aIsReserved && bIsReserved) || (!aIsReserved && !bIsReserved)) {
-      return a.localeCompare(b);
+      // pass
     } else if (aIsReserved && !bIsReserved) {
       return -1;
+    } else {
+      return 1;
+    }
+  }
+
+  if (options.callbacksLast) {
+    const aIsCallback = isCallbackPropName(aProp);
+    const bIsCallback = isCallbackPropName(bProp);
+    if ((aIsCallback && bIsCallback) || (!aIsCallback && !bIsCallback)) {
+      // pass
+    } else if (aIsCallback && !bIsCallback) {
+      return 1;
+    } else {
+      return -1;
+    }
+  }
+
+  if (options.shorthandFirst || options.shorthandLast) {
+    const shorthandSign = options.shorthandFirst ? -1 : 1;
+    if (!a.value && !b.value) {
+      // pass
+    } else if (!a.value) {
+      return shorthandSign;
+    } else {
+      return -shorthandSign;
     }
-    return 1;
   }
-  return a.localeCompare(b);
+
+  if (options.noSortAlphabetically) {
+    return 0;
+  }
+
+  if (options.ignoreCase) {
+    aProp = aProp.toLowerCase();
+    bProp = bProp.toLowerCase();
+  }
+  return aProp.localeCompare(bProp);
 }
 
 /**
@@ -79,15 +111,21 @@ const generateFixerFunction = (node, context, reservedList) => {
   const attributes = node.attributes.slice(0);
   const configuration = context.options[0] || {};
   const ignoreCase = configuration.ignoreCase || false;
+  const callbacksLast = configuration.callbacksLast || false;
+  const shorthandFirst = configuration.shorthandFirst || false;
+  const shorthandLast = configuration.shorthandLast || false;
+  const noSortAlphabetically = configuration.noSortAlphabetically || false;
   const reservedFirst = configuration.reservedFirst || false;
 
   // Sort props according to the context. Only supports ignoreCase.
   // Since we cannot safely move JSXSpreadAttribute (due to potential variable overrides),
   // we only consider groups of sortable attributes.
+  const options = {ignoreCase, callbacksLast, shorthandFirst, shorthandLast,
+    noSortAlphabetically, reservedFirst, reservedList};
   const sortableAttributeGroups = getGroupsOfSortableAttributes(attributes);
   const sortedAttributeGroups = sortableAttributeGroups.slice(0).map(group =>
     group.slice(0).sort((a, b) =>
-      propNameCompare(propName(a), propName(b), {ignoreCase, reservedFirst, reservedList})
+      contextCompare(a, b, options)
     )
   );
 
@@ -267,7 +305,8 @@ module.exports = {
               // Encountered a non-callback prop after a callback prop
               context.report({
                 node: memo,
-                message: 'Callbacks must be listed after all other props'
+                message: 'Callbacks must be listed after all other props',
+                fix: generateFixerFunction(node, context, reservedList)
               });
               return memo;
             }
@@ -280,7 +319,8 @@ module.exports = {
             if (!currentValue && previousValue) {
               context.report({
                 node: memo,
-                message: 'Shorthand props must be listed before all other props'
+                message: 'Shorthand props must be listed before all other props',
+                fix: generateFixerFunction(node, context, reservedList)
               });
               return memo;
             }
@@ -293,7 +333,8 @@ module.exports = {
             if (currentValue && !previousValue) {
               context.report({
                 node: memo,
-                message: 'Shorthand props must be listed after all other props'
+                message: 'Shorthand props must be listed after all other props',
+                fix: generateFixerFunction(node, context, reservedList)
               });
               return memo;
             }
diff --git a/tests/lib/rules/jsx-sort-props.js b/tests/lib/rules/jsx-sort-props.js
index 082ef11f4e..33712a5996 100644
--- a/tests/lib/rules/jsx-sort-props.js
+++ b/tests/lib/rules/jsx-sort-props.js
@@ -336,59 +336,75 @@ ruleTester.run('jsx-sort-props', rule, {
     {
       code: '<App key="key" b c="c" />',
       errors: [expectedShorthandLastError],
-      options: reservedFirstWithShorthandLast
+      options: reservedFirstWithShorthandLast,
+      output: '<App key="key" c="c" b />'
     },
     {
       code: '<App ref="ref" key="key" isShorthand veryLastAttribute="yes" />',
       errors: [expectedError, expectedShorthandLastError],
-      options: reservedFirstWithShorthandLast
+      options: reservedFirstWithShorthandLast,
+      output: '<App ref="ref" key="key" veryLastAttribute="yes" isShorthand />'
     },
     {
       code: '<App a z onFoo onBar />;',
       errors: [expectedError],
-      options: callbacksLastArgs
+      options: callbacksLastArgs,
+      output: '<App a z onBar onFoo />;'
     },
     {
       code: '<App a onBar onFoo z />;',
       errors: [expectedCallbackError],
-      options: callbacksLastArgs
+      options: callbacksLastArgs,
+      output: '<App a z onBar onFoo />;'
     },
     {
       code: '<App a="a" b />;',
       errors: [expectedShorthandFirstError],
-      options: shorthandFirstArgs
+      options: shorthandFirstArgs,
+      output: '<App b a="a" />;'
     },
     {
       code: '<App z x a="a" />;',
       errors: [expectedError],
-      options: shorthandFirstArgs
+      options: shorthandFirstArgs,
+      output: '<App x z a="a" />;'
     },
     {
       code: '<App b a="a" />;',
       errors: [expectedShorthandLastError],
-      options: shorthandLastArgs
+      options: shorthandLastArgs,
+      output: '<App a="a" b />;'
     },
     {
       code: '<App a="a" onBar onFoo z x />;',
       errors: [shorthandAndCallbackLastArgs],
-      options: shorthandLastArgs
+      options: shorthandLastArgs,
+      output: '<App a="a" onBar onFoo x z />;'
+    },
+    {
+      code: '<App b a />;',
+      errors: [expectedError],
+      options: sortAlphabeticallyArgs,
+      output: '<App a b />;'
     },
-    {code: '<App b a />;', errors: [expectedError], options: sortAlphabeticallyArgs},
     // reservedFirst
     {
       code: '<App a key={1} />',
       options: reservedFirstAsBooleanArgs,
-      errors: [expectedReservedFirstError]
+      errors: [expectedReservedFirstError],
+      output: '<App key={1} a />'
     },
     {
       code: '<div a dangerouslySetInnerHTML={{__html: "EPR"}} />',
       options: reservedFirstAsBooleanArgs,
-      errors: [expectedReservedFirstError]
+      errors: [expectedReservedFirstError],
+      output: '<div dangerouslySetInnerHTML={{__html: "EPR"}} a />'
     },
     {
       code: '<App ref="r" key={2} b />',
       options: reservedFirstAsBooleanArgs,
-      errors: [expectedError]
+      errors: [expectedError],
+      output: '<App key={2} ref="r" b />'
     },
     {
       code: '<App key={2} b a />',
@@ -411,12 +427,14 @@ ruleTester.run('jsx-sort-props', rule, {
     {
       code: '<App key={3} children={<App />} />',
       options: reservedFirstAsArrayArgs,
-      errors: [expectedError]
+      errors: [expectedError],
+      output: '<App children={<App />} key={3} />'
     },
     {
       code: '<App z ref="r" />',
       options: reservedFirstWithNoSortAlphabeticallyArgs,
-      errors: [expectedReservedFirstError]
+      errors: [expectedReservedFirstError],
+      output: '<App ref="r" z />'
     },
     {
       code: '<App key={4} />',

From 4b2d7f27bb6a9932726eb783cc8df8b23f71a74e Mon Sep 17 00:00:00 2001
From: George Guliashvili <george.giorgi.guliashvili@gmail.com>
Date: Thu, 25 Apr 2019 23:28:28 +0100
Subject: [PATCH 2/2] Simplify prop sorting code

---
 lib/rules/jsx-sort-props.js       | 18 ++++++------------
 tests/lib/rules/jsx-sort-props.js |  2 +-
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js
index 5255093ae6..a0b83a4d06 100644
--- a/lib/rules/jsx-sort-props.js
+++ b/lib/rules/jsx-sort-props.js
@@ -34,11 +34,9 @@ function contextCompare(a, b, options) {
   if (options.reservedFirst) {
     const aIsReserved = isReservedPropName(aProp, options.reservedList);
     const bIsReserved = isReservedPropName(bProp, options.reservedList);
-    if ((aIsReserved && bIsReserved) || (!aIsReserved && !bIsReserved)) {
-      // pass
-    } else if (aIsReserved && !bIsReserved) {
+    if (aIsReserved && !bIsReserved) {
       return -1;
-    } else {
+    } else if (!aIsReserved && bIsReserved) {
       return 1;
     }
   }
@@ -46,22 +44,18 @@ function contextCompare(a, b, options) {
   if (options.callbacksLast) {
     const aIsCallback = isCallbackPropName(aProp);
     const bIsCallback = isCallbackPropName(bProp);
-    if ((aIsCallback && bIsCallback) || (!aIsCallback && !bIsCallback)) {
-      // pass
-    } else if (aIsCallback && !bIsCallback) {
+    if (aIsCallback && !bIsCallback) {
       return 1;
-    } else {
+    } else if (!aIsCallback && bIsCallback) {
       return -1;
     }
   }
 
   if (options.shorthandFirst || options.shorthandLast) {
     const shorthandSign = options.shorthandFirst ? -1 : 1;
-    if (!a.value && !b.value) {
-      // pass
-    } else if (!a.value) {
+    if (!a.value && b.value) {
       return shorthandSign;
-    } else {
+    } else if (a.value && !b.value) {
       return -shorthandSign;
     }
   }
diff --git a/tests/lib/rules/jsx-sort-props.js b/tests/lib/rules/jsx-sort-props.js
index 33712a5996..a58dd57c91 100644
--- a/tests/lib/rules/jsx-sort-props.js
+++ b/tests/lib/rules/jsx-sort-props.js
@@ -343,7 +343,7 @@ ruleTester.run('jsx-sort-props', rule, {
       code: '<App ref="ref" key="key" isShorthand veryLastAttribute="yes" />',
       errors: [expectedError, expectedShorthandLastError],
       options: reservedFirstWithShorthandLast,
-      output: '<App ref="ref" key="key" veryLastAttribute="yes" isShorthand />'
+      output: '<App key="key" ref="ref" veryLastAttribute="yes" isShorthand />'
     },
     {
       code: '<App a z onFoo onBar />;',