From 29dbca73d762a809adb2f457b527e144426d54a7 Mon Sep 17 00:00:00 2001 From: Mark de Dios Date: Fri, 15 Mar 2019 10:27:10 -0700 Subject: [PATCH] Fix: implicit-arrow-linebreak adds extra characters (fixes #11268) (#11407) * add failing test case * Add specified test case from issue * Remove calculation for white spaces from formatComments and addParentheses, add condition to check for arrow expressions with block statements * Fix linting errors, ensure npm test runs without fail * Change new test case to check for prepended comment, add unIndent function to tests for implicit arrow linebreak * Add optional token type to arrow body * Add failing test case * Remove null return from autofixBesides, refactor body setting lines to use .body * Fix linting errors * Fix linting error in unIndent rule in implicit arrow linebreak test * Add another test case where nested arrow function contains block statement * Add constraint for block statement in add parentheses --- lib/rules/implicit-arrow-linebreak.js | 40 +-- tests/lib/rules/implicit-arrow-linebreak.js | 361 ++++++++++++-------- 2 files changed, 226 insertions(+), 175 deletions(-) diff --git a/lib/rules/implicit-arrow-linebreak.js b/lib/rules/implicit-arrow-linebreak.js index ad0d70da66c..50f64561847 100644 --- a/lib/rules/implicit-arrow-linebreak.js +++ b/lib/rules/implicit-arrow-linebreak.js @@ -6,8 +6,7 @@ const { isArrowToken, - isParenthesised, - isOpeningParenToken + isParenthesised } = require("../util/ast-utils"); //------------------------------------------------------------------------------ @@ -57,8 +56,7 @@ module.exports = { * @param {Integer} column The column number of the first token * @returns {string} A string of comment text joined by line breaks */ - function formatComments(comments, column) { - const whiteSpaces = " ".repeat(column); + function formatComments(comments) { return `${comments.map(comment => { @@ -67,12 +65,12 @@ module.exports = { } return `/*${comment.value}*/`; - }).join(`\n${whiteSpaces}`)}\n${whiteSpaces}`; + }).join("\n")}\n`; } /** * Finds the first token to prepend comments to depending on the parent type - * @param {Node} node The validated node + * @param {ASTNode} node The validated node * @returns {Token|Node} The node to prepend comments to */ function findFirstToken(node) { @@ -109,24 +107,19 @@ module.exports = { let followingBody = arrowBody; let currentArrow = arrow; - while (currentArrow) { + while (currentArrow && followingBody.type !== "BlockStatement") { if (!isParenthesised(sourceCode, followingBody)) { parenthesesFixes.push( fixer.insertTextAfter(currentArrow, " (") ); - const paramsToken = sourceCode.getTokenBefore(currentArrow, token => - isOpeningParenToken(token) || token.type === "Identifier"); - - const whiteSpaces = " ".repeat(paramsToken.loc.start.column); - - closingParentheses = `\n${whiteSpaces})${closingParentheses}`; + closingParentheses = `\n)${closingParentheses}`; } currentArrow = sourceCode.getTokenAfter(currentArrow, isArrowToken); if (currentArrow) { - followingBody = sourceCode.getTokenAfter(currentArrow, token => !isOpeningParenToken(token)); + followingBody = followingBody.body; } } @@ -137,10 +130,10 @@ module.exports = { /** * Autofixes the function body to collapse onto the same line as the arrow. - * If comments exist, prepends the comments before the arrow function. - * If the function body contains arrow functions, appends the function bodies with parentheses. + * If comments exist, checks if the function body contains arrow functions, and appends the body with parentheses. + * Otherwise, prepends the comments before the arrow function. * @param {Token} arrowToken The arrow token. - * @param {ASTNode} arrowBody the function body + * @param {ASTNode|Token} arrowBody the function body * @param {ASTNode} node The evaluated node * @returns {Function} autofixer -- validates the node to adhere to besides */ @@ -161,23 +154,18 @@ module.exports = { ) { // If any arrow functions follow, return the necessary parens fixes. - if (sourceCode.getTokenAfter(arrowToken, isArrowToken) && arrowBody.parent.parent.type !== "VariableDeclarator") { + if (node.body.type === "ArrowFunctionExpression" && + arrowBody.parent.parent.type !== "VariableDeclarator" + ) { return addParentheses(fixer, arrowToken, arrowBody); } - - // If any arrow functions precede, the necessary fixes have already been returned, so return null. - if (sourceCode.getTokenBefore(arrowToken, isArrowToken) && arrowBody.parent.parent.type !== "VariableDeclarator") { - return null; - } } const firstToken = findFirstToken(node); - const commentText = formatComments(comments, firstToken.loc.start.column); - const commentBeforeExpression = fixer.insertTextBeforeRange( firstToken.range, - commentText + formatComments(comments) ); return [placeBesides, commentBeforeExpression]; diff --git a/tests/lib/rules/implicit-arrow-linebreak.js b/tests/lib/rules/implicit-arrow-linebreak.js index 0ed7213dc9c..9244557066f 100644 --- a/tests/lib/rules/implicit-arrow-linebreak.js +++ b/tests/lib/rules/implicit-arrow-linebreak.js @@ -20,6 +20,20 @@ const UNEXPECTED_LINEBREAK = { messageId: "unexpected" }; const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } }); +/** + * Prevents leading spaces in a multiline template literal from appearing in the resulting string + * @param {string[]} strings The strings in the template literal + * @returns {string} The template literal, with spaces removed from all lines + */ +function unIndent(strings) { + const templateValue = strings[0]; + const lines = templateValue.replace(/^\n/u, "").replace(/\n\s*$/u, "").split("\n"); + const lineIndents = lines.filter(line => line.trim()).map(line => line.match(/ */u)[0].length); + const minLineIndent = Math.min(...lineIndents); + + return lines.map(line => line.slice(minLineIndent)).join("\n"); +} + ruleTester.run("implicit-arrow-linebreak", rule, { valid: [ @@ -200,23 +214,23 @@ ruleTester.run("implicit-arrow-linebreak", rule, { `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` + code: unIndent` (foo) => // test comment bar `, - output: ` + output: unIndent` // test comment (foo) => bar `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` + code: unIndent` const foo = () => // comment [] `, - output: ` + output: unIndent` // comment const foo = () => [] `, @@ -255,30 +269,30 @@ ruleTester.run("implicit-arrow-linebreak", rule, { errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - (foo) => - // comment - // another comment - bar`, - output: ` - // comment - // another comment - (foo) => bar`, + code: unIndent` + (foo) => + // comment + // another comment + bar`, + output: unIndent` + // comment + // another comment + (foo) => bar`, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - (foo) => - // comment - ( - // another comment - bar - )`, - output: ` - // comment - (foo) => ( - // another comment - bar - )`, + code: unIndent` + (foo) => + // comment + ( + // another comment + bar + )`, + output: unIndent` + // comment + (foo) => ( + // another comment + bar + )`, errors: [UNEXPECTED_LINEBREAK] }, { @@ -290,36 +304,34 @@ ruleTester.run("implicit-arrow-linebreak", rule, { output: "//comment \n(foo) => bar", errors: [UNEXPECTED_LINEBREAK] }, { - code: ` + code: unIndent` (foo) => /* test comment */ bar `, - output: ` + output: unIndent` /* test comment */ (foo) => bar `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` + code: unIndent` (foo) => // hi bar => // there - baz; - `, - output: ` + baz;`, + output: unIndent` (foo) => ( // hi bar => ( // there baz - ) - ); - `, + ) + );`, errors: [UNEXPECTED_LINEBREAK, UNEXPECTED_LINEBREAK] }, { - code: ` + code: unIndent` (foo) => // hi bar => ( @@ -327,74 +339,74 @@ ruleTester.run("implicit-arrow-linebreak", rule, { baz ) `, - output: ` + output: unIndent` (foo) => ( // hi bar => ( // there baz ) - ) + ) `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - const foo = { - id: 'bar', - prop: (foo1) => - // comment - 'returning this string', - } + code: unIndent` + const foo = { + id: 'bar', + prop: (foo1) => + // comment + 'returning this string', + } `, - output: ` - const foo = { - id: 'bar', - // comment - prop: (foo1) => 'returning this string', - } + output: unIndent` + const foo = { + id: 'bar', + // comment + prop: (foo1) => 'returning this string', + } `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - [ foo => - // comment - 'bar' - ] + code: unIndent` + [ foo => + // comment + 'bar' + ] `, - output: ` - [ // comment - foo => 'bar' - ] + output: unIndent` + [ // comment + foo => 'bar' + ] `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - "foo".split('').map((char) => + code: unIndent` + "foo".split('').map((char) => // comment char - ) + ) `, - output: ` - // comment - "foo".split('').map((char) => char - ) + output: unIndent` + // comment + "foo".split('').map((char) => char + ) `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` + code: unIndent` new Promise((resolve, reject) => // comment resolve() ) `, - output: ` + output: unIndent` new Promise(// comment - (resolve, reject) => resolve() + (resolve, reject) => resolve() ) `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` + code: unIndent` () => /* succinct @@ -403,7 +415,7 @@ ruleTester.run("implicit-arrow-linebreak", rule, { */ bar `, - output: ` + output: unIndent` /* succinct explanation @@ -413,7 +425,7 @@ ruleTester.run("implicit-arrow-linebreak", rule, { `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` + code: unIndent` stepOne => /* here is @@ -423,7 +435,7 @@ ruleTester.run("implicit-arrow-linebreak", rule, { stepTwo => // then this happens stepThree`, - output: ` + output: unIndent` stepOne => ( /* here is @@ -433,137 +445,188 @@ ruleTester.run("implicit-arrow-linebreak", rule, { stepTwo => ( // then this happens stepThree - ) + ) )`, errors: [UNEXPECTED_LINEBREAK, UNEXPECTED_LINEBREAK] }, { - code: ` - () => - /* - multi - line - */ - bar => + code: unIndent` + () => /* - many - lines + multi + line */ - baz - `, - output: ` - () => ( - /* - multi - line - */ - bar => ( + bar => + /* + many + lines + */ + baz + `, + output: unIndent` + () => ( /* - many - lines + multi + line */ - baz + bar => ( + /* + many + lines + */ + baz + ) ) - ) `, errors: [UNEXPECTED_LINEBREAK, UNEXPECTED_LINEBREAK] }, { - code: ` - foo('', boo => + code: unIndent` + foo('', boo => // comment bar - ) + ) `, - output: ` - // comment - foo('', boo => bar - ) + output: unIndent` + // comment + foo('', boo => bar + ) `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - async foo => - // comment - 'string' + code: unIndent` + async foo => + // comment + 'string' `, - output: ` - // comment - async foo => 'string' + output: unIndent` + // comment + async foo => 'string' `, parserOptions: { ecmaVersion: 8 }, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - async foo => + code: unIndent` + async foo => + // comment + // another + bar; + `, + output: unIndent` // comment // another - bar; - `, - output: ` - // comment - // another - async foo => bar; + async foo => bar; `, parserOptions: { ecmaVersion: 8 }, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - async (foo) => - // comment - 'string' + code: unIndent` + async (foo) => + // comment + 'string' `, - output: ` - // comment - async (foo) => 'string' + output: unIndent` + // comment + async (foo) => 'string' `, parserOptions: { ecmaVersion: 8 }, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - const foo = 1, + code: unIndent` + const foo = 1, bar = 2, baz = () => // comment qux `, - output: ` - const foo = 1, + output: unIndent` + const foo = 1, bar = 2, // comment - baz = () => qux + baz = () => qux `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - const foo = () => + code: unIndent` + const foo = () => //comment qux, bar = 2, baz = 3 `, - output: ` - //comment - const foo = () => qux, + output: unIndent` + //comment + const foo = () => qux, bar = 2, baz = 3 `, errors: [UNEXPECTED_LINEBREAK] }, { - code: ` - const foo = () => - //two - 1, - boo = () => - //comment - 2, - bop = "what" + code: unIndent` + const foo = () => + //two + 1, + boo = () => + //comment + 2, + bop = "what" `, - output: ` - //two - const foo = () => 1, - //comment + output: unIndent` + //two + const foo = () => 1, + //comment boo = () => 2, - bop = "what" + bop = "what" `, errors: [UNEXPECTED_LINEBREAK, UNEXPECTED_LINEBREAK] + }, { + code: unIndent` + start() + .then(() => + /* If I put a comment here, eslint --fix breaks badly */ + process && typeof process.send === 'function' && process.send('ready') + ) + .catch(err => { + /* catch seems to be needed here */ + console.log('Error: ', err) + })`, + output: unIndent` + /* If I put a comment here, eslint --fix breaks badly */ + start() + .then(() => process && typeof process.send === 'function' && process.send('ready') + ) + .catch(err => { + /* catch seems to be needed here */ + console.log('Error: ', err) + })`, + errors: [UNEXPECTED_LINEBREAK] + }, { + code: unIndent` + hello(response => + // comment + response, param => param)`, + output: unIndent` + // comment + hello(response => response, param => param)`, + errors: [UNEXPECTED_LINEBREAK] + }, { + code: unIndent` + start( + arr => + // cometh + bod => { + // soon + yyyy + } + )`, + output: unIndent` + start( + arr => ( + // cometh + bod => { + // soon + yyyy + } + ) + )`, + errors: [UNEXPECTED_LINEBREAK] }, // 'below' option