Merge pull request #882 from kentor/jsx-max-props-per-line-updates

Jsx max props per line updates
jsx-eslint · Feb 15, 2017 · 3b25a3b · 3b25a3b
2 parents 54abb8b + a84d5f5
commit 3b25a3b
Show file tree

Hide file tree

Showing 3 changed files with 129 additions and 21 deletions.
diff --git a/docs/rules/jsx-max-props-per-line.md b/docs/rules/jsx-max-props-per-line.md
@@ -4,12 +4,16 @@ Limiting the maximum of props on a single line can improve readability.
 
 ## Rule Details
 
-This rule checks all JSX elements and verifies that the number of props per line do not exceed the maximum allowed. A spread attribute counts as one prop. This rule is off by default and when on the default maximum of props on one line is `1`.
+This rule checks all JSX elements and verifies that the number of props per line do not exceed the maximum allowed. Props are considered to be in a new line if there is a line break between the start of the prop and the end of the previous prop. A spread attribute counts as one prop. This rule is off by default and when on the default maximum of props on one line is `1`.
 
 The following patterns are considered warnings:
 
 ```jsx
 <Hello lastName="Smith" firstName="John" />;
+
+<Hello foo={{
+  bar
+}} baz />;
 ```
 
 The following patterns are not considered warnings:
@@ -31,7 +35,7 @@ The following patterns are not considered warnings:
 
 ```js
 ...
-"jsx-max-props-per-line": [<enabled>, { "maximum": <number> }]
+"jsx-max-props-per-line": [<enabled>, { "maximum": <number>, "when": <string> }]
 ...
 ```
 
@@ -42,20 +46,42 @@ Maximum number of props allowed on a single line. Default to `1`.
 The following patterns are considered warnings:
 
 ```jsx
-// [1, {maximum: 2}]
+// [1, { "maximum": 2 }]
 <Hello firstName="John" lastName="Smith" tel={5555555} />;
 ```
 
 The following patterns are not considered warnings:
 
 ```jsx
-// [1, {maximum: 2}]
+// [1, { "maximum": 2 }]
 <Hello
   firstName="John" lastName="Smith"
   tel={5555555}
 />;
 ```
 
+### `when`
+
+Possible values:
+- `always` (default) - Always check for max props per line.
+- `multiline` - Only check for max props per line when jsx tag spans multiple lines.
+
+The following patterns are considered warnings:
+```jsx
+// [1, { "when": "always" }]
+<Hello firstName="John" lastName="Smith" />
+```
+
+The following patterns are not considered warnings:
+```jsx
+// [1, { "when": "multiline" }]
+<Hello firstName="John" lastName="Smith" />
+<Hello 
+  firstName="John" 
+  lastName="Smith" 
+/>
+```
+
 ## When not to use
 
 If you are not using JSX then you can disable this rule.
diff --git a/lib/rules/jsx-max-props-per-line.js b/lib/rules/jsx-max-props-per-line.js
@@ -5,8 +5,6 @@
 
 'use strict';
 
-var has = require('has');
-
 // ------------------------------------------------------------------------------
 // Rule Definition
 // ------------------------------------------------------------------------------
@@ -25,6 +23,10 @@ module.exports = {
         maximum: {
           type: 'integer',
           minimum: 1
+        },
+        when: {
+          type: 'string',
+          enum: ['always', 'multiline']
         }
       }
     }]
