From 618bdb7c5a0ade4ad1face7d3af8f5caae2b7c26 Mon Sep 17 00:00:00 2001 From: TSMMark Date: Tue, 24 Oct 2017 23:31:21 -0400 Subject: [PATCH 01/14] new rule: one-element-per-line --- lib/rules/jsx-one-element-per-line.js | 134 +++++++++++ tests/lib/rules/jsx-one-element-per-line.js | 246 ++++++++++++++++++++ 2 files changed, 380 insertions(+) create mode 100644 lib/rules/jsx-one-element-per-line.js create mode 100644 tests/lib/rules/jsx-one-element-per-line.js diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-element-per-line.js new file mode 100644 index 0000000000..3671c5d795 --- /dev/null +++ b/lib/rules/jsx-one-element-per-line.js @@ -0,0 +1,134 @@ +/** + * @fileoverview Limit to one element tag per line in JSX + * @author Mark Allen + */ + +'use strict'; + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'Limit to one element tag per line in JSX', + category: 'Stylistic Issues', + recommended: false + }, + fixable: 'whitespace', + schema: [] + }, + + create: function (context) { + const sourceCode = context.getSourceCode(); + + function generateFixFunction(elementInLine) { + const nodeText = sourceCode.getText(elementInLine); + + return function(fixer) { + return fixer.replaceText(elementInLine, `\n${nodeText}`); + }; + } + + return { + JSXOpeningElement: function (node) { + if (!node.parent) { + return; + } + + const openingElementEndLine = node.loc.end.line; + const openingElementEndColumn = node.loc.end.column; + + // Children + if (node.parent.children.length) { + const childrenOnLine = []; + + node.parent.children.forEach(childNode => { + if (!childNode.openingElement || childNode.openingElement.loc.start.line !== openingElementEndLine) { + return; + } + + childrenOnLine.push(childNode); + }); + + if (!childrenOnLine.length) { + return; + } + + childrenOnLine.forEach(elementInLine => { + context.report({ + node: elementInLine, + message: `Opening tag for Element \`${elementInLine.openingElement.name.name}\` must be placed on a new line`, + fix: generateFixFunction(elementInLine) + }); + }); + } + + // Siblings + if (node.parent.parent && node.parent.parent.children && node.parent.parent.children.length) { + const siblingsOnLine = []; + node.parent.parent.children.forEach(childNode => { + if ( + node.parent === childNode || + !childNode.openingElement || + childNode.openingElement.loc.start.line !== openingElementEndLine || + childNode.openingElement.loc.start.column < openingElementEndColumn + ) { + return; + } + + siblingsOnLine.push(childNode); + }); + + if (!siblingsOnLine.length) { + return; + } + + siblingsOnLine.forEach(elementInLine => { + context.report({ + node: elementInLine, + message: `Opening tag for Element \`${elementInLine.openingElement.name.name}\` must be placed on a new line`, + fix: generateFixFunction(elementInLine) + }); + }); + } + }, + + JSXClosingElement: function (node) { + if (!node.parent || !node.parent.children.length) { + return; + } + + const childrenOnLine = []; + const closingElementStartLine = node.loc.end.line; + + node.parent.children.forEach(childNode => { + const reportableLines = [childNode.openingElement, childNode.closingElement].reduce((lines, el) => { + if (!el) { + return lines; + } + + return lines.concat([el.loc.start.line, el.loc.end.line]); + }, []); + + if (reportableLines.indexOf(closingElementStartLine) === -1) { + return; + } + + childrenOnLine.push(childNode); + }); + + if (!childrenOnLine.length) { + return; + } + + context.report({ + node: node, + message: `Closing tag for Element \`${node.name.name}\` must be placed on a new line`, + fix: generateFixFunction(node) + }); + } + }; + } +}; diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js new file mode 100644 index 0000000000..04ba7a21de --- /dev/null +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -0,0 +1,246 @@ +/** + * @fileoverview Limit to one element tag per line in JSX + * @author Mark Allen + */ + +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/jsx-one-element-per-line'); +const RuleTester = require('eslint').RuleTester; + +const parserOptions = { + ecmaVersion: 8, + sourceType: 'module', + ecmaFeatures: { + experimentalObjectRestSpread: true, + jsx: true + } +}; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({parserOptions}); +ruleTester.run('jsx-max-elements-per-line', rule, { + valid: [{ + code: '' + }, { + code: '' + }, { + code: '{"foo"}' + }, { + code: 'foo' + }, { + code: '' + }, { + code: [ + '', + ' ', + '' + ].join('\n') + }, { + code: [ + '', + ' ', + '' + ].join('\n') + }, { + code: [ + '', + ' ', + ' ', + '' + ].join('\n') + }, { + code: [ + '', + '', + '' + ].join('\n') + }, { + code: [ + '<', + 'App', + '>', + ' <', + ' Foo', + ' />', + '' + ].join('\n') + }], + + invalid: [{ + code: [ + '', + ' ', + '' + ].join('\n'), + output: [ + '', + ' ', + '', + '' + ].join('\n'), + errors: [{message: 'Opening tag for Element `Bar` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + ' ', + '' + ].join('\n'), + output: [ + '', + ' ', + '', + '' + ].join('\n'), + errors: [{message: 'Opening tag for Element `Bar` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + '' + ].join('\n'), + output: [ + '', + '', + '' + ].join('\n'), + errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + '' + ].join('\n'), + output: [ + '', + '', + '' + ].join('\n'), + errors: [{message: 'Opening tag for Element `Foo` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + '' + ].join('\n'), + output: [ + '', + '', + '' + ].join('\n'), + errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + '' + ].join('\n'), + output: [ + '', + '', + '' + ].join('\n'), + errors: [{message: 'Opening tag for Element `Foo` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + '' + ].join('\n'), + output: [ + '', + '', + '' + ].join('\n'), + errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + '' + ].join('\n'), + output: [ + '', + '', + '' + ].join('\n'), + errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + '' + ].join('\n'), + output: [ + '', + '', + '' + ].join('\n'), + errors: [{message: 'Opening tag for Element `Foo` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + ' ' + ].join('\n'), + output: [ + '', + ' ', + '' + ].join('\n'), + errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + ' ' + ].join('\n'), + output: [ + '', + ' ', + '' + ].join('\n'), + errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + ' ' + ].join('\n'), + output: [ + '', + ' ', + '' + ].join('\n'), + errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + parserOptions: parserOptions + }] +}); From 7f6784e094a6e6308433243f49c80a3c002b1385 Mon Sep 17 00:00:00 2001 From: TSMMark Date: Tue, 24 Oct 2017 23:36:43 -0400 Subject: [PATCH 02/14] require jsx-one-element-per-line in index.js --- index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/index.js b/index.js index 13b72fdd4d..db7d7a4adc 100644 --- a/index.js +++ b/index.js @@ -22,6 +22,7 @@ const allRules = { 'jsx-indent-props': require('./lib/rules/jsx-indent-props'), 'jsx-key': require('./lib/rules/jsx-key'), 'jsx-max-props-per-line': require('./lib/rules/jsx-max-props-per-line'), + 'jsx-one-element-per-line': require('./lib/rules/jsx-one-element-per-line'), 'jsx-no-bind': require('./lib/rules/jsx-no-bind'), 'jsx-no-comment-textnodes': require('./lib/rules/jsx-no-comment-textnodes'), 'jsx-no-duplicate-props': require('./lib/rules/jsx-no-duplicate-props'), From 256e5bc67aae21fd0f90602fa99a62661408dc9a Mon Sep 17 00:00:00 2001 From: TSMMark Date: Wed, 25 Oct 2017 00:32:37 -0400 Subject: [PATCH 03/14] fix sibling reporting --- lib/rules/jsx-one-element-per-line.js | 27 +++++---------------- tests/lib/rules/jsx-one-element-per-line.js | 16 ++++++++++++ 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-element-per-line.js index 3671c5d795..362b4fc80f 100644 --- a/lib/rules/jsx-one-element-per-line.js +++ b/lib/rules/jsx-one-element-per-line.js @@ -38,7 +38,6 @@ module.exports = { } const openingElementEndLine = node.loc.end.line; - const openingElementEndColumn = node.loc.end.column; // Children if (node.parent.children.length) { @@ -67,30 +66,16 @@ module.exports = { // Siblings if (node.parent.parent && node.parent.parent.children && node.parent.parent.children.length) { - const siblingsOnLine = []; - node.parent.parent.children.forEach(childNode => { - if ( - node.parent === childNode || - !childNode.openingElement || - childNode.openingElement.loc.start.line !== openingElementEndLine || - childNode.openingElement.loc.start.column < openingElementEndColumn - ) { - return; - } + const firstSibling = node.parent.parent.children.find(sibling => sibling.type === 'JSXElement'); - siblingsOnLine.push(childNode); - }); - - if (!siblingsOnLine.length) { + if (firstSibling === node.parent) { return; } - siblingsOnLine.forEach(elementInLine => { - context.report({ - node: elementInLine, - message: `Opening tag for Element \`${elementInLine.openingElement.name.name}\` must be placed on a new line`, - fix: generateFixFunction(elementInLine) - }); + context.report({ + node: node, + message: `Opening tag for Element \`${node.name.name}\` must be placed on a new line`, + fix: generateFixFunction(node) }); } }, diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js index 04ba7a21de..7143659971 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -242,5 +242,21 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ].join('\n'), errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], parserOptions: parserOptions + }, { + code: [ + '', + ' ', + '' + ].join('\n'), + output: [ + '', + ' ', + '', + '' + ].join('\n'), + errors: [{message: 'Opening tag for Element `Bar` must be placed on a new line'}], + parserOptions: parserOptions }] }); From 3893c3d3320a6a67db2cdb571590548c2010996e Mon Sep 17 00:00:00 2001 From: TSMMark Date: Wed, 25 Oct 2017 00:48:13 -0400 Subject: [PATCH 04/14] short-circuit childrenOnLine finder when a child is found --- lib/rules/jsx-one-element-per-line.js | 22 ++++++++++----------- tests/lib/rules/jsx-one-element-per-line.js | 16 +++++++++++++++ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-element-per-line.js index 362b4fc80f..58d766e385 100644 --- a/lib/rules/jsx-one-element-per-line.js +++ b/lib/rules/jsx-one-element-per-line.js @@ -39,6 +39,8 @@ module.exports = { const openingElementEndLine = node.loc.end.line; + // TODO: Just check if my parent opening tag is on the same line as me. + // Children if (node.parent.children.length) { const childrenOnLine = []; @@ -55,11 +57,11 @@ module.exports = { return; } - childrenOnLine.forEach(elementInLine => { + childrenOnLine.forEach(childOnLine => { context.report({ - node: elementInLine, - message: `Opening tag for Element \`${elementInLine.openingElement.name.name}\` must be placed on a new line`, - fix: generateFixFunction(elementInLine) + node: childOnLine, + message: `Opening tag for Element \`${childOnLine.openingElement.name.name}\` must be placed on a new line`, + fix: generateFixFunction(childOnLine) }); }); } @@ -85,11 +87,10 @@ module.exports = { return; } - const childrenOnLine = []; const closingElementStartLine = node.loc.end.line; - node.parent.children.forEach(childNode => { - const reportableLines = [childNode.openingElement, childNode.closingElement].reduce((lines, el) => { + const anyChildrenOnLine = node.parent.children.some(child => { + const reportableLines = [child.openingElement, child.closingElement].reduce((lines, el) => { if (!el) { return lines; } @@ -97,14 +98,11 @@ module.exports = { return lines.concat([el.loc.start.line, el.loc.end.line]); }, []); - if (reportableLines.indexOf(closingElementStartLine) === -1) { - return; - } - childrenOnLine.push(childNode); + return reportableLines.indexOf(closingElementStartLine) !== -1; }); - if (!childrenOnLine.length) { + if (!anyChildrenOnLine) { return; } diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js index 7143659971..4be28a97f7 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -258,5 +258,21 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ].join('\n'), errors: [{message: 'Opening tag for Element `Bar` must be placed on a new line'}], parserOptions: parserOptions + }, { + code: [ + '', + ' ', + ' ', + '' + ].join('\n'), + output: [ + '', + ' ', + ' ', + '', + '' + ].join('\n'), + errors: [{message: 'Closing tag for Element `Foo` must be placed on a new line'}], + parserOptions: parserOptions }] }); From 652683d612f951e9e2ff2a0d21f6ed1694ae2139 Mon Sep 17 00:00:00 2001 From: TSMMark Date: Wed, 25 Oct 2017 00:56:43 -0400 Subject: [PATCH 05/14] refactor rule to simply check parent line instead of all children --- lib/rules/jsx-one-element-per-line.js | 46 +++++++++------------------ 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-element-per-line.js index 58d766e385..f395f7e95a 100644 --- a/lib/rules/jsx-one-element-per-line.js +++ b/lib/rules/jsx-one-element-per-line.js @@ -37,48 +37,32 @@ module.exports = { return; } - const openingElementEndLine = node.loc.end.line; + const openingStartLine = node.loc.start.line; - // TODO: Just check if my parent opening tag is on the same line as me. + // Parent + if (node.parent.parent && node.parent.parent.openingElement) { + const parentOpeningStartLine = node.parent.parent.openingElement.loc.end.line; - // Children - if (node.parent.children.length) { - const childrenOnLine = []; - - node.parent.children.forEach(childNode => { - if (!childNode.openingElement || childNode.openingElement.loc.start.line !== openingElementEndLine) { - return; - } - - childrenOnLine.push(childNode); - }); - - if (!childrenOnLine.length) { - return; - } - - childrenOnLine.forEach(childOnLine => { + if (parentOpeningStartLine === openingStartLine) { context.report({ - node: childOnLine, - message: `Opening tag for Element \`${childOnLine.openingElement.name.name}\` must be placed on a new line`, - fix: generateFixFunction(childOnLine) + node: node, + message: `Opening tag for Element \`${node.name.name}\` must be placed on a new line`, + fix: generateFixFunction(node) }); - }); + } } // Siblings if (node.parent.parent && node.parent.parent.children && node.parent.parent.children.length) { const firstSibling = node.parent.parent.children.find(sibling => sibling.type === 'JSXElement'); - if (firstSibling === node.parent) { - return; + if (firstSibling !== node.parent) { + context.report({ + node: node, + message: `Opening tag for Element \`${node.name.name}\` must be placed on a new line`, + fix: generateFixFunction(node) + }); } - - context.report({ - node: node, - message: `Opening tag for Element \`${node.name.name}\` must be placed on a new line`, - fix: generateFixFunction(node) - }); } }, From a45e9c40f81f916386b01903c67c46c870ad8bda Mon Sep 17 00:00:00 2001 From: TSMMark Date: Wed, 25 Oct 2017 01:04:00 -0400 Subject: [PATCH 06/14] credit where credit is due --- lib/rules/jsx-one-element-per-line.js | 2 +- tests/lib/rules/jsx-one-element-per-line.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-element-per-line.js index f395f7e95a..add339deb7 100644 --- a/lib/rules/jsx-one-element-per-line.js +++ b/lib/rules/jsx-one-element-per-line.js @@ -1,6 +1,6 @@ /** * @fileoverview Limit to one element tag per line in JSX - * @author Mark Allen + * @author Mark Ivan Allen */ 'use strict'; diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js index 4be28a97f7..e23987fa47 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -1,6 +1,6 @@ /** * @fileoverview Limit to one element tag per line in JSX - * @author Mark Allen + * @author Mark Ivan Allen */ 'use strict'; From 8e81d904ed906c756a1513cc1ede4510c8b16d6f Mon Sep 17 00:00:00 2001 From: TSMMark Date: Wed, 25 Oct 2017 01:26:59 -0400 Subject: [PATCH 07/14] fix always reporting new line required for all siblings --- lib/rules/jsx-one-element-per-line.js | 38 ++++++++++++--------- tests/lib/rules/jsx-one-element-per-line.js | 7 ++++ 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-element-per-line.js index add339deb7..b0d2a8498e 100644 --- a/lib/rules/jsx-one-element-per-line.js +++ b/lib/rules/jsx-one-element-per-line.js @@ -31,6 +31,23 @@ module.exports = { }; } + function elementDoesOpenOrCloseOnLine (element, line) { + const reportableLines = [element.openingElement, element.closingElement].reduce((lines, el) => { + if (!el) { + return lines; + } + + return lines.concat([el.loc.start.line, el.loc.end.line]); + }, []); + + + return reportableLines.indexOf(line) !== -1; + } + + function anyElementDoesOpenOrCloseOnLine (elements, line) { + return elements.some(element => elementDoesOpenOrCloseOnLine(element, line)); + } + return { JSXOpeningElement: function (node) { if (!node.parent) { @@ -54,9 +71,11 @@ module.exports = { // Siblings if (node.parent.parent && node.parent.parent.children && node.parent.parent.children.length) { - const firstSibling = node.parent.parent.children.find(sibling => sibling.type === 'JSXElement'); + const firstSiblingOnLine = node.parent.parent.children.find(sibling => ( + sibling.type === 'JSXElement' && elementDoesOpenOrCloseOnLine(sibling, openingStartLine) + )); - if (firstSibling !== node.parent) { + if (firstSiblingOnLine !== node.parent) { context.report({ node: node, message: `Opening tag for Element \`${node.name.name}\` must be placed on a new line`, @@ -73,20 +92,7 @@ module.exports = { const closingElementStartLine = node.loc.end.line; - const anyChildrenOnLine = node.parent.children.some(child => { - const reportableLines = [child.openingElement, child.closingElement].reduce((lines, el) => { - if (!el) { - return lines; - } - - return lines.concat([el.loc.start.line, el.loc.end.line]); - }, []); - - - return reportableLines.indexOf(closingElementStartLine) !== -1; - }); - - if (!anyChildrenOnLine) { + if (!anyElementDoesOpenOrCloseOnLine(node.parent.children, closingElementStartLine)) { return; } diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js index e23987fa47..507bc37c10 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -43,6 +43,13 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ' ', '' ].join('\n') + }, { + code: [ + '', + ' ', + ' ', + '' + ].join('\n') }, { code: [ '', From e51c4cffc3a4676db68707320979ea6616d92ee0 Mon Sep 17 00:00:00 2001 From: TSMMark Date: Wed, 25 Oct 2017 02:13:32 -0400 Subject: [PATCH 08/14] add an invalid test case using html tags --- tests/lib/rules/jsx-one-element-per-line.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js index 507bc37c10..08329d2787 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -99,6 +99,20 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ].join('\n'), errors: [{message: 'Opening tag for Element `Bar` must be placed on a new line'}], parserOptions: parserOptions + }, { + code: [ + '
', + ' foo', + '
' + ].join('\n'), + output: [ + '
', + ' foo', + '', + '
' + ].join('\n'), + errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], + parserOptions: parserOptions }, { code: [ '', From 799dfa68accaaa50b38f01d176d96ad83c898864 Mon Sep 17 00:00:00 2001 From: TSMMark Date: Wed, 25 Oct 2017 15:16:21 -0400 Subject: [PATCH 09/14] failing tests describing how to handle non-element jsx nodes --- tests/lib/rules/jsx-one-element-per-line.js | 108 +++++++++++++++++++- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js index 08329d2787..6217a795c5 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -56,6 +56,12 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ' ', '' ].join('\n') + }, { + code: [ + '', + ' foo bar baz whatever ', + '' + ].join('\n') }, { code: [ '', @@ -102,17 +108,115 @@ ruleTester.run('jsx-max-elements-per-line', rule, { }, { code: [ '
', - ' foo', + ' foo', '
' ].join('\n'), output: [ '
', - ' foo', + ' ', + 'foo', + '
' + ].join('\n'), + errors: [{message: 'Literal `foo` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' {"foo"}', + '
' + ].join('\n'), + output: [ + '
', + ' ', + '{"foo"}', + '
' + ].join('\n'), + errors: [{message: 'Literal `{"foo"}` must be placed on a new line'}], // TODO: Replace `Literal` with proper term. + parserOptions: parserOptions + }, { + code: [ + '
', + ' foo', + '
' + ].join('\n'), + output: [ + '
', + ' foo', '', '
' ].join('\n'), errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], parserOptions: parserOptions + }, { + code: [ + '
', + ' {"foo"}', + '
' + ].join('\n'), + output: [ + '
', + ' {"foo"}', + '', + '
' + ].join('\n'), + errors: [{message: 'Opening tag for Element `span` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' foo ', + '
' + ].join('\n'), + errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' ', + '
' + ].join('\n'), + errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' foo', + '
' + ].join('\n'), + errors: [{message: 'Literal ` foo` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' {"foo"} ', + '
' + ].join('\n'), + errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' {"foo"} bar', + '
' + ].join('\n'), + errors: [{message: 'Literal `bar` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' foo {"bar"}', + '
' + ].join('\n'), + errors: [{message: 'Literal `{"bar"}` must be placed on a new line'}], // TODO: Replace `Literal` with proper term. + parserOptions: parserOptions + }, { + code: [ + '
', + ' {"foo"}', + '
' + ].join('\n'), + errors: [{message: 'Literal `{"foo"}` must be placed on a new line'}], // TODO: Replace `Literal` with proper term. + parserOptions: parserOptions }, { code: [ '', From 7bdc8562d0aa975105ba8b400a90b69e8e930034 Mon Sep 17 00:00:00 2001 From: TSMMark Date: Fri, 27 Oct 2017 00:43:40 -0400 Subject: [PATCH 10/14] rework internals to check children of JSXElement --- lib/rules/jsx-one-element-per-line.js | 158 ++++--- tests/lib/rules/jsx-one-element-per-line.js | 497 ++++++++++++++++++-- 2 files changed, 559 insertions(+), 96 deletions(-) diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-element-per-line.js index b0d2a8498e..9c67e3319d 100644 --- a/lib/rules/jsx-one-element-per-line.js +++ b/lib/rules/jsx-one-element-per-line.js @@ -23,83 +23,111 @@ module.exports = { create: function (context) { const sourceCode = context.getSourceCode(); - function generateFixFunction(elementInLine) { - const nodeText = sourceCode.getText(elementInLine); - - return function(fixer) { - return fixer.replaceText(elementInLine, `\n${nodeText}`); - }; - } - - function elementDoesOpenOrCloseOnLine (element, line) { - const reportableLines = [element.openingElement, element.closingElement].reduce((lines, el) => { - if (!el) { - return lines; - } - - return lines.concat([el.loc.start.line, el.loc.end.line]); - }, []); - - - return reportableLines.indexOf(line) !== -1; - } - - function anyElementDoesOpenOrCloseOnLine (elements, line) { - return elements.some(element => elementDoesOpenOrCloseOnLine(element, line)); - } - return { - JSXOpeningElement: function (node) { - if (!node.parent) { + JSXElement: function (node) { + const children = node.children; + + if (!children || !children.length) { return; } - const openingStartLine = node.loc.start.line; + const openingElement = node.openingElement; + const closingElement = node.closingElement; + const openingElementEndLine = openingElement.loc.end.line; + const closingElementStartLine = closingElement.loc.start.line; - // Parent - if (node.parent.parent && node.parent.parent.openingElement) { - const parentOpeningStartLine = node.parent.parent.openingElement.loc.end.line; + const childrenGroupedByLine = {}; - if (parentOpeningStartLine === openingStartLine) { - context.report({ - node: node, - message: `Opening tag for Element \`${node.name.name}\` must be placed on a new line`, - fix: generateFixFunction(node) - }); - } - } + children.forEach(child => { + let countNewLinesBeforeContent = 0; + let countNewLinesAfterContent = 0; - // Siblings - if (node.parent.parent && node.parent.parent.children && node.parent.parent.children.length) { - const firstSiblingOnLine = node.parent.parent.children.find(sibling => ( - sibling.type === 'JSXElement' && elementDoesOpenOrCloseOnLine(sibling, openingStartLine) - )); + if (child.type === 'Literal') { + if (child.value.match(/^\s*$/)) { + return; + } - if (firstSiblingOnLine !== node.parent) { - context.report({ - node: node, - message: `Opening tag for Element \`${node.name.name}\` must be placed on a new line`, - fix: generateFixFunction(node) - }); + countNewLinesBeforeContent = (child.raw.match(/^ *\n/g) || []).length; + countNewLinesAfterContent = (child.raw.match(/\n *$/g) || []).length; } - } - }, - - JSXClosingElement: function (node) { - if (!node.parent || !node.parent.children.length) { - return; - } - const closingElementStartLine = node.loc.end.line; + const startLine = child.loc.start.line + countNewLinesBeforeContent; + const endLine = child.loc.end.line - countNewLinesAfterContent; + + if (startLine === endLine) { + if (!childrenGroupedByLine[startLine]) { + childrenGroupedByLine[startLine] = []; + } + childrenGroupedByLine[startLine].push(child); + } else { + if (!childrenGroupedByLine[startLine]) { + childrenGroupedByLine[startLine] = []; + } + childrenGroupedByLine[startLine].push(child); + if (!childrenGroupedByLine[endLine]) { + childrenGroupedByLine[endLine] = []; + } + childrenGroupedByLine[endLine].push(child); + } + }); - if (!anyElementDoesOpenOrCloseOnLine(node.parent.children, closingElementStartLine)) { - return; - } + Object.keys(childrenGroupedByLine).forEach(line => { + line = parseInt(line, 10); + const firstIndex = 0; + const lastIndex = childrenGroupedByLine[line].length - 1; + + childrenGroupedByLine[line].forEach((child, i) => { + let prevChild; + if (i === firstIndex) { + if (line === openingElementEndLine) { + prevChild = openingElement; + } + } else { + prevChild = childrenGroupedByLine[line][i - 1]; + } + let nextChild; + if (i === lastIndex) { + if (line === closingElementStartLine) { + nextChild = closingElement; + } + } else { + // We don't need to append a trailing because the next child will prepend a leading. + // nextChild = childrenGroupedByLine[line][i + 1]; + } + + function spaceBetweenPrev () { + return (prevChild.type === 'Literal' && prevChild.raw.match(/ $/)) || + (child.type === 'Literal' && child.raw.match(/^ /)) || + sourceCode.isSpaceBetweenTokens(prevChild, child); + } + function spaceBetweenNext () { + return (nextChild.type === 'Literal' && nextChild.raw.match(/^ /)) || + (child.type === 'Literal' && child.raw.match(/ $/)) || + sourceCode.isSpaceBetweenTokens(child, nextChild); + } + const leadingSpace = prevChild && spaceBetweenPrev() ? '\n{\' \'}' : ''; + const trailingSpace = nextChild && spaceBetweenNext() ? '{\' \'}\n' : ''; + const leadingNewLine = prevChild ? '\n' : ''; + const trailingNewLine = nextChild ? '\n' : ''; + + if (!prevChild && !nextChild) { + return; + } + + const source = sourceCode.getText(child); + + function nodeDescriptor (n) { + return n.openingElement ? n.openingElement.name.name : source.replace(/\n/g, ''); + } - context.report({ - node: node, - message: `Closing tag for Element \`${node.name.name}\` must be placed on a new line`, - fix: generateFixFunction(node) + context.report({ + node: child, + message: `\`${nodeDescriptor(child)}\` must be placed on a new line`, + fix: function (fixer) { + return fixer.replaceText(child, `${leadingSpace}${leadingNewLine}${source}${trailingNewLine}${trailingSpace}`); + } + }); + }); }); } }; diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js index 6217a795c5..820a841c76 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -26,15 +26,11 @@ const parserOptions = { // ------------------------------------------------------------------------------ const ruleTester = new RuleTester({parserOptions}); -ruleTester.run('jsx-max-elements-per-line', rule, { +ruleTester.run('jsx-one-element-per-line', rule, { valid: [{ code: '' }, { code: '' - }, { - code: '{"foo"}' - }, { - code: 'foo' }, { code: '' }, { @@ -92,6 +88,58 @@ ruleTester.run('jsx-max-elements-per-line', rule, { }], invalid: [{ + code: '{"foo"}', + output: [ + '', + '{"foo"}', + '' + ].join('\n'), + errors: [{message: '`{"foo"}` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: 'foo', + output: [ + '', + 'foo', + '' + ].join('\n'), + errors: [{message: '`foo` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' foo {"bar"}', + '
' + ].join('\n'), + output: [ + '
', + ' foo ', + '{\' \'}', + '{"bar"}', + '
' + ].join('\n'), + errors: [ + {message: '`{"bar"}` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + code: [ + '
', + ' {"foo"} bar', + '
' + ].join('\n'), + output: [ + '
', + ' {"foo"}', + '{\' \'}', + ' bar', + '
' + ].join('\n'), + errors: [ + {message: '` bar` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { code: [ '', ' ', @@ -103,7 +151,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `Bar` must be placed on a new line'}], + errors: [{message: '`Bar` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -117,7 +165,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { 'foo', '' ].join('\n'), - errors: [{message: 'Literal `foo` must be placed on a new line'}], + errors: [{message: '`foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -131,7 +179,148 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '{"foo"}', '' ].join('\n'), - errors: [{message: 'Literal `{"foo"}` must be placed on a new line'}], // TODO: Replace `Literal` with proper term. + errors: [{message: '`{"foo"}` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' {"foo"} { I18n.t(\'baz\') }', + '
' + ].join('\n'), + output: [ + '
', + ' {"foo"} ', + '{\' \'}', + '{ I18n.t(\'baz\') }', + '
' + ].join('\n'), + errors: [ + {message: '`{ I18n.t(\'baz\') }` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + code: [ + '{ bar } { I18n.t(\'baz\') }' + ].join('\n'), + output: [ + '', + '{ bar } ', + '{\' \'}', + ' ', + '{\' \'}', + '{ I18n.t(\'baz\') }', + '' + ].join('\n'), + errors: [ + {message: '`{ bar }` must be placed on a new line'}, + {message: '`Text` must be placed on a new line'}, + {message: '`{ I18n.t(\'baz\') }` must be placed on a new line'} + ], + parserOptions: parserOptions + + }, { + code: [ + ' ' + ].join('\n'), + output: [ + ' ', + '{\' \'}', + ' ', + '{\' \'}', + '', + '' + ].join('\n'), + errors: [ + {message: '`Bar` must be placed on a new line'}, + {message: '`Baz` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + code: [ + ' ' + ].join('\n'), + output: [ + ' ', + '{\' \'}', + ' ', + '{\' \'}', + ' ', + '{\' \'}', + ' ', + '{\' \'}', + '', + '{\' \'}', + ' ' + ].join('\n'), + errors: [ + {message: '`Bar` must be placed on a new line'}, + {message: '`Baz` must be placed on a new line'}, + {message: '`Bunk` must be placed on a new line'}, + {message: '`Bruno` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + code: [ + ' ' + ].join('\n'), + output: [ + ' ', + '{\' \'}', + '', + '' + ].join('\n'), + errors: [ + {message: '`Bar` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + code: [ + ' ', + '' + ].join('\n'), + output: [ + ' ', + '{\' \'}', + '', + '' + ].join('\n'), + errors: [ + {message: '`Bar` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + code: [ + '', + ' ', + '' + ].join('\n'), + output: [ + '', + ' ', + '{\' \'}', + '', + '' + ].join('\n'), + errors: [ + {message: '`Baz` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + code: [ + '', + ' { bar } { I18n.t(\'baz\') }', + '' + ].join('\n'), + output: [ + '', + ' { bar } ', + '{\' \'}', + '{ I18n.t(\'baz\') }', + '' + ].join('\n'), + errors: [ + {message: '`{ I18n.t(\'baz\') }` must be placed on a new line'} + ], parserOptions: parserOptions }, { code: [ @@ -145,7 +334,9 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], + errors: [ + {message: '`input` must be placed on a new line'} + ], parserOptions: parserOptions }, { code: [ @@ -159,7 +350,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `span` must be placed on a new line'}], + errors: [{message: '`span` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -167,7 +358,29 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ' foo ', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], + output: [ + '
', + ' foo ', + '{\' \'}', + '', + '
' + ].join('\n'), + errors: [{message: '`input` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '
', + ' foo', + '
' + ].join('\n'), + output: [ + '
', + ' ', + '{\' \'}', + ' foo', + '
' + ].join('\n'), + errors: [{message: '` foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -175,15 +388,34 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ' ', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], + output: [ + '
', + ' ', + '{\' \'}', + '', + '
' + ].join('\n'), + errors: [ + {message: '`input` must be placed on a new line'} + ], parserOptions: parserOptions }, { code: [ '
', - ' foo', + ' ', + '{\' \'}', + '
' + ].join('\n'), + output: [ + '
', + ' ', + '{\' \'}', + '', '
' ].join('\n'), - errors: [{message: 'Literal ` foo` must be placed on a new line'}], + errors: [ + {message: '`input` must be placed on a new line'} + ], parserOptions: parserOptions }, { code: [ @@ -191,7 +423,16 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ' {"foo"} ', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `input` must be placed on a new line'}], + output: [ + '
', + ' {"foo"} ', + '{\' \'}', + '', + '
' + ].join('\n'), + errors: [ + {message: '`input` must be placed on a new line'} + ], parserOptions: parserOptions }, { code: [ @@ -199,7 +440,14 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ' {"foo"} bar', '' ].join('\n'), - errors: [{message: 'Literal `bar` must be placed on a new line'}], + output: [ + '
', + ' {"foo"}', + '{\' \'}', + ' bar', + '
' + ].join('\n'), + errors: [{message: '` bar` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -207,7 +455,16 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ' foo {"bar"}', '' ].join('\n'), - errors: [{message: 'Literal `{"bar"}` must be placed on a new line'}], // TODO: Replace `Literal` with proper term. + output: [ + '
', + ' foo ', + '{\' \'}', + '{"bar"}', + '
' + ].join('\n'), + errors: [ + {message: '`{"bar"}` must be placed on a new line'} + ], parserOptions: parserOptions }, { code: [ @@ -215,7 +472,16 @@ ruleTester.run('jsx-max-elements-per-line', rule, { ' {"foo"}', '' ].join('\n'), - errors: [{message: 'Literal `{"foo"}` must be placed on a new line'}], // TODO: Replace `Literal` with proper term. + output: [ + '
', + ' ', + '{\' \'}', + '{"foo"}', + '
' + ].join('\n'), + errors: [ + {message: '`{"foo"}` must be placed on a new line'} + ], parserOptions: parserOptions }, { code: [ @@ -229,7 +495,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `Bar` must be placed on a new line'}], + errors: [{message: '`Bar` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -241,7 +507,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -253,7 +519,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `Foo` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -265,7 +531,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -279,7 +545,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '/>', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `Foo` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -293,7 +559,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -309,7 +575,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '/>', '' ].join('\n'), - errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -323,7 +589,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `Foo` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -337,7 +603,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '>', '' ].join('\n'), - errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -351,7 +617,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { 'Foo>', '' ].join('\n'), - errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -365,7 +631,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { 'Foo>', '' ].join('\n'), - errors: [{message: 'Closing tag for Element `App` must be placed on a new line'}], + errors: [{message: '`Foo` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -381,7 +647,7 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Opening tag for Element `Bar` must be placed on a new line'}], + errors: [{message: '`Bar` must be placed on a new line'}], parserOptions: parserOptions }, { code: [ @@ -397,7 +663,176 @@ ruleTester.run('jsx-max-elements-per-line', rule, { '', '' ].join('\n'), - errors: [{message: 'Closing tag for Element `Foo` must be placed on a new line'}], + errors: [{message: '`Bar` must be placed on a new line'}], + parserOptions: parserOptions + }, { + code: [ + '', + ' ', + ' baz ', + ' ', + '' + ].join('\n'), + output: [ + '', + ' ', + ' ', + '{\' \'}', + ' baz ', + '{\' \'}', + '', + ' ', + '' + ].join('\n'), + errors: [ + {message: '` baz ` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + '', + ' foo {"bar"} baz', + '' + ].join('\n'), + output: [ + '', + ' foo ', + '{\' \'}', + '{"bar"} baz', + '' + ].join('\n'), + errors: [ + {message: '`{"bar"}` must be placed on a new line'}, + {message: '` baz` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + '', + '', + ' foo {"bar"} baz', + '', + '' + ].join('\n'), + output: [ + '', + '', + ' foo ', + '{\' \'}', + '{"bar"} baz', + '', + '' + ].join('\n'), + errors: [ + {message: '`{"bar"}` must be placed on a new line'}, + {message: '` baz` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + '', + '', + ' foo ', + '{\' \'}', + '{"bar"} baz', + '', + '' + ].join('\n'), + output: [ + '', + '', + ' foo ', + '{\' \'}', + '{"bar"}', + '{\' \'}', + ' baz', + '', + '' + ].join('\n'), + errors: [ + {message: '` baz` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + '{', + ' foo', + '}' + ].join('\n'), + output: [ + '', + '{', + ' foo', + '}' + ].join('\n'), + errors: [ + {message: '`{ foo}` must be placed on a new line'}, + {message: '`{ foo}` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + '', + '{', + ' foo', + '}' + ].join('\n'), + output: [ + '', + '{', + ' foo', + '}', + '' + ].join('\n'), + errors: [ + {message: '`{ foo}` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + ' {', + ' foo', + '} ' + ].join('\n'), + output: [ + ' ', + '{\' \'}', + '{', + ' foo', + '} ' + ].join('\n'), + errors: [ + {message: '`{ foo}` must be placed on a new line'}, + {message: '`{ foo}` must be placed on a new line'} + ], + parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + ' ', + '{\' \'}', + '{', + ' foo', + '} ' + ].join('\n'), + output: [ + ' ', + '{\' \'}', + '{', + ' foo', + '}', + '{\' \'}', + ' ' + ].join('\n'), + errors: [ + {message: '`{ foo}` must be placed on a new line'} + ], parserOptions: parserOptions }] }); From cb5f4476d83c4c6e5b7e7ce58b5413941d9ba933 Mon Sep 17 00:00:00 2001 From: TSMMark Date: Fri, 27 Oct 2017 12:28:15 -0400 Subject: [PATCH 11/14] handle certain cases in one pass --- lib/rules/jsx-one-element-per-line.js | 67 +++++++++++++++++---- tests/lib/rules/jsx-one-element-per-line.js | 49 +++++++-------- 2 files changed, 79 insertions(+), 37 deletions(-) diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-element-per-line.js index 9c67e3319d..b2b510734e 100644 --- a/lib/rules/jsx-one-element-per-line.js +++ b/lib/rules/jsx-one-element-per-line.js @@ -23,6 +23,14 @@ module.exports = { create: function (context) { const sourceCode = context.getSourceCode(); + function nodeKey (node) { + return `${node.loc.start.line},${node.loc.start.column}`; + } + + function nodeDescriptor (n) { + return n.openingElement ? n.openingElement.name.name : sourceCode.getText(n).replace(/\n/g, ''); + } + return { JSXElement: function (node) { const children = node.children; @@ -37,6 +45,7 @@ module.exports = { const closingElementStartLine = closingElement.loc.start.line; const childrenGroupedByLine = {}; + const fixDetailsByNode = {}; children.forEach(child => { let countNewLinesBeforeContent = 0; @@ -105,28 +114,60 @@ module.exports = { (child.type === 'Literal' && child.raw.match(/ $/)) || sourceCode.isSpaceBetweenTokens(child, nextChild); } - const leadingSpace = prevChild && spaceBetweenPrev() ? '\n{\' \'}' : ''; - const trailingSpace = nextChild && spaceBetweenNext() ? '{\' \'}\n' : ''; - const leadingNewLine = prevChild ? '\n' : ''; - const trailingNewLine = nextChild ? '\n' : ''; if (!prevChild && !nextChild) { return; } const source = sourceCode.getText(child); + const leadingSpace = !!(prevChild && spaceBetweenPrev()); + const trailingSpace = !!(nextChild && spaceBetweenNext()); + const leadingNewLine = !!(prevChild); + const trailingNewLine = !!(nextChild); + + const key = nodeKey(child); + + if (!fixDetailsByNode[key]) { + fixDetailsByNode[key] = { + node: child, + source: source, + descriptor: nodeDescriptor(child) + }; + } - function nodeDescriptor (n) { - return n.openingElement ? n.openingElement.name.name : source.replace(/\n/g, ''); + if (leadingSpace) { + fixDetailsByNode[key].leadingSpace = true; + } + if (leadingNewLine) { + fixDetailsByNode[key].leadingNewLine = true; } + if (trailingNewLine) { + fixDetailsByNode[key].trailingNewLine = true; + } + if (trailingSpace) { + fixDetailsByNode[key].trailingSpace = true; + } + }); + }); - context.report({ - node: child, - message: `\`${nodeDescriptor(child)}\` must be placed on a new line`, - fix: function (fixer) { - return fixer.replaceText(child, `${leadingSpace}${leadingNewLine}${source}${trailingNewLine}${trailingSpace}`); - } - }); + Object.keys(fixDetailsByNode).forEach(key => { + const details = fixDetailsByNode[key]; + + const nodeToReport = details.node; + const descriptor = details.descriptor; + const source = details.source; + + const leadingSpaceString = details.leadingSpace ? '\n{\' \'}' : ''; + const trailingSpaceString = details.trailingSpace ? '{\' \'}\n' : ''; + const leadingNewLineString = details.leadingNewLine ? '\n' : ''; + const trailingNewLineString = details.trailingNewLine ? '\n' : ''; + + context.report({ + node: nodeToReport, + message: `\`${descriptor}\` must be placed on a new line`, + fix: function (fixer) { + return fixer.replaceText(nodeToReport, `${leadingSpaceString}${leadingNewLineString}${source}${trailingNewLineString}${trailingSpaceString}`); + } }); }); } diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-element-per-line.js index 820a841c76..f0131ad642 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-element-per-line.js @@ -707,6 +707,28 @@ ruleTester.run('jsx-one-element-per-line', rule, { {message: '` baz` must be placed on a new line'} ], parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + '', + ' foo ', + '{\' \'}', + '{"bar"} baz', + '' + ].join('\n'), + output: [ + '', + ' foo ', + '{\' \'}', + '{"bar"}', + '{\' \'}', + ' baz', + '' + ].join('\n'), + errors: [ + {message: '` baz` must be placed on a new line'} + ], + parserOptions: parserOptions }, { // Would be nice to handle in one pass, but multipass works fine. code: [ @@ -757,31 +779,11 @@ ruleTester.run('jsx-one-element-per-line', rule, { ], parserOptions: parserOptions }, { - // Would be nice to handle in one pass, but multipass works fine. code: [ '{', ' foo', '}' ].join('\n'), - output: [ - '', - '{', - ' foo', - '}' - ].join('\n'), - errors: [ - {message: '`{ foo}` must be placed on a new line'}, - {message: '`{ foo}` must be placed on a new line'} - ], - parserOptions: parserOptions - }, { - // Would be nice to handle in one pass, but multipass works fine. - code: [ - '', - '{', - ' foo', - '}' - ].join('\n'), output: [ '', '{', @@ -794,7 +796,6 @@ ruleTester.run('jsx-one-element-per-line', rule, { ], parserOptions: parserOptions }, { - // Would be nice to handle in one pass, but multipass works fine. code: [ ' {', ' foo', @@ -805,15 +806,15 @@ ruleTester.run('jsx-one-element-per-line', rule, { '{\' \'}', '{', ' foo', - '} ' + '}', + '{\' \'}', + ' ' ].join('\n'), errors: [ - {message: '`{ foo}` must be placed on a new line'}, {message: '`{ foo}` must be placed on a new line'} ], parserOptions: parserOptions }, { - // Would be nice to handle in one pass, but multipass works fine. code: [ ' ', '{\' \'}', From a18c30b3b2c0c7be23614c165ac9d27b5ce59ba2 Mon Sep 17 00:00:00 2001 From: TSMMark Date: Fri, 27 Oct 2017 14:41:44 -0400 Subject: [PATCH 12/14] rename to jsx-one-expression-per-line Now that we handle all nodes instead of only React elements, this name seems more fitting. --- index.js | 2 +- ...one-element-per-line.js => jsx-one-expression-per-line.js} | 0 ...one-element-per-line.js => jsx-one-expression-per-line.js} | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) rename lib/rules/{jsx-one-element-per-line.js => jsx-one-expression-per-line.js} (100%) rename tests/lib/rules/{jsx-one-element-per-line.js => jsx-one-expression-per-line.js} (99%) diff --git a/index.js b/index.js index db7d7a4adc..8a8df3b788 100644 --- a/index.js +++ b/index.js @@ -22,7 +22,7 @@ const allRules = { 'jsx-indent-props': require('./lib/rules/jsx-indent-props'), 'jsx-key': require('./lib/rules/jsx-key'), 'jsx-max-props-per-line': require('./lib/rules/jsx-max-props-per-line'), - 'jsx-one-element-per-line': require('./lib/rules/jsx-one-element-per-line'), + 'jsx-one-expression-per-line': require('./lib/rules/jsx-one-expression-per-line'), 'jsx-no-bind': require('./lib/rules/jsx-no-bind'), 'jsx-no-comment-textnodes': require('./lib/rules/jsx-no-comment-textnodes'), 'jsx-no-duplicate-props': require('./lib/rules/jsx-no-duplicate-props'), diff --git a/lib/rules/jsx-one-element-per-line.js b/lib/rules/jsx-one-expression-per-line.js similarity index 100% rename from lib/rules/jsx-one-element-per-line.js rename to lib/rules/jsx-one-expression-per-line.js diff --git a/tests/lib/rules/jsx-one-element-per-line.js b/tests/lib/rules/jsx-one-expression-per-line.js similarity index 99% rename from tests/lib/rules/jsx-one-element-per-line.js rename to tests/lib/rules/jsx-one-expression-per-line.js index f0131ad642..49607d9d79 100644 --- a/tests/lib/rules/jsx-one-element-per-line.js +++ b/tests/lib/rules/jsx-one-expression-per-line.js @@ -9,7 +9,7 @@ // Requirements // ------------------------------------------------------------------------------ -const rule = require('../../../lib/rules/jsx-one-element-per-line'); +const rule = require('../../../lib/rules/jsx-one-expression-per-line'); const RuleTester = require('eslint').RuleTester; const parserOptions = { @@ -26,7 +26,7 @@ const parserOptions = { // ------------------------------------------------------------------------------ const ruleTester = new RuleTester({parserOptions}); -ruleTester.run('jsx-one-element-per-line', rule, { +ruleTester.run('jsx-one-expression-per-line', rule, { valid: [{ code: '' }, { From 1344f1dad2478d15a378d681128206bf969c733b Mon Sep 17 00:00:00 2001 From: TSMMark Date: Sat, 28 Oct 2017 13:15:14 -0400 Subject: [PATCH 13/14] strip spaces but leave line returns --- lib/rules/jsx-one-expression-per-line.js | 11 +++++-- .../lib/rules/jsx-one-expression-per-line.js | 30 +++++++++++++++---- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/lib/rules/jsx-one-expression-per-line.js b/lib/rules/jsx-one-expression-per-line.js index b2b510734e..a0f12a0565 100644 --- a/lib/rules/jsx-one-expression-per-line.js +++ b/lib/rules/jsx-one-expression-per-line.js @@ -87,6 +87,8 @@ module.exports = { childrenGroupedByLine[line].forEach((child, i) => { let prevChild; + let nextChild; + if (i === firstIndex) { if (line === openingElementEndLine) { prevChild = openingElement; @@ -94,7 +96,7 @@ module.exports = { } else { prevChild = childrenGroupedByLine[line][i - 1]; } - let nextChild; + if (i === lastIndex) { if (line === closingElementStartLine) { nextChild = closingElement; @@ -109,6 +111,7 @@ module.exports = { (child.type === 'Literal' && child.raw.match(/^ /)) || sourceCode.isSpaceBetweenTokens(prevChild, child); } + function spaceBetweenNext () { return (nextChild.type === 'Literal' && nextChild.raw.match(/^ /)) || (child.type === 'Literal' && child.raw.match(/ $/)) || @@ -155,18 +158,20 @@ module.exports = { const nodeToReport = details.node; const descriptor = details.descriptor; - const source = details.source; + const source = details.source.replace(/(^ +| +(?=\n)*$)/g, ''); const leadingSpaceString = details.leadingSpace ? '\n{\' \'}' : ''; const trailingSpaceString = details.trailingSpace ? '{\' \'}\n' : ''; const leadingNewLineString = details.leadingNewLine ? '\n' : ''; const trailingNewLineString = details.trailingNewLine ? '\n' : ''; + const replaceText = `${leadingSpaceString}${leadingNewLineString}${source}${trailingNewLineString}${trailingSpaceString}`; + context.report({ node: nodeToReport, message: `\`${descriptor}\` must be placed on a new line`, fix: function (fixer) { - return fixer.replaceText(nodeToReport, `${leadingSpaceString}${leadingNewLineString}${source}${trailingNewLineString}${trailingSpaceString}`); + return fixer.replaceText(nodeToReport, replaceText); } }); }); diff --git a/tests/lib/rules/jsx-one-expression-per-line.js b/tests/lib/rules/jsx-one-expression-per-line.js index 49607d9d79..ad9b3baf3f 100644 --- a/tests/lib/rules/jsx-one-expression-per-line.js +++ b/tests/lib/rules/jsx-one-expression-per-line.js @@ -132,7 +132,7 @@ ruleTester.run('jsx-one-expression-per-line', rule, { '
', ' {"foo"}', '{\' \'}', - ' bar', + 'bar', '
' ].join('\n'), errors: [ @@ -377,7 +377,7 @@ ruleTester.run('jsx-one-expression-per-line', rule, { '
', ' ', '{\' \'}', - ' foo', + 'foo', '
' ].join('\n'), errors: [{message: '` foo` must be placed on a new line'}], @@ -444,7 +444,7 @@ ruleTester.run('jsx-one-expression-per-line', rule, { '
', ' {"foo"}', '{\' \'}', - ' bar', + 'bar', '
' ].join('\n'), errors: [{message: '` bar` must be placed on a new line'}], @@ -678,7 +678,7 @@ ruleTester.run('jsx-one-expression-per-line', rule, { ' ', ' ', '{\' \'}', - ' baz ', + 'baz', '{\' \'}', '', ' ', @@ -707,6 +707,24 @@ ruleTester.run('jsx-one-expression-per-line', rule, { {message: '` baz` must be placed on a new line'} ], parserOptions: parserOptions + }, { + // Would be nice to handle in one pass, but multipass works fine. + code: [ + '', + ' foo {"bar"}', + '' + ].join('\n'), + output: [ + '', + ' foo ', + '{\' \'}', + '{"bar"}', + '' + ].join('\n'), + errors: [ + {message: '`{"bar"}` must be placed on a new line'} + ], + parserOptions: parserOptions }, { // Would be nice to handle in one pass, but multipass works fine. code: [ @@ -722,7 +740,7 @@ ruleTester.run('jsx-one-expression-per-line', rule, { '{\' \'}', '{"bar"}', '{\' \'}', - ' baz', + 'baz', '' ].join('\n'), errors: [ @@ -770,7 +788,7 @@ ruleTester.run('jsx-one-expression-per-line', rule, { '{\' \'}', '{"bar"}', '{\' \'}', - ' baz', + 'baz', '', '' ].join('\n'), From cd3648c069b646caa79256a6e65f24f64306235c Mon Sep 17 00:00:00 2001 From: TSMMark Date: Mon, 30 Oct 2017 15:57:44 -0400 Subject: [PATCH 14/14] use better practices thanks @lencioni --- lib/rules/jsx-one-expression-per-line.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/rules/jsx-one-expression-per-line.js b/lib/rules/jsx-one-expression-per-line.js index a0f12a0565..0ee1746232 100644 --- a/lib/rules/jsx-one-expression-per-line.js +++ b/lib/rules/jsx-one-expression-per-line.js @@ -52,7 +52,7 @@ module.exports = { let countNewLinesAfterContent = 0; if (child.type === 'Literal') { - if (child.value.match(/^\s*$/)) { + if (/^\s*$/.test(child.raw)) { return; } @@ -80,8 +80,8 @@ module.exports = { } }); - Object.keys(childrenGroupedByLine).forEach(line => { - line = parseInt(line, 10); + Object.keys(childrenGroupedByLine).forEach(_line => { + const line = parseInt(_line, 10); const firstIndex = 0; const lastIndex = childrenGroupedByLine[line].length - 1; @@ -107,14 +107,14 @@ module.exports = { } function spaceBetweenPrev () { - return (prevChild.type === 'Literal' && prevChild.raw.match(/ $/)) || - (child.type === 'Literal' && child.raw.match(/^ /)) || + return (prevChild.type === 'Literal' && / $/.test(prevChild.raw)) || + (child.type === 'Literal' && /^ /.test(child.raw)) || sourceCode.isSpaceBetweenTokens(prevChild, child); } function spaceBetweenNext () { - return (nextChild.type === 'Literal' && nextChild.raw.match(/^ /)) || - (child.type === 'Literal' && child.raw.match(/ $/)) || + return (nextChild.type === 'Literal' && /^ /.test(nextChild.raw)) || + (child.type === 'Literal' && / $/.test(child.raw)) || sourceCode.isSpaceBetweenTokens(child, nextChild); } @@ -125,8 +125,8 @@ module.exports = { const source = sourceCode.getText(child); const leadingSpace = !!(prevChild && spaceBetweenPrev()); const trailingSpace = !!(nextChild && spaceBetweenNext()); - const leadingNewLine = !!(prevChild); - const trailingNewLine = !!(nextChild); + const leadingNewLine = !!prevChild; + const trailingNewLine = !!nextChild; const key = nodeKey(child);