Repository: yannickcr/eslint-plugin-react (js)

Issue:
jsx-sort-props rule is not working as it should
Rule configuration:
```js
'react/jsx-sort-props': [
  'error',
  {
    noSortAlphabetically: false,
    ignoreCase: true,
    callbacksLast: false,
    shorthandLast: true,
    reservedFirst: true,
  },
],
```

As a result `callbacksLast`, `shorthandLast` options are overridden by `noSortAlphabetically`, so props are sorting only by alphabet ignoring shorthands at the end. Also, how can I reach this sort order:

- object spread props
- reserved props
- common props (html attributes, etc.)
- callback props
- shorthand/boolean props

Example:
```jsx
<button
  {...rest}
  key={1}
  type="button"
  onClick={() => {}}
  disabled
/>
```

Pull request description:
Fix `reservedFirst` for `jsx-sort-props` rule
Hello,

this PR should (hopefully) fix #1692 .
Previously if `reservedFirst` was enabled, it would customly check for the sorting of the props, if the currently viewed items were of the same type (either both reserved or both unreserved) and if not otherwise set up (i.e. `noSortAlphabetically` was enabled).

This PR now adds test cases for the viewed case: `reservedFirst` and `shorthandLast` (I didn't add any more test cases, but can do so, if requested). Also it removes the special handling of the sorting but delegates to the rest of the handler to take care of any other options (e.g. `shorthandLast`, `shorthandFirst`, `callbackLast`, etc.). (Also there was a missing parameter in the shadowed alphabetically-sorting check, which I added).

Yeah. If you need/require anything else from this PR, hit me up and I'll add it. Additionally I hope it's ok, when I regularly rebase this onto master, if any other development is done (if not, let me know as well). :smile:

Patch:
diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js
index 99aba6b0..d6e5a982 100644
--- a/lib/rules/jsx-sort-props.js
+++ b/lib/rules/jsx-sort-props.js
@@ -245,15 +245,8 @@ module.exports = {
             const previousIsReserved = isReservedPropName(previousPropName, reservedList);
             const currentIsReserved = isReservedPropName(currentPropName, reservedList);
 
-            if ((previousIsReserved && currentIsReserved) || (!previousIsReserved && !currentIsReserved)) {
-              if (!noSortAlphabetically && currentPropName < previousPropName) {
-                context.report({
-                  node: decl,
-                  message: 'Props should be sorted alphabetically',
-                  fix: generateFixerFunction(node, context, reservedList)
-                });
-                return memo;
-              }
+            if (previousIsReserved && !currentIsReserved) {
+              return decl;
             }
             if (!previousIsReserved && currentIsReserved) {
               context.report({
@@ -261,8 +254,8 @@ module.exports = {
                 message: 'Reserved props must be listed before all other props',
                 fix: generateFixerFunction(node, context, reservedList)
               });
+              return memo;
             }
-            return decl;
           }
 
           if (callbacksLast) {
@@ -310,7 +303,7 @@ module.exports = {
             context.report({
               node: decl,
               message: 'Props should be sorted alphabetically',
-              fix: generateFixerFunction(node, context)
+              fix: generateFixerFunction(node, context, reservedList)
             });
             return memo;
           }


Test patch:
diff --git a/tests/lib/rules/jsx-sort-props.js b/tests/lib/rules/jsx-sort-props.js
index 412ef4ae..2f32e6b2 100644
--- a/tests/lib/rules/jsx-sort-props.js
+++ b/tests/lib/rules/jsx-sort-props.js
@@ -88,6 +88,10 @@ const reservedFirstWithNoSortAlphabeticallyArgs = [{
   noSortAlphabetically: true,
   reservedFirst: true
 }];
+const reservedFirstWithShorthandLast = [{
+  reservedFirst: true,
+  shorthandLast: true
+}];
 const reservedFirstAsEmptyArrayArgs = [{
   reservedFirst: []
 }];
@@ -149,6 +153,10 @@ ruleTester.run('jsx-sort-props', rule, {
     {
       code: '<div ref="r" dangerouslySetInnerHTML={{__html: "EPR"}} key={0} children={<App />} b a c />',
       options: reservedFirstWithNoSortAlphabeticallyArgs
+    },
+    {
+      code: '<App key="key" c="c" b />',
+      options: reservedFirstWithShorthandLast
     }
   ],
   invalid: [
@@ -230,6 +238,16 @@ ruleTester.run('jsx-sort-props', rule, {
     `,
       errors: 3
     },
+    {
+      code: '<App key="key" b c="c" />',
+      errors: [expectedShorthandLastError],
+      options: reservedFirstWithShorthandLast
+    },
+    {
+      code: '<App ref="ref" key="key" isShorthand veryLastAttribute="yes" />',
+      errors: [expectedError, expectedShorthandLastError],
+      options: reservedFirstWithShorthandLast
+    },
     {
       code: '<App a z onFoo onBar />;',
       errors: [expectedError],
@@ -293,7 +311,7 @@ ruleTester.run('jsx-sort-props', rule, {
       code: '<App dangerouslySetInnerHTML={{__html: "EPR"}} e key={2} b />',
       options: reservedFirstAsBooleanArgs,
       output: '<App key={2} b dangerouslySetInnerHTML={{__html: "EPR"}} e />',
-      errors: [expectedReservedFirstError]
+      errors: [expectedReservedFirstError, expectedError]
     },
     {
       code: '<App key={3} children={<App />} />',