@@ -35,6 +37,7 @@ module.exports = {
     var sourceCode = context.getSourceCode();
     var configuration = context.options[0] || {};
     var maximum = configuration.maximum || 1;
+    var when = configuration.when || 'always';
 
     function getPropName(propNode) {
       if (propNode.type === 'JSXSpreadAttribute') {
@@ -45,30 +48,35 @@ module.exports = {
 
     return {
       JSXOpeningElement: function (node) {
-        var props = {};
+        if (!node.attributes.length) {
+          return;
+        }
+
+        if (when === 'multiline' && node.loc.start.line === node.loc.end.line) {
+          return;
+        }
 
-        node.attributes.forEach(function(decl) {
-          var line = decl.loc.start.line;
-          if (props[line]) {
-            props[line].push(decl);
+        var firstProp = node.attributes[0];
+        var linePartitionedProps = [[firstProp]];
+
+        node.attributes.reduce(function(last, decl) {
+          if (last.loc.end.line === decl.loc.start.line) {
+            linePartitionedProps[linePartitionedProps.length - 1].push(decl);
           } else {
-            props[line] = [decl];
+            linePartitionedProps.push([decl]);
           }
+          return decl;
         });
 
-        for (var line in props) {
-          if (!has(props, line)) {
-            continue;
-          }
-          if (props[line].length > maximum) {
-            var name = getPropName(props[line][maximum]);
+        linePartitionedProps.forEach(function(propsInLine) {
+          if (propsInLine.length > maximum) {
+            var name = getPropName(propsInLine[maximum]);
             context.report({
-              node: props[line][maximum],
+              node: propsInLine[maximum],
               message: 'Prop `' + name + '` must be placed on a new line'
             });
-            break;
           }
-        }
+        });
       }
     };
   }

diff --git a/tests/lib/rules/jsx-max-props-per-line.js b/tests/lib/rules/jsx-max-props-per-line.js
@@ -25,12 +25,27 @@ var parserOptions = {
 var ruleTester = new RuleTester();
 ruleTester.run('jsx-max-props-per-line', rule, {
   valid: [{
+    code: '<App />',
+    parserOptions: parserOptions
+  }, {
     code: '<App foo />',
     parserOptions: parserOptions
   }, {
     code: '<App foo bar />',
     options: [{maximum: 2}],
     parserOptions: parserOptions
+  }, {
+    code: '<App foo bar />',
+    options: [{when: 'multiline'}],
+    parserOptions: parserOptions
+  }, {
+    code: '<App foo {...this.props} />',
+    options: [{when: 'multiline'}],
+    parserOptions: parserOptions
+  }, {
+    code: '<App foo bar baz />',
+    options: [{maximum: 2, when: 'multiline'}],
+    parserOptions: parserOptions
   }, {
     code: '<App {...this.props} bar />',
     options: [{maximum: 2}],
@@ -89,5 +104,64 @@ ruleTester.run('jsx-max-props-per-line', rule, {
     ].join('\n'),
     errors: [{message: 'Prop `this.props` must be placed on a new line'}],
     parserOptions: parserOptions
+  }, {
+    code: [
+      '<App',
+      '  foo={{',
+      '  }} bar',
+      '/>'
+    ].join('\n'),
+    errors: [{message: 'Prop `bar` must be placed on a new line'}],
+    parserOptions: parserOptions
+  }, {
+    code: [
+      '<App foo={{',
+      '}} bar />'
+    ].join('\n'),
+    errors: [{message: 'Prop `bar` must be placed on a new line'}],
+    parserOptions: parserOptions
+  }, {
+    code: [
+      '<App foo bar={{',
+      '}} baz />'
+    ].join('\n'),
+    options: [{maximum: 2}],
+    errors: [{message: 'Prop `baz` must be placed on a new line'}],
+    parserOptions: parserOptions
+  }, {
+    code: [
+      '<App foo={{',
+      '}} {...rest} />'
+    ].join('\n'),
+    errors: [{message: 'Prop `rest` must be placed on a new line'}],
+    parserOptions: parserOptions
+  }, {
+    code: [
+      '<App {',
+      '  ...this.props',
+      '} bar />'
+    ].join('\n'),
+    errors: [{message: 'Prop `bar` must be placed on a new line'}],
+    parserOptions: parserOptions
+  }, {
+    code: [
+      '<App {',
+      '  ...this.props',
+      '} {',
+      '  ...rest',
+      '} />'
+    ].join('\n'),
+    errors: [{message: 'Prop `rest` must be placed on a new line'}],
+    parserOptions: parserOptions
+  }, {
+    code: [
+      '<App',
+      '  foo={{',
+      '  }} bar baz',
+      '/>'
+    ].join('\n'),
+    options: [{maximum: 2}],
+    errors: [{message: 'Prop `baz` must be placed on a new line'}],
+    parserOptions: parserOptions
   }]
 });