Repository: yannickcr/eslint-plugin-react
Pull request #886 (2016-10-05 04:04:25)

jsx-first-prop-new-line: autofix
Hi I have made an autofix for the rule jsx-first-prop-new-line for a fork and I thought it would be useful to have it on the main repo.

I am aware and monitoring #883. Will update the autofix after that is approved! Meanwhile, PR for visibility and to start the process :)


snowypowers: Rebased on top of #883. No major changes except for adding outputs to test cases.
However, I would like to take this moment to point out that the current code uses a foreach loop:
```javascript
        if (
          (configuration === 'multiline' && isMultilineJSX(node)) ||
          (configuration === 'multiline-multiprop' && isMultilineJSX(node) && node.attributes.length > 1) ||
          (configuration === 'always')
        ) {
          node.attributes.forEach(function(decl) {
            ...collapsed code...
         }
```

This results in multiple errors being thrown in fail cases with multiple props:
```jsx
options: ['always']

<Foo propOne="one" propTwo="two"/>
```

Seeing that this rule is only concerned with amending the position of the first prop, should the function be only checking for the first prop and throwing error if the first prop fails?

Possible fix:
```javascript
// We check for at least one prop to ensure we can safely use node.attributes[0] inside
        if (
          (configuration === 'multiline' && isMultilineJSX(node) && node.attributes.length > 0) ||
          (configuration === 'multiline-multiprop' && isMultilineJSX(node) && node.attributes.length > 1) ||
          (configuration === 'always' && node.attributes.length > 0)
        ) {
          var decl = node.attributes[0];
         //Code to verify and fix decl
```
ljharb: @snowypowers while i think it makes sense to only throw for the first error as long as the autofixer results in no remaining cases of the error.

