From bd20a795bda69e5b3848a931918de46c9080da91 Mon Sep 17 00:00:00 2001 From: Kenneth Chung Date: Mon, 3 Oct 2016 21:40:43 -0700 Subject: [PATCH 1/4] jsx-max-props-per-line: Fix when prop spans multiple lines In this patch we now consider props to be on the same "line" if the line of where one prop ends matches the line of where the next prop begins. e.g. ``` ``` `foo` and `bar` are now considered to be on the same line. Previously, we would only look at the line in which the prop begins. --- docs/rules/jsx-max-props-per-line.md | 6 ++++- lib/rules/jsx-max-props-per-line.js | 33 +++++++++++------------ tests/lib/rules/jsx-max-props-per-line.js | 19 +++++++++++++ 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/docs/rules/jsx-max-props-per-line.md b/docs/rules/jsx-max-props-per-line.md index ff50d50a4c..a8222c9069 100644 --- 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 ; + +; ``` The following patterns are not considered warnings: diff --git a/lib/rules/jsx-max-props-per-line.js b/lib/rules/jsx-max-props-per-line.js index 0838edc68c..4714b717e4 100644 --- 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 // ------------------------------------------------------------------------------ @@ -45,30 +43,31 @@ module.exports = { return { JSXOpeningElement: function (node) { - var props = {}; + if (!node.attributes.length) { + return; + } + + var firstProp = node.attributes[0]; + var linePartitionedProps = [[firstProp]]; - node.attributes.forEach(function(decl) { - var line = decl.loc.start.line; - if (props[line]) { - props[line].push(decl); + 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 index 982936e0be..f676a7f57e 100644 --- a/tests/lib/rules/jsx-max-props-per-line.js +++ b/tests/lib/rules/jsx-max-props-per-line.js @@ -89,5 +89,24 @@ 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: [ + '' + ].join('\n'), + errors: [{message: 'Prop `bar` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '' + ].join('\n'), + options: [{maximum: 2}], + errors: [{message: 'Prop `baz` must be placed on a new line'}], + parserOptions: parserOptions }] }); From b485e2854124aa31b9b047b461d76bf153829cd1 Mon Sep 17 00:00:00 2001 From: Kenneth Chung Date: Mon, 3 Oct 2016 21:50:27 -0700 Subject: [PATCH 2/4] jsx-max-props-per-line: Add `when` option If `when` is set to `multiline`, then this rule skips checking entirely if the jsx opening tag is a single line. Defaults `when` to `always` which is the current behavior. --- docs/rules/jsx-max-props-per-line.md | 24 ++++++++++++++++++++++- lib/rules/jsx-max-props-per-line.js | 9 +++++++++ tests/lib/rules/jsx-max-props-per-line.js | 11 +++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/docs/rules/jsx-max-props-per-line.md b/docs/rules/jsx-max-props-per-line.md index a8222c9069..06152d073a 100644 --- a/docs/rules/jsx-max-props-per-line.md +++ b/docs/rules/jsx-max-props-per-line.md @@ -35,7 +35,7 @@ The following patterns are not considered warnings: ```js ... -"jsx-max-props-per-line": [, { "maximum": }] +"jsx-max-props-per-line": [, { "maximum": , "when": }] ... ``` @@ -60,6 +60,28 @@ The following patterns are not considered warnings: />; ``` +### `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}] + +``` + +The following patterns are not considered warnings: +```jsx +// [1, {when: multiline}] + + +``` + ## 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 index 4714b717e4..1157784ca0 100644 --- a/lib/rules/jsx-max-props-per-line.js +++ b/lib/rules/jsx-max-props-per-line.js @@ -23,6 +23,10 @@ module.exports = { maximum: { type: 'integer', minimum: 1 + }, + when: { + type: 'string', + enum: ['always', 'multiline'] } } }] @@ -33,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') { @@ -47,6 +52,10 @@ module.exports = { return; } + if (when === 'multiline' && node.loc.start.line === node.loc.end.line) { + return; + } + var firstProp = node.attributes[0]; var linePartitionedProps = [[firstProp]]; diff --git a/tests/lib/rules/jsx-max-props-per-line.js b/tests/lib/rules/jsx-max-props-per-line.js index f676a7f57e..b322c367f9 100644 --- a/tests/lib/rules/jsx-max-props-per-line.js +++ b/tests/lib/rules/jsx-max-props-per-line.js @@ -25,12 +25,23 @@ var parserOptions = { var ruleTester = new RuleTester(); ruleTester.run('jsx-max-props-per-line', rule, { valid: [{ + code: '', + parserOptions: parserOptions + }, { code: '', parserOptions: parserOptions }, { code: '', options: [{maximum: 2}], parserOptions: parserOptions + }, { + code: '', + options: [{when: 'multiline'}], + parserOptions: parserOptions + }, { + code: '', + options: [{maximum: 2, when: 'multiline'}], + parserOptions: parserOptions }, { code: '', options: [{maximum: 2}], From b52f3b8cb43d888d132d2a08773a316733b67e2d Mon Sep 17 00:00:00 2001 From: Kenneth Chung Date: Mon, 3 Oct 2016 22:59:43 -0700 Subject: [PATCH 3/4] jsx-max-props-per-line: Add additional test cases Make sure it works when a prop is on the same line as the start line of the tag, as well as with spread props that spans multiple lines. --- tests/lib/rules/jsx-max-props-per-line.js | 44 +++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/lib/rules/jsx-max-props-per-line.js b/tests/lib/rules/jsx-max-props-per-line.js index b322c367f9..55ce2d2e79 100644 --- a/tests/lib/rules/jsx-max-props-per-line.js +++ b/tests/lib/rules/jsx-max-props-per-line.js @@ -38,6 +38,10 @@ ruleTester.run('jsx-max-props-per-line', rule, { code: '', options: [{when: 'multiline'}], parserOptions: parserOptions + }, { + code: '', + options: [{when: 'multiline'}], + parserOptions: parserOptions }, { code: '', options: [{maximum: 2, when: 'multiline'}], @@ -109,6 +113,46 @@ ruleTester.run('jsx-max-props-per-line', rule, { ].join('\n'), errors: [{message: 'Prop `bar` must be placed on a new line'}], parserOptions: parserOptions + }, { + code: [ + '' + ].join('\n'), + errors: [{message: 'Prop `bar` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '' + ].join('\n'), + options: [{maximum: 2}], + errors: [{message: 'Prop `baz` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '' + ].join('\n'), + errors: [{message: 'Prop `rest` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '' + ].join('\n'), + errors: [{message: 'Prop `bar` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '' + ].join('\n'), + errors: [{message: 'Prop `rest` must be placed on a new line'}], + parserOptions: parserOptions }, { code: [ ' Date: Sun, 29 Jan 2017 17:46:09 -0800 Subject: [PATCH 4/4] jsx-max-props-per-line: Use json format in docs To be consistent with the "Rule Options" section and other docs --- docs/rules/jsx-max-props-per-line.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/rules/jsx-max-props-per-line.md b/docs/rules/jsx-max-props-per-line.md index 06152d073a..490b50e0f1 100644 --- a/docs/rules/jsx-max-props-per-line.md +++ b/docs/rules/jsx-max-props-per-line.md @@ -46,14 +46,14 @@ 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 }] ; ``` The following patterns are not considered warnings: ```jsx -// [1, {maximum: 2}] +// [1, { "maximum": 2 }] ``` The following patterns are not considered warnings: ```jsx -// [1, {when: multiline}] +// [1, { "when": "multiline" }]