From 1b8beb039cda267222b28ca5d93542e35cdf8407 Mon Sep 17 00:00:00 2001 From: Yosuke Ota Date: Wed, 16 Dec 2020 14:38:17 +0900 Subject: [PATCH] Fixed wrong autofix when properties on the same line in new-line-between-multi-line-property. (#1378) I also improved the position of the report. --- .../new-line-between-multi-line-property.js | 107 ++++++---- .../new-line-between-multi-line-property.js | 185 ++++++++++++++++-- typings/eslint/index.d.ts | 2 +- 3 files changed, 247 insertions(+), 47 deletions(-) diff --git a/lib/rules/new-line-between-multi-line-property.js b/lib/rules/new-line-between-multi-line-property.js index b40f4ffbb..b48d78379 100644 --- a/lib/rules/new-line-between-multi-line-property.js +++ b/lib/rules/new-line-between-multi-line-property.js @@ -5,6 +5,46 @@ 'use strict' const utils = require('../utils') + +/** + * @param {Token} node + */ +function isComma(node) { + return node.type === 'Punctuator' && node.value === ',' +} + +/** + * Check whether the between given nodes has empty line. + * @param {SourceCode} sourceCode + * @param {ASTNode} pre + * @param {ASTNode} cur + */ +function* iterateBetweenTokens(sourceCode, pre, cur) { + yield sourceCode.getLastToken(pre) + yield* sourceCode.getTokensBetween(pre, cur, { + includeComments: true + }) + yield sourceCode.getFirstToken(cur) +} + +/** + * Check whether the between given nodes has empty line. + * @param {SourceCode} sourceCode + * @param {ASTNode} pre + * @param {ASTNode} cur + */ +function hasEmptyLine(sourceCode, pre, cur) { + /** @type {Token|null} */ + let preToken = null + for (const token of iterateBetweenTokens(sourceCode, pre, cur)) { + if (preToken && token.loc.start.line - preToken.loc.end.line >= 2) { + return true + } + preToken = token + } + return false +} + // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ @@ -35,9 +75,6 @@ module.exports = { /** @param {RuleContext} context */ create(context) { - // always insert one line - const insertLine = 1 - let minLineOfMultilineProperty = 2 if ( context.options && @@ -71,42 +108,44 @@ module.exports = { const cur = properties[i] const pre = properties[i - 1] - const leadingComments = sourceCode - .getTokensBetween(pre, cur, { includeComments: true }) - .filter((token) => ['Line', 'Block'].includes(token.type)) const lineCountOfPreProperty = pre.loc.end.line - pre.loc.start.line + 1 - let curStartLine = cur.loc.start.line - if (leadingComments.length) { - curStartLine = leadingComments[0].loc.start.line + if (lineCountOfPreProperty < minLineOfMultilineProperty) { + continue + } + + if (hasEmptyLine(sourceCode, pre, cur)) { + continue } - const lineCountBetweenCurAndPreProperty = - curStartLine - pre.loc.end.line - 1 - if ( - lineCountOfPreProperty >= minLineOfMultilineProperty && - lineCountBetweenCurAndPreProperty < insertLine - ) { - context.report({ - node: pre, - loc: pre.loc, - message: - 'Enforce new lines between multi-line properties in Vue components.', - fix(fixer) { - let firstPositionOfLine = cur.range[0] - cur.loc.start.column - if (leadingComments.length) { - const firstComment = leadingComments[0] - firstPositionOfLine = - firstComment.range[0] - firstComment.loc.start.column + + context.report({ + node: pre, + loc: { + start: pre.loc.end, + end: cur.loc.start + }, + message: + 'Enforce new lines between multi-line properties in Vue components.', + fix(fixer) { + /** @type {Token|null} */ + let preToken = null + for (const token of iterateBetweenTokens( + sourceCode, + pre, + cur + )) { + if ( + preToken && + preToken.loc.end.line < token.loc.start.line + ) { + return fixer.insertTextAfter(preToken, '\n') } - // this action equal to insert number of line before node - return fixer.insertTextAfterRange( - [firstPositionOfLine, firstPositionOfLine], - '\n' - // to avoid conflict with no-multiple-empty-lines, only insert one newline - ) + preToken = token } - }) - } + const commaToken = sourceCode.getTokenAfter(pre, isComma) + return fixer.insertTextAfter(commaToken || pre, '\n\n') + } + }) } } }) diff --git a/tests/lib/rules/new-line-between-multi-line-property.js b/tests/lib/rules/new-line-between-multi-line-property.js index 740654f1a..41dce3434 100644 --- a/tests/lib/rules/new-line-between-multi-line-property.js +++ b/tests/lib/rules/new-line-between-multi-line-property.js @@ -103,6 +103,19 @@ ruleTester.run('new-line-between-multi-line-property', rule, { } ` + }, + { + filename: 'test.vue', + code: ` + ` } ], @@ -152,7 +165,7 @@ ruleTester.run('new-line-between-multi-line-property', rule, { { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 10 + line: 13 } ] }, @@ -263,12 +276,12 @@ ruleTester.run('new-line-between-multi-line-property', rule, { { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 5 + line: 18 }, { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 19 + line: 33 } ] }, @@ -319,7 +332,7 @@ ruleTester.run('new-line-between-multi-line-property', rule, { { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 9 + line: 13 } ] }, @@ -372,7 +385,7 @@ ruleTester.run('new-line-between-multi-line-property', rule, { { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 9 + line: 13 } ] }, @@ -389,7 +402,7 @@ ruleTester.run('new-line-between-multi-line-property', rule, { required: true }, }, - + methods: { test() { @@ -414,7 +427,7 @@ ruleTester.run('new-line-between-multi-line-property', rule, { required: true }, }, - + methods: { test() { @@ -435,7 +448,7 @@ ruleTester.run('new-line-between-multi-line-property', rule, { { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 12 + line: 14 } ] }, @@ -514,17 +527,17 @@ ruleTester.run('new-line-between-multi-line-property', rule, { { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 4 + line: 8 }, { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 5 + line: 12 }, { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 9 + line: 15 } ] }, @@ -606,8 +619,120 @@ ruleTester.run('new-line-between-multi-line-property', rule, { { message: 'Enforce new lines between multi-line properties in Vue components.', - line: 4 + line: 8 + }, + { + message: + 'Enforce new lines between multi-line properties in Vue components.', + line: 13 + }, + { + message: + 'Enforce new lines between multi-line properties in Vue components.', + line: 16 + } + ] + }, + { + filename: 'test.vue', + code: ` + `, + output: ` + `, + errors: [ + { + message: + 'Enforce new lines between multi-line properties in Vue components.', + line: 5 + } + ] + }, + { + filename: 'test.vue', + code: ` + `, + output: ` + `, + errors: [ + { + message: + 'Enforce new lines between multi-line properties in Vue components.', + line: 5 + } + ] + }, + { + filename: 'test.vue', + code: ` + `, + output: ` + `, + errors: [ { message: 'Enforce new lines between multi-line properties in Vue components.', @@ -617,6 +742,42 @@ ruleTester.run('new-line-between-multi-line-property', rule, { message: 'Enforce new lines between multi-line properties in Vue components.', line: 10 + }, + { + message: + 'Enforce new lines between multi-line properties in Vue components.', + line: 14 + } + ] + }, + { + filename: 'test.vue', + code: ` + `, + output: ` + `, + errors: [ + { + message: + 'Enforce new lines between multi-line properties in Vue components.', + line: 5 } ] } diff --git a/typings/eslint/index.d.ts b/typings/eslint/index.d.ts index 0c99161f8..f977b45ae 100644 --- a/typings/eslint/index.d.ts +++ b/typings/eslint/index.d.ts @@ -150,7 +150,7 @@ export class SourceCode /*extends ESLintSourceCode*/ { getLastToken(node: VNODE.HasLocation, options: number): AST.Token getLastToken( node: VNODE.HasLocation, - optionss: SourceCode.CursorWithSkipOptions + options: SourceCode.CursorWithSkipOptions ): AST.Token | null getLastTokens( node: VNODE.HasLocation,