From 52651261edbb584d7372120daa3cfdcc42e0df7e Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Sat, 1 Oct 2016 11:40:56 +0800 Subject: [PATCH 01/11] jsx-first-pro-new-line autofix implementation Test should ignore indent for now --- lib/rules/jsx-first-prop-new-line.js | 11 +++++++++-- tests/lib/rules/jsx-first-prop-new-line.js | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/rules/jsx-first-prop-new-line.js b/lib/rules/jsx-first-prop-new-line.js index 981dc62e7e..ef07809f99 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'] @@ -39,7 +40,10 @@ module.exports = { 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.insertTextAfter(node.name, '\n'); + } }); } }); @@ -48,7 +52,10 @@ module.exports = { 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; } diff --git a/tests/lib/rules/jsx-first-prop-new-line.js b/tests/lib/rules/jsx-first-prop-new-line.js index 9feb9cfd5d..ab54ecbcd8 100644 --- a/tests/lib/rules/jsx-first-prop-new-line.js +++ b/tests/lib/rules/jsx-first-prop-new-line.js @@ -153,6 +153,10 @@ ruleTester.run('jsx-first-prop-new-line', rule, { invalid: [ { code: '', + output: [ + '' + ].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: [ + '' + ].join('\n'), options: ['always'], errors: [{message: 'Property should be placed on a new line'}], parser: parserOptions @@ -174,6 +184,11 @@ ruleTester.run('jsx-first-prop-new-line', rule, { ' propTwo="two"', '/>' ].join('\n'), + output: [ + '' + ].join('\n'), options: ['never'], errors: [{message: 'Property should be placed on the same line as the component declaration'}], parser: parserOptions From e441aae63e920d8782114e55b90771ea6a5e6a27 Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Mon, 3 Oct 2016 12:08:31 +0800 Subject: [PATCH 02/11] Account for newline indents - Adjust old cases to use default 2 indents - Add new case to use optional rule for 4 indents --- tests/lib/rules/jsx-first-prop-new-line.js | 26 +++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/lib/rules/jsx-first-prop-new-line.js b/tests/lib/rules/jsx-first-prop-new-line.js index ab54ecbcd8..c1a3cf65b8 100644 --- a/tests/lib/rules/jsx-first-prop-new-line.js +++ b/tests/lib/rules/jsx-first-prop-new-line.js @@ -155,7 +155,7 @@ ruleTester.run('jsx-first-prop-new-line', rule, { code: '', output: [ '' + ' prop="one" />' ].join('\n'), options: ['always'], errors: [{message: 'Property should be placed on a new line'}], @@ -169,7 +169,7 @@ ruleTester.run('jsx-first-prop-new-line', rule, { ].join('\n'), output: [ '' ].join('\n'), @@ -177,16 +177,32 @@ ruleTester.run('jsx-first-prop-new-line', rule, { errors: [{message: 'Property should be placed on a new line'}], parser: parserOptions }, + { + code: [ + ' ' + ].join('\n'), + output: [ + ' ' + ].join('\n'), + options: ['always', 4], + errors: [{message: 'Property should be placed on a new line'}], + parser: parserOptions + }, { code: [ '' ].join('\n'), output: [ '' ].join('\n'), options: ['never'], From 9182124a65a58f3408d8b5ff709f0ac3fa1259a7 Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Mon, 3 Oct 2016 12:10:27 +0800 Subject: [PATCH 03/11] Add function to add indents in newline Utilised getNodeIndent helper from jsx-indent-props as base-code --- lib/rules/jsx-first-prop-new-line.js | 45 +++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/lib/rules/jsx-first-prop-new-line.js b/lib/rules/jsx-first-prop-new-line.js index ef07809f99..f58da8f25a 100644 --- a/lib/rules/jsx-first-prop-new-line.js +++ b/lib/rules/jsx-first-prop-new-line.js @@ -20,10 +20,50 @@ module.exports = { schema: [{ enum: ['always', 'never', 'multiline', 'multiline-multiprop'] }] + }, create: function (context) { var configuration = context.options[0]; + var extraColumnStart = 0; + var indentType = 'space'; + var indentSize = 2; + var sourceCode = context.getSourceCode(); + + if (context.options.length > 1) { + if (context.options[1] === 'tab') { + indentSize = 1; + indentType = 'tab'; + } else if (typeof context.options[1] === 'number') { + indentSize = context.options[1]; + indentType = 'space'; + } + } + + function getNodeIndent(node, byLastLine, excludeCommas) { + byLastLine = byLastLine || false; + excludeCommas = excludeCommas || false; + + var src = sourceCode.getText(node, node.loc.start.column + extraColumnStart); + var lines = src.split('\n'); + if (byLastLine) { + src = lines[lines.length - 1]; + } else { + src = lines[0]; + } + + var skip = excludeCommas ? ',' : ''; + + var regExp; + if (indentType === 'space') { + regExp = new RegExp('^[ ' + skip + ']+'); + } else { + regExp = new RegExp('^[\t' + skip + ']+'); + } + + var indent = regExp.exec(src); + return indent ? indent[0].length : 0; + } function isMultilineJSX(jsxNode) { return jsxNode.loc.start.line < jsxNode.loc.end.line; @@ -42,7 +82,10 @@ module.exports = { node: decl, message: 'Property should be placed on a new line', fix: function(fixer) { - return fixer.insertTextAfter(node.name, '\n'); + var nodeIndent = getNodeIndent(node, false, false); + var neededIndent = nodeIndent + indentSize; + var insert = '\n' + Array(neededIndent + 1).join(indentType === 'space' ? ' ' : '\t'); + return fixer.replaceTextRange([node.name.end, decl.start], insert); } }); } From 9834e7a5e9a73bdb616087bd09c6f70ec7f77b28 Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Mon, 3 Oct 2016 18:34:17 +0800 Subject: [PATCH 04/11] Test case for tabs --- tests/lib/rules/jsx-first-prop-new-line.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/lib/rules/jsx-first-prop-new-line.js b/tests/lib/rules/jsx-first-prop-new-line.js index c1a3cf65b8..74dc2c0465 100644 --- a/tests/lib/rules/jsx-first-prop-new-line.js +++ b/tests/lib/rules/jsx-first-prop-new-line.js @@ -193,6 +193,22 @@ ruleTester.run('jsx-first-prop-new-line', rule, { errors: [{message: 'Property should be placed on a new line'}], parser: parserOptions }, + { + code: [ + '\t' + ].join('\n'), + output: [ + '\t' + ].join('\n'), + options: ['always', 'tab'], + errors: [{message: 'Property should be placed on a new line'}], + parser: parserOptions + }, { code: [ ' Date: Tue, 4 Oct 2016 15:31:35 +0800 Subject: [PATCH 05/11] Simplify getNodeIndent cut out unused vars based on feedback --- lib/rules/jsx-first-prop-new-line.js | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/lib/rules/jsx-first-prop-new-line.js b/lib/rules/jsx-first-prop-new-line.js index f58da8f25a..1864004384 100644 --- a/lib/rules/jsx-first-prop-new-line.js +++ b/lib/rules/jsx-first-prop-new-line.js @@ -25,7 +25,6 @@ module.exports = { create: function (context) { var configuration = context.options[0]; - var extraColumnStart = 0; var indentType = 'space'; var indentSize = 2; var sourceCode = context.getSourceCode(); @@ -40,27 +39,14 @@ module.exports = { } } - function getNodeIndent(node, byLastLine, excludeCommas) { - byLastLine = byLastLine || false; - excludeCommas = excludeCommas || false; - - var src = sourceCode.getText(node, node.loc.start.column + extraColumnStart); - var lines = src.split('\n'); - if (byLastLine) { - src = lines[lines.length - 1]; - } else { - src = lines[0]; - } - - var skip = excludeCommas ? ',' : ''; - + function getNodeIndent(node) { + var src = sourceCode.getText(node, node.loc.start.column).split('\n')[0]; var regExp; if (indentType === 'space') { - regExp = new RegExp('^[ ' + skip + ']+'); + regExp = new RegExp('^[ ]+'); } else { - regExp = new RegExp('^[\t' + skip + ']+'); + regExp = new RegExp('^[\t' + ']+'); } - var indent = regExp.exec(src); return indent ? indent[0].length : 0; } @@ -82,8 +68,7 @@ module.exports = { node: decl, message: 'Property should be placed on a new line', fix: function(fixer) { - var nodeIndent = getNodeIndent(node, false, false); - var neededIndent = nodeIndent + indentSize; + var neededIndent = getNodeIndent(node) + indentSize; var insert = '\n' + Array(neededIndent + 1).join(indentType === 'space' ? ' ' : '\t'); return fixer.replaceTextRange([node.name.end, decl.start], insert); } From a7764f70ff65df3750431ab1a8349aa34f228caf Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Tue, 4 Oct 2016 22:40:00 +0800 Subject: [PATCH 06/11] readme update Add fixable for readme and update docs to reflect additional argument --- README.md | 2 +- docs/rules/jsx-first-prop-new-line.md | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 11b8861fec..e8c80b7236 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 309efb967d..7d16a388ae 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. + ## Rule Details This rule checks whether the first property of all JSX elements is correctly placed. There are three possible configurations: @@ -10,6 +12,12 @@ This rule checks whether the first property of all JSX elements is correctly pla * `multiline`: The first property should always be placed on a new line when the JSX tag takes up multiple lines. * `multiline-multiprop`: The first property should always be placed on a new line if the JSX tag takes up multiple lines and there are multiple properties. +In order to utilise autofix, please specify the indentation style as an additional argument in your configuration: + +```json +"jsx-first-prop-new-line": ["always", 2] +``` + The following patterns are considered warnings when configured `"always"`: ```jsx From 2507f37b15602f07f5543b34092bf6bb4e4915e8 Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Tue, 1 Nov 2016 10:40:52 +0800 Subject: [PATCH 07/11] Remove indentation arg --- lib/rules/jsx-first-prop-new-line.js | 30 +--------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/lib/rules/jsx-first-prop-new-line.js b/lib/rules/jsx-first-prop-new-line.js index 1864004384..f725ab98f6 100644 --- a/lib/rules/jsx-first-prop-new-line.js +++ b/lib/rules/jsx-first-prop-new-line.js @@ -20,36 +20,10 @@ module.exports = { schema: [{ enum: ['always', 'never', 'multiline', 'multiline-multiprop'] }] - }, create: function (context) { var configuration = context.options[0]; - var indentType = 'space'; - var indentSize = 2; - var sourceCode = context.getSourceCode(); - - if (context.options.length > 1) { - if (context.options[1] === 'tab') { - indentSize = 1; - indentType = 'tab'; - } else if (typeof context.options[1] === 'number') { - indentSize = context.options[1]; - indentType = 'space'; - } - } - - function getNodeIndent(node) { - var src = sourceCode.getText(node, node.loc.start.column).split('\n')[0]; - var regExp; - if (indentType === 'space') { - regExp = new RegExp('^[ ]+'); - } else { - regExp = new RegExp('^[\t' + ']+'); - } - var indent = regExp.exec(src); - return indent ? indent[0].length : 0; - } function isMultilineJSX(jsxNode) { return jsxNode.loc.start.line < jsxNode.loc.end.line; @@ -68,9 +42,7 @@ module.exports = { node: decl, message: 'Property should be placed on a new line', fix: function(fixer) { - var neededIndent = getNodeIndent(node) + indentSize; - var insert = '\n' + Array(neededIndent + 1).join(indentType === 'space' ? ' ' : '\t'); - return fixer.replaceTextRange([node.name.end, decl.start], insert); + return fixer.replaceTextRange([node.name.end, decl.start], '\n'); } }); } From 5e144a11aa2eb2171232a6fb176ff2e11d66f038 Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Tue, 1 Nov 2016 10:41:21 +0800 Subject: [PATCH 08/11] Remove indentation arg from test cases Revert back to 3 cases as per parent repo --- tests/lib/rules/jsx-first-prop-new-line.js | 36 ++-------------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/tests/lib/rules/jsx-first-prop-new-line.js b/tests/lib/rules/jsx-first-prop-new-line.js index 74dc2c0465..7e38e1af75 100644 --- a/tests/lib/rules/jsx-first-prop-new-line.js +++ b/tests/lib/rules/jsx-first-prop-new-line.js @@ -155,7 +155,7 @@ ruleTester.run('jsx-first-prop-new-line', rule, { code: '', output: [ '' + 'prop="one" />' ].join('\n'), options: ['always'], errors: [{message: 'Property should be placed on a new line'}], @@ -169,7 +169,7 @@ ruleTester.run('jsx-first-prop-new-line', rule, { ].join('\n'), output: [ '' ].join('\n'), @@ -177,38 +177,6 @@ ruleTester.run('jsx-first-prop-new-line', rule, { errors: [{message: 'Property should be placed on a new line'}], parser: parserOptions }, - { - code: [ - ' ' - ].join('\n'), - output: [ - ' ' - ].join('\n'), - options: ['always', 4], - errors: [{message: 'Property should be placed on a new line'}], - parser: parserOptions - }, - { - code: [ - '\t' - ].join('\n'), - output: [ - '\t' - ].join('\n'), - options: ['always', 'tab'], - errors: [{message: 'Property should be placed on a new line'}], - parser: parserOptions - }, { code: [ ' Date: Tue, 1 Nov 2016 10:44:42 +0800 Subject: [PATCH 09/11] Readme update s/utilise/utilize & simple warning --- docs/rules/jsx-first-prop-new-line.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/docs/rules/jsx-first-prop-new-line.md b/docs/rules/jsx-first-prop-new-line.md index 7d16a388ae..67e45d4067 100644 --- a/docs/rules/jsx-first-prop-new-line.md +++ b/docs/rules/jsx-first-prop-new-line.md @@ -2,7 +2,7 @@ Ensure correct position of the first property. -**Fixable:** This rule is automatically fixable using the `--fix` flag on the command line. +**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 @@ -12,12 +12,6 @@ This rule checks whether the first property of all JSX elements is correctly pla * `multiline`: The first property should always be placed on a new line when the JSX tag takes up multiple lines. * `multiline-multiprop`: The first property should always be placed on a new line if the JSX tag takes up multiple lines and there are multiple properties. -In order to utilise autofix, please specify the indentation style as an additional argument in your configuration: - -```json -"jsx-first-prop-new-line": ["always", 2] -``` - The following patterns are considered warnings when configured `"always"`: ```jsx From 0aeb95c03581859dc4373d480cc282bdbe53eb67 Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Tue, 1 Nov 2016 19:55:51 +0800 Subject: [PATCH 10/11] Edit multiline-multiprop test cases for fixes --- tests/lib/rules/jsx-first-prop-new-line.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/lib/rules/jsx-first-prop-new-line.js b/tests/lib/rules/jsx-first-prop-new-line.js index 7e38e1af75..e0c93c93ba 100644 --- a/tests/lib/rules/jsx-first-prop-new-line.js +++ b/tests/lib/rules/jsx-first-prop-new-line.js @@ -198,6 +198,11 @@ ruleTester.run('jsx-first-prop-new-line', rule, { '' ].join('\n'), + output: [ + '' + ].join('\n'), options: ['multiline'], errors: [{message: 'Property should be placed on a new line'}], parser: parserOptions @@ -207,6 +212,11 @@ ruleTester.run('jsx-first-prop-new-line', rule, { '' ].join('\n'), + output: [ + '' + ].join('\n'), options: ['multiline-multiprop'], errors: [{message: 'Property should be placed on a new line'}], parser: parserOptions From a37687ec1b983a86ab65be07d651f39bb63818e5 Mon Sep 17 00:00:00 2001 From: Yak Jun Xiang Date: Thu, 3 Nov 2016 00:14:14 +0800 Subject: [PATCH 11/11] Add fix for multple props on first line fix referenced from GovTechSG's fork --- lib/rules/jsx-first-prop-new-line.js | 3 ++- tests/lib/rules/jsx-first-prop-new-line.js | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/rules/jsx-first-prop-new-line.js b/lib/rules/jsx-first-prop-new-line.js index f725ab98f6..6307439b8b 100644 --- a/lib/rules/jsx-first-prop-new-line.js +++ b/lib/rules/jsx-first-prop-new-line.js @@ -36,7 +36,7 @@ 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, @@ -46,6 +46,7 @@ module.exports = { } }); } + return true; }); } else if (configuration === 'never' && node.attributes.length > 0) { var firstNode = node.attributes[0]; diff --git a/tests/lib/rules/jsx-first-prop-new-line.js b/tests/lib/rules/jsx-first-prop-new-line.js index e0c93c93ba..a7b558ce30 100644 --- a/tests/lib/rules/jsx-first-prop-new-line.js +++ b/tests/lib/rules/jsx-first-prop-new-line.js @@ -152,10 +152,10 @@ ruleTester.run('jsx-first-prop-new-line', rule, { invalid: [ { - code: '', + code: '', output: [ '' + 'propOne="one" propTwo="two" />' ].join('\n'), options: ['always'], errors: [{message: 'Property should be placed on a new line'}],