From cbf3585f1d6c60414c07380367a8b4505ee3538d Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Sat, 24 Oct 2020 02:38:32 +0200 Subject: [PATCH] Update: skip keyword check for fns in space-before-blocks (fixes #13553) (#13712) --- lib/rules/space-before-blocks.js | 43 +++- .../return-type-keyword-1.js | 206 ++++++++++++++++++ .../return-type-keyword-2.js | 98 +++++++++ tests/lib/rules/space-before-blocks.js | 18 +- 4 files changed, 355 insertions(+), 10 deletions(-) create mode 100644 tests/fixtures/parsers/space-before-blocks/return-type-keyword-1.js create mode 100644 tests/fixtures/parsers/space-before-blocks/return-type-keyword-2.js diff --git a/lib/rules/space-before-blocks.js b/lib/rules/space-before-blocks.js index 9b56481bf35..87ef9bfd880 100644 --- a/lib/rules/space-before-blocks.js +++ b/lib/rules/space-before-blocks.js @@ -5,8 +5,31 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + const astUtils = require("./utils/ast-utils"); +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Checks whether the given node represents the body of a function. + * @param {ASTNode} node the node to check. + * @returns {boolean} `true` if the node is function body. + */ +function isFunctionBody(node) { + const parent = node.parent; + + return ( + node.type === "BlockStatement" && + astUtils.isFunction(parent) && + parent.body === node + ); +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -82,13 +105,16 @@ module.exports = { } /** - * Checks whether or not a given token is an arrow operator (=>) or a keyword - * in order to avoid to conflict with `arrow-spacing` and `keyword-spacing`. - * @param {Token} token A token to check. - * @returns {boolean} `true` if the token is an arrow operator. + * Checks whether the spacing before the given block is already controlled by another rule: + * - `arrow-spacing` checks spaces after `=>`. + * - `keyword-spacing` checks spaces after keywords in certain contexts. + * @param {Token} precedingToken first token before the block. + * @param {ASTNode|Token} node `BlockStatement` node or `{` token of a `SwitchStatement` node. + * @returns {boolean} `true` if requiring or disallowing spaces before the given block could produce conflicts with other rules. */ - function isConflicted(token) { - return (token.type === "Punctuator" && token.value === "=>") || token.type === "Keyword"; + function isConflicted(precedingToken, node) { + return astUtils.isArrowToken(precedingToken) || + astUtils.isKeywordToken(precedingToken) && !isFunctionBody(node); } /** @@ -99,13 +125,12 @@ module.exports = { function checkPrecedingSpace(node) { const precedingToken = sourceCode.getTokenBefore(node); - if (precedingToken && !isConflicted(precedingToken) && astUtils.isTokenOnSameLine(precedingToken, node)) { + if (precedingToken && !isConflicted(precedingToken, node) && astUtils.isTokenOnSameLine(precedingToken, node)) { const hasSpace = sourceCode.isSpaceBetweenTokens(precedingToken, node); - const parent = context.getAncestors().pop(); let requireSpace; let requireNoSpace; - if (parent.type === "FunctionExpression" || parent.type === "FunctionDeclaration") { + if (isFunctionBody(node)) { requireSpace = alwaysFunctions; requireNoSpace = neverFunctions; } else if (node.type === "ClassBody") { diff --git a/tests/fixtures/parsers/space-before-blocks/return-type-keyword-1.js b/tests/fixtures/parsers/space-before-blocks/return-type-keyword-1.js new file mode 100644 index 00000000000..295eb650bce --- /dev/null +++ b/tests/fixtures/parsers/space-before-blocks/return-type-keyword-1.js @@ -0,0 +1,206 @@ +"use strict"; + +/** + * Parser: @typescript-eslint/parser@4.2.0 + * Source code: + * class A { foo(bar: string): void{} } + */ + +exports.parse = () => ({ + type: "Program", + body: [ + { + type: "ClassDeclaration", + id: { + type: "Identifier", + name: "A", + range: [6, 7], + loc: { start: { line: 1, column: 6 }, end: { line: 1, column: 7 } }, + }, + body: { + type: "ClassBody", + body: [ + { + type: "MethodDefinition", + key: { + type: "Identifier", + name: "foo", + range: [10, 13], + loc: { + start: { line: 1, column: 10 }, + end: { line: 1, column: 13 }, + }, + }, + value: { + type: "FunctionExpression", + id: null, + generator: false, + expression: false, + async: false, + body: { + type: "BlockStatement", + body: [], + range: [32, 34], + loc: { + start: { line: 1, column: 32 }, + end: { line: 1, column: 34 }, + }, + }, + range: [13, 34], + params: [ + { + type: "Identifier", + name: "bar", + range: [14, 25], + loc: { + start: { line: 1, column: 14 }, + end: { line: 1, column: 25 }, + }, + typeAnnotation: { + type: "TSTypeAnnotation", + loc: { + start: { line: 1, column: 17 }, + end: { line: 1, column: 25 }, + }, + range: [17, 25], + typeAnnotation: { + type: "TSStringKeyword", + range: [19, 25], + loc: { + start: { line: 1, column: 19 }, + end: { line: 1, column: 25 }, + }, + }, + }, + }, + ], + loc: { + start: { line: 1, column: 13 }, + end: { line: 1, column: 34 }, + }, + returnType: { + type: "TSTypeAnnotation", + loc: { + start: { line: 1, column: 26 }, + end: { line: 1, column: 32 }, + }, + range: [26, 32], + typeAnnotation: { + type: "TSVoidKeyword", + range: [28, 32], + loc: { + start: { line: 1, column: 28 }, + end: { line: 1, column: 32 }, + }, + }, + }, + }, + computed: false, + static: false, + kind: "method", + range: [10, 34], + loc: { + start: { line: 1, column: 10 }, + end: { line: 1, column: 34 }, + }, + }, + ], + range: [8, 36], + loc: { start: { line: 1, column: 8 }, end: { line: 1, column: 36 } }, + }, + superClass: null, + range: [0, 36], + loc: { start: { line: 1, column: 0 }, end: { line: 1, column: 36 } }, + }, + ], + sourceType: "script", + range: [0, 36], + loc: { start: { line: 1, column: 0 }, end: { line: 1, column: 36 } }, + tokens: [ + { + type: "Keyword", + value: "class", + range: [0, 5], + loc: { start: { line: 1, column: 0 }, end: { line: 1, column: 5 } }, + }, + { + type: "Identifier", + value: "A", + range: [6, 7], + loc: { start: { line: 1, column: 6 }, end: { line: 1, column: 7 } }, + }, + { + type: "Punctuator", + value: "{", + range: [8, 9], + loc: { start: { line: 1, column: 8 }, end: { line: 1, column: 9 } }, + }, + { + type: "Identifier", + value: "foo", + range: [10, 13], + loc: { start: { line: 1, column: 10 }, end: { line: 1, column: 13 } }, + }, + { + type: "Punctuator", + value: "(", + range: [13, 14], + loc: { start: { line: 1, column: 13 }, end: { line: 1, column: 14 } }, + }, + { + type: "Identifier", + value: "bar", + range: [14, 17], + loc: { start: { line: 1, column: 14 }, end: { line: 1, column: 17 } }, + }, + { + type: "Punctuator", + value: ":", + range: [17, 18], + loc: { start: { line: 1, column: 17 }, end: { line: 1, column: 18 } }, + }, + { + type: "Identifier", + value: "string", + range: [19, 25], + loc: { start: { line: 1, column: 19 }, end: { line: 1, column: 25 } }, + }, + { + type: "Punctuator", + value: ")", + range: [25, 26], + loc: { start: { line: 1, column: 25 }, end: { line: 1, column: 26 } }, + }, + { + type: "Punctuator", + value: ":", + range: [26, 27], + loc: { start: { line: 1, column: 26 }, end: { line: 1, column: 27 } }, + }, + { + type: "Keyword", + value: "void", + range: [28, 32], + loc: { start: { line: 1, column: 28 }, end: { line: 1, column: 32 } }, + }, + { + type: "Punctuator", + value: "{", + range: [32, 33], + loc: { start: { line: 1, column: 32 }, end: { line: 1, column: 33 } }, + }, + { + type: "Punctuator", + value: "}", + range: [33, 34], + loc: { start: { line: 1, column: 33 }, end: { line: 1, column: 34 } }, + }, + { + type: "Punctuator", + value: "}", + range: [35, 36], + loc: { start: { line: 1, column: 35 }, end: { line: 1, column: 36 } }, + }, + ], + comments: [], + }); diff --git a/tests/fixtures/parsers/space-before-blocks/return-type-keyword-2.js b/tests/fixtures/parsers/space-before-blocks/return-type-keyword-2.js new file mode 100644 index 00000000000..48c444b36e1 --- /dev/null +++ b/tests/fixtures/parsers/space-before-blocks/return-type-keyword-2.js @@ -0,0 +1,98 @@ +"use strict"; + +/** + * Parser: @typescript-eslint/parser@4.2.0 + * Source code: + * function foo(): null {} + */ + +exports.parse = () => ({ + type: "Program", + body: [ + { + type: "FunctionDeclaration", + id: { + type: "Identifier", + name: "foo", + range: [9, 12], + loc: { start: { line: 1, column: 9 }, end: { line: 1, column: 12 } }, + }, + generator: false, + expression: false, + async: false, + params: [], + body: { + type: "BlockStatement", + body: [], + range: [21, 23], + loc: { start: { line: 1, column: 21 }, end: { line: 1, column: 23 } }, + }, + range: [0, 23], + loc: { start: { line: 1, column: 0 }, end: { line: 1, column: 23 } }, + returnType: { + type: "TSTypeAnnotation", + loc: { start: { line: 1, column: 14 }, end: { line: 1, column: 20 } }, + range: [14, 20], + typeAnnotation: { + type: "TSNullKeyword", + range: [16, 20], + loc: { start: { line: 1, column: 16 }, end: { line: 1, column: 20 } }, + }, + }, + }, + ], + sourceType: "script", + range: [0, 23], + loc: { start: { line: 1, column: 0 }, end: { line: 1, column: 23 } }, + tokens: [ + { + type: "Keyword", + value: "function", + range: [0, 8], + loc: { start: { line: 1, column: 0 }, end: { line: 1, column: 8 } }, + }, + { + type: "Identifier", + value: "foo", + range: [9, 12], + loc: { start: { line: 1, column: 9 }, end: { line: 1, column: 12 } }, + }, + { + type: "Punctuator", + value: "(", + range: [12, 13], + loc: { start: { line: 1, column: 12 }, end: { line: 1, column: 13 } }, + }, + { + type: "Punctuator", + value: ")", + range: [13, 14], + loc: { start: { line: 1, column: 13 }, end: { line: 1, column: 14 } }, + }, + { + type: "Punctuator", + value: ":", + range: [14, 15], + loc: { start: { line: 1, column: 14 }, end: { line: 1, column: 15 } }, + }, + { + type: "Keyword", + value: "null", + range: [16, 20], + loc: { start: { line: 1, column: 16 }, end: { line: 1, column: 20 } }, + }, + { + type: "Punctuator", + value: "{", + range: [21, 22], + loc: { start: { line: 1, column: 21 }, end: { line: 1, column: 22 } }, + }, + { + type: "Punctuator", + value: "}", + range: [22, 23], + loc: { start: { line: 1, column: 22 }, end: { line: 1, column: 23 } }, + }, + ], + comments: [], + }); diff --git a/tests/lib/rules/space-before-blocks.js b/tests/lib/rules/space-before-blocks.js index 7b36d9f30ee..7424aec8458 100644 --- a/tests/lib/rules/space-before-blocks.js +++ b/tests/lib/rules/space-before-blocks.js @@ -10,7 +10,8 @@ //------------------------------------------------------------------------------ const rule = require("../../../lib/rules/space-before-blocks"), - { RuleTester } = require("../../../lib/rule-tester"); + { RuleTester } = require("../../../lib/rule-tester"), + fixtureParser = require("../../fixtures/fixture-parser"); //------------------------------------------------------------------------------ // Tests @@ -555,6 +556,21 @@ ruleTester.run("space-before-blocks", rule, { options: classesNeverOthersOffArgs, parserOptions: { ecmaVersion: 6 }, errors: [expectedNoSpacingError] + }, + + // https://github.com/eslint/eslint/issues/13553 + { + code: "class A { foo(bar: string): void{} }", + output: "class A { foo(bar: string): void {} }", + parser: fixtureParser("space-before-blocks", "return-type-keyword-1"), + errors: [expectedSpacingError] + }, + { + code: "function foo(): null {}", + output: "function foo(): null{}", + options: neverArgs, + parser: fixtureParser("space-before-blocks", "return-type-keyword-2"), + errors: [expectedNoSpacingError] } ] });