Patch:
diff --git a/README.md b/README.md
index 11b8861f..e8c80b72 100644
--- a/README.md
+++ b/README.md
@@ -117,7 +117,7 @@ Finally, enable all of the rules that you would like to use.  Use [our preset](#
 * [react/jsx-curly-spacing](docs/rules/jsx-curly-spacing.md): Enforce or disallow spaces inside of curly braces in JSX attributes (fixable)
 * [react/jsx-equals-spacing](docs/rules/jsx-equals-spacing.md): Enforce or disallow spaces around equal signs in JSX attributes (fixable)
 * [react/jsx-filename-extension](docs/rules/jsx-filename-extension.md): Restrict file extensions that may contain JSX
-* [react/jsx-first-prop-new-line](docs/rules/jsx-first-prop-new-line.md): Enforce position of the first prop in JSX
+* [react/jsx-first-prop-new-line](docs/rules/jsx-first-prop-new-line.md): Enforce position of the first prop in JSX (fixable)
 * [react/jsx-handler-names](docs/rules/jsx-handler-names.md): Enforce event handler naming conventions in JSX
 * [react/jsx-indent](docs/rules/jsx-indent.md): Validate JSX indentation
 * [react/jsx-indent-props](docs/rules/jsx-indent-props.md): Validate props indentation in JSX (fixable)
diff --git a/docs/rules/jsx-first-prop-new-line.md b/docs/rules/jsx-first-prop-new-line.md
index 309efb96..67e45d40 100644
--- a/docs/rules/jsx-first-prop-new-line.md
+++ b/docs/rules/jsx-first-prop-new-line.md
@@ -2,6 +2,8 @@
 
 Ensure correct position of the first property.
 
+**Fixable:** This rule is automatically fixable using the `--fix` flag on the command line. However, fix does not include indentation. Please rerun lint to correct those errors.
+
 ## Rule Details
 
 This rule checks whether the first property of all JSX elements is correctly placed. There are three possible configurations:
diff --git a/lib/rules/jsx-first-prop-new-line.js b/lib/rules/jsx-first-prop-new-line.js
index 981dc62e..6307439b 100644
--- a/lib/rules/jsx-first-prop-new-line.js
+++ b/lib/rules/jsx-first-prop-new-line.js
@@ -15,6 +15,7 @@ module.exports = {
       category: 'Stylistic Issues',
       recommended: false
     },
+    fixable: 'code',
 
     schema: [{
       enum: ['always', 'never', 'multiline', 'multiline-multiprop']
@@ -35,20 +36,27 @@ module.exports = {
           (configuration === 'multiline-multiprop' && isMultilineJSX(node) && node.attributes.length > 1) ||
           (configuration === 'always')
         ) {
-          node.attributes.forEach(function(decl) {
+          node.attributes.some(function(decl) {
             if (decl.loc.start.line === node.loc.start.line) {
               context.report({
                 node: decl,
-                message: 'Property should be placed on a new line'
+                message: 'Property should be placed on a new line',
+                fix: function(fixer) {
+                  return fixer.replaceTextRange([node.name.end, decl.start], '\n');
+                }
               });
             }
+            return true;
           });
         } else if (configuration === 'never' && node.attributes.length > 0) {
           var firstNode = node.attributes[0];
           if (node.loc.start.line < firstNode.loc.start.line) {
             context.report({
               node: firstNode,
-              message: 'Property should be placed on the same line as the component declaration'
+              message: 'Property should be placed on the same line as the component declaration',
+              fix: function(fixer) {
+                return fixer.replaceTextRange([node.name.end, firstNode.start], ' ');
+              }
             });
             return;
           }


Test patch:
diff --git a/tests/lib/rules/jsx-first-prop-new-line.js b/tests/lib/rules/jsx-first-prop-new-line.js
index 9feb9cfd..a7b558ce 100644
--- a/tests/lib/rules/jsx-first-prop-new-line.js
+++ b/tests/lib/rules/jsx-first-prop-new-line.js
@@ -152,7 +152,11 @@ ruleTester.run('jsx-first-prop-new-line', rule, {
 
   invalid: [
     {
-      code: '<Foo prop="one" />',
+      code: '<Foo propOne="one" propTwo="two" />',
+      output: [
+        '<Foo',
+        'propOne="one" propTwo="two" />'
+      ].join('\n'),
       options: ['always'],
       errors: [{message: 'Property should be placed on a new line'}],
       parser: parserOptions
@@ -163,6 +167,12 @@ ruleTester.run('jsx-first-prop-new-line', rule, {
         '  propTwo="two"',
         '/>'
       ].join('\n'),
+      output: [
+        '<Foo',
+        'propOne="one"',
+        '  propTwo="two"',
+        '/>'
+      ].join('\n'),
       options: ['always'],
       errors: [{message: 'Property should be placed on a new line'}],
       parser: parserOptions
@@ -170,8 +180,13 @@ ruleTester.run('jsx-first-prop-new-line', rule, {
     {
       code: [
         '<Foo',
-        ' propOne="one"',
-        ' propTwo="two"',
+        '  propOne="one"',
+        '  propTwo="two"',
+        '/>'
+      ].join('\n'),
+      output: [
+        '<Foo propOne="one"',
+        '  propTwo="two"',
         '/>'
       ].join('\n'),
       options: ['never'],
@@ -183,6 +198,11 @@ ruleTester.run('jsx-first-prop-new-line', rule, {
         '<Foo prop={{',
         '}} />'
       ].join('\n'),
+      output: [
+        '<Foo',
+        'prop={{',
+        '}} />'
+      ].join('\n'),
       options: ['multiline'],
       errors: [{message: 'Property should be placed on a new line'}],
       parser: parserOptions
@@ -192,6 +212,11 @@ ruleTester.run('jsx-first-prop-new-line', rule, {
         '<Foo bar={{',
         '}} baz />'
       ].join('\n'),
+      output: [
+        '<Foo',
+        'bar={{',
+        '}} baz />'
+      ].join('\n'),
       options: ['multiline-multiprop'],
       errors: [{message: 'Property should be placed on a new line'}],
       parser: parserOptions