Repository: yannickcr/eslint-plugin-react
Pull request #1273 (2017-06-25 05:39:16)

Add fixer for jsx/sort-props
Hi all.

I've been meaning to dabble with eslint for a while, and this was a small project I wanted to take on. We're currently using this linter on our codebase, and I thought it would be great to have a fixer for this one. 

This particular rule has so many configurations, and I didn't bother to handle all of them although it wouldn't be hard to do so. I may do that in a follow-up PR.

I tested using the following:

```
node_modules/mocha/bin/_mocha /Users/amirsharif/Projects/eslint-plugin-react/tests/lib/rules/jsx-sort-props.js
```

I also added an additional, multi-line test. 


Patch:
diff --git a/README.md b/README.md
index 2dd2cddb..6bb08367 100644
--- a/README.md
+++ b/README.md
@@ -142,7 +142,7 @@ Finally, enable all of the rules that you would like to use.  Use [our preset](#
 * [react/jsx-no-target-blank](docs/rules/jsx-no-target-blank.md): Prevent usage of unsafe `target='_blank'`
 * [react/jsx-no-undef](docs/rules/jsx-no-undef.md): Disallow undeclared variables in JSX
 * [react/jsx-pascal-case](docs/rules/jsx-pascal-case.md): Enforce PascalCase for user-defined JSX components
-* [react/jsx-sort-props](docs/rules/jsx-sort-props.md): Enforce props alphabetical sorting
+* [react/jsx-sort-props](docs/rules/jsx-sort-props.md): Enforce props alphabetical sorting (fixable)
 * [react/jsx-space-before-closing](docs/rules/jsx-space-before-closing.md): Validate spacing before closing bracket in JSX (fixable)
 * [react/jsx-tag-spacing](docs/rules/jsx-tag-spacing.md): Validate whitespace in and around the JSX opening and closing brackets (fixable)
 * [react/jsx-uses-react](docs/rules/jsx-uses-react.md): Prevent React to be incorrectly marked as unused
diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js
index 3a4c09b4..3eee76c6 100644
--- a/lib/rules/jsx-sort-props.js
+++ b/lib/rules/jsx-sort-props.js
@@ -40,6 +40,77 @@ function isReservedPropName(name, list) {
   return list.indexOf(name) >= 0;
 }
 
+function alphabeticalCompare(a, b, ignoreCase) {
+  if (ignoreCase) {
+    a = a.toLowerCase();
+    b = b.toLowerCase();
+  }
+  return a.localeCompare(b);
+}
+
+/**
+ * Create an array of arrays where each subarray is composed of attributes
+ * that are considered sortable.
+ * @param {Array<JSXSpreadAttribute|JSXAttribute>} attributes
+ * @return {Array<Array<JSXAttribute>}
+ */
+function getGroupsOfSortableAttributes(attributes) {
+  const sortableAttributeGroups = [];
+  let groupCount = 0;
+  for (let i = 0; i < attributes.length; i++) {
+    const lastAttr = attributes[i - 1];
+    // If we have no groups or if the last attribute was JSXSpreadAttribute
+    // then we start a new group. Append attributes to the group until we
+    // come across another JSXSpreadAttribute or exhaust the array.
+    if (
+      !lastAttr ||
+      (lastAttr.type === 'JSXSpreadAttribute' &&
+        attributes[i].type !== 'JSXSpreadAttribute')
+    ) {
+      groupCount++;
+      sortableAttributeGroups[groupCount - 1] = [];
+    }
+    if (attributes[i].type !== 'JSXSpreadAttribute') {
+      sortableAttributeGroups[groupCount - 1].push(attributes[i]);
+    }
+  }
+  return sortableAttributeGroups;
+}
+
+const generateFixerFunction = (node, context) => {
+  const sourceCode = context.getSourceCode();
+  const attributes = node.attributes.slice(0);
+  const configuration = context.options[0] || {};
+  const ignoreCase = configuration.ignoreCase || 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 sortableAttributeGroups = getGroupsOfSortableAttributes(attributes);
+  const sortedAttributeGroups = sortableAttributeGroups.slice(0).map(group =>
+    group.slice(0).sort((a, b) =>
+      alphabeticalCompare(propName(a), propName(b), ignoreCase)
+    )
+  );
+
+  return function(fixer) {
+    const fixers = [];
+
+    // Replace each unsorted attribute with the sorted one.
+    sortableAttributeGroups.forEach((sortableGroup, ii) => {
+      sortableGroup.forEach((attr, jj) => {
+        const sortedAttr = sortedAttributeGroups[ii][jj];
+        const sortedAttrText = sourceCode.getText(sortedAttr);
+        fixers.push(
+          fixer.replaceTextRange([attr.start, attr.end], sortedAttrText)
+        );
+      });
+    });
+
+    return fixers;
+  };
+};
+
 /**
  * Checks if the `reservedFirst` option is valid
  * @param {Object} context The context of the rule
@@ -88,7 +159,7 @@ module.exports = {
       category: 'Stylistic Issues',
       recommended: false
     },
-
+    fixable: 'code',
     schema: [{
       type: 'object',
       properties: {
@@ -168,7 +239,8 @@ module.exports = {
               if (!noSortAlphabetically && currentPropName < previousPropName) {
                 context.report({
                   node: decl,
-                  message: 'Props should be sorted alphabetically'
+                  message: 'Props should be sorted alphabetically',
+                  fix: generateFixerFunction(node, context)
                 });
                 return memo;
               }
@@ -228,7 +300,8 @@ module.exports = {
           if (!noSortAlphabetically && currentPropName < previousPropName) {
             context.report({
               node: decl,
-              message: 'Props should be sorted alphabetically'
+              message: 'Props should be sorted alphabetically',
+              fix: generateFixerFunction(node, context)
             });
             return memo;
           }


Test patch:
diff --git a/tests/lib/rules/jsx-sort-props.js b/tests/lib/rules/jsx-sort-props.js
index 08628da9..4ec08dd5 100644
--- a/tests/lib/rules/jsx-sort-props.js
+++ b/tests/lib/rules/jsx-sort-props.js
@@ -153,15 +153,82 @@ ruleTester.run('jsx-sort-props', rule, {
     }
   ],
   invalid: [
-    {code: '<App b a />;', errors: [expectedError]},
-    {code: '<App {...this.props} b a />;', errors: [expectedError]},
-    {code: '<App c {...this.props} b a />;', errors: [expectedError]},
-    {code: '<App a A />;', errors: [expectedError]},
-    {code: '<App B a />;', options: ignoreCaseArgs, errors: [expectedError]},
-    {code: '<App B A c />;', options: ignoreCaseArgs, errors: [expectedError]},
-    {code: '<App c="a" a="c" b="b" />;', errors: 2},
-    {code: '<App {...this.props} c="a" a="c" b="b" />;', errors: 2},
-    {code: '<App d="d" b="b" {...this.props} c="a" a="c" />;', errors: 2},
+    {
+      code: '<App b a />;',
+      errors: [expectedError],
+      output: '<App a b />;'
+    },
+    {
+      code: '<App {...this.props} b a />;',
+      errors: [expectedError],
+      output: '<App {...this.props} a b />;'
+    },
+    {
+      code: '<App c {...this.props} b a />;',
+      errors: [expectedError],
+      output: '<App c {...this.props} a b />;'
+    },
+    {
+      code: '<App a A />;',
+      errors: [expectedError],
+      output: '<App a A />;'
+    },
+    {
+      code: '<App B a />;',
+      options: ignoreCaseArgs,
+      errors: [expectedError],
+      output: '<App a B />;'
+    },
+    {
+      code: '<App B A c />;',
+      options: ignoreCaseArgs,
+      errors: [expectedError],
+      output: '<App A B c />;'
+    },
+    {
+      code: '<App c="a" a="c" b="b" />;',
+      output: '<App a="c" b="b" c="a" />;',
+      errors: 2
+    },
+    {
+      code: '<App {...this.props} c="a" a="c" b="b" />;',
+      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 b="b" d="d" {...this.props} a="c" c="a" />;',
+      errors: 2
+    },
+    {
+      code: [
+        '<App',
+        'a={true}',
+        'z',
+        'r',
+        '_onClick={function(){}}',
+        'onHandle={function(){}}',
+        '{...this.props}',
+        'b={false}',
+        '{...otherProps}>',
+        '  {test}',
+        '</App>'
+      ].join('\n'),
+      output: [
+        '<App',
+        '_onClick={function(){}}',
+        'a={true}',
+        'onHandle={function(){}}',
+        'r',
+        'z',
+        '{...this.props}',
+        'b={false}',
+        '{...otherProps}>',
+        '  {test}',
+        '</App>'
+      ].join('\n'),
+      errors: 3
+    },
     {
       code: '<App a z onFoo onBar />;',
       errors: [expectedError],