From 9e29e189752f06362fd1956659e07834efb746a5 Mon Sep 17 00:00:00 2001 From: Kai Cataldo <7041728+kaicataldo@users.noreply.github.com> Date: Thu, 7 Nov 2019 17:32:52 -0500 Subject: [PATCH] Fix: sourceCode#isSpaceBetweenTokens() checks non-adjacent tokens (#12491) * Fix: sourceCode#isSpaceBetweenTokens() checks non-adjacent tokens * Incorporate feedback :) * Handle args in reverse order and when args overlap * Extract helper function --- lib/source-code/source-code.js | 49 +++- tests/lib/source-code/source-code.js | 350 +++++++++++++++++++++++++-- 2 files changed, 366 insertions(+), 33 deletions(-) diff --git a/lib/source-code/source-code.js b/lib/source-code/source-code.js index 86a56803ed7..8a669095949 100644 --- a/lib/source-code/source-code.js +++ b/lib/source-code/source-code.js @@ -78,6 +78,18 @@ function sortedMerge(tokens, comments) { return result; } +/** + * Determines if two nodes or tokens overlap. + * @param {ASTNode|Token} first The first node or token to check. + * @param {ASTNode|Token} second The second node or token to check. + * @returns {boolean} True if the two nodes or tokens overlap. + * @private + */ +function nodesOrTokensOverlap(first, second) { + return (first.range[0] <= second.range[0] && first.range[1] >= second.range[0]) || + (second.range[0] <= first.range[0] && second.range[1] >= first.range[0]); +} + //------------------------------------------------------------------------------ // Public Interface //------------------------------------------------------------------------------ @@ -411,19 +423,38 @@ class SourceCode extends TokenStore { } /** - * Determines if two tokens have at least one whitespace character - * between them. This completely disregards comments in making the - * determination, so comments count as zero-length substrings. - * @param {Token} first The token to check after. - * @param {Token} second The token to check before. - * @returns {boolean} True if there is only space between tokens, false - * if there is anything other than whitespace between tokens. + * Determines if two nodes or tokens have at least one whitespace character + * between them. Order does not matter. Returns false if the given nodes or + * tokens overlap. + * @param {ASTNode|Token} first The first node or token to check between. + * @param {ASTNode|Token} second The second node or token to check between. + * @returns {boolean} True if there is a whitespace character between + * any of the tokens found between the two given nodes or tokens. * @public */ isSpaceBetweenTokens(first, second) { - const text = this.text.slice(first.range[1], second.range[0]); + if (nodesOrTokensOverlap(first, second)) { + return false; + } + + const [startingNodeOrToken, endingNodeOrToken] = first.range[1] <= second.range[0] + ? [first, second] + : [second, first]; + const firstToken = this.getLastToken(startingNodeOrToken) || startingNodeOrToken; + const finalToken = this.getFirstToken(endingNodeOrToken) || endingNodeOrToken; + let currentToken = firstToken; + + while (currentToken !== finalToken) { + const nextToken = this.getTokenAfter(currentToken, { includeComments: true }); + + if (currentToken.range[1] !== nextToken.range[0]) { + return true; + } + + currentToken = nextToken; + } - return /\s/u.test(text.replace(/\/\*.*?\*\//gus, "")); + return false; } /** diff --git a/tests/lib/source-code/source-code.js b/tests/lib/source-code/source-code.js index 9e8da484428..d4d1dd4dd27 100644 --- a/tests/lib/source-code/source-code.js +++ b/tests/lib/source-code/source-code.js @@ -1790,32 +1790,334 @@ describe("SourceCode", () => { }); describe("isSpaceBetweenTokens()", () => { + describe("should return true when there is at least one whitespace character between two tokens", () => { + leche.withData([ + ["let foo", true], + ["let foo", true], + ["let /**/ foo", true], + ["let/**/foo", false], + ["let/*\n*/foo", false] + ], (code, expected) => { + describe("when the first given is located before the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[0], + sourceCode.ast.tokens[sourceCode.ast.tokens.length - 1] + ), + expected + ); + }); + }); - leche.withData([ - ["let foo = bar;", true], - ["let foo = bar;", true], - ["let /**/ foo = bar;", true], - ["let/**/foo = bar;", false], - ["let/*\n*/foo = bar", false], - ["a+b", false], - ["a/**/+b", false], - ["a/* */+b", false], - ["a/**/ +b", true], - ["a/**/ /**/+b", true], - ["a/**/\n/**/+b", true], - ["a +b", true] - ], (code, expected) => { - - it("should return true when there is one space between tokens", () => { - const ast = espree.parse(code, DEFAULT_CONFIG), - sourceCode = new SourceCode(code, ast); + describe("when the first given is located after the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[sourceCode.ast.tokens.length - 1], + sourceCode.ast.tokens[0] + ), + expected + ); + }); + }); + }); - assert.strictEqual( - sourceCode.isSpaceBetweenTokens( - sourceCode.ast.tokens[0], sourceCode.ast.tokens[1] - ), - expected - ); + leche.withData([ + ["a+b", false], + ["a +b", true], + ["a/**/+b", false], + ["a/* */+b", false], + ["a/**/ +b", true], + ["a/**/ /**/+b", true], + ["a/* */ /* */+b", true], + ["a/**/\n/**/+b", true], + ["a/* */\n/* */+b", true], + ["a/**/+b/**/+c", false], + ["a/* */+b/* */+c", false], + ["a/**/+b /**/+c", true], + ["a/* */+b /* */+c", true], + ["a/**/ +b/**/+c", true], + ["a/* */ +b/* */+c", true], + ["a/**/+b\t/**/+c", true], + ["a/* */+b\t/* */+c", true], + ["a/**/\t+b/**/+c", true], + ["a/* */\t+b/* */+c", true], + ["a/**/+b\n/**/+c", true], + ["a/* */+b\n/* */+c", true], + ["a/**/\n+b/**/+c", true], + ["a/* */\n+b/* */+c", true], + ["a/* */+' /**/ '/* */+c", false], + ["a/* */+ ' /**/ '/* */+c", true], + ["a/* */+' /**/ ' /* */+c", true], + ["a/* */+ ' /**/ ' /* */+c", true], + ["a/* */+` /*\n*/ `/* */+c", false], + ["a/* */+ ` /*\n*/ `/* */+c", true], + ["a/* */+` /*\n*/ ` /* */+c", true], + ["a/* */+ ` /*\n*/ ` /* */+c", true] + ], (code, expected) => { + describe("when the first given is located before the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[0], + sourceCode.ast.tokens[sourceCode.ast.tokens.length - 2] + ), + expected + ); + }); + }); + + describe("when the first given is located after the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[sourceCode.ast.tokens.length - 2], + sourceCode.ast.tokens[0] + ), + expected + ); + }); + }); + }); + }); + + describe("should return true when there is at least one whitespace character between a token and a node", () => { + leche.withData([ + [";let foo = bar", false], + [";/**/let foo = bar", false], + [";/* */let foo = bar", false], + ["; let foo = bar", true], + ["; let foo = bar", true], + ["; /**/let foo = bar", true], + ["; /* */let foo = bar", true], + [";/**/ let foo = bar", true], + [";/* */ let foo = bar", true], + ["; /**/ let foo = bar", true], + ["; /* */ let foo = bar", true], + [";\tlet foo = bar", true], + [";\tlet foo = bar", true], + [";\t/**/let foo = bar", true], + [";\t/* */let foo = bar", true], + [";/**/\tlet foo = bar", true], + [";/* */\tlet foo = bar", true], + [";\t/**/\tlet foo = bar", true], + [";\t/* */\tlet foo = bar", true], + [";\nlet foo = bar", true], + [";\nlet foo = bar", true], + [";\n/**/let foo = bar", true], + [";\n/* */let foo = bar", true], + [";/**/\nlet foo = bar", true], + [";/* */\nlet foo = bar", true], + [";\n/**/\nlet foo = bar", true], + [";\n/* */\nlet foo = bar", true] + ], (code, expected) => { + describe("when the first given is located before the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[0], + sourceCode.ast.body[sourceCode.ast.body.length - 1] + ), + expected + ); + }); + }); + + describe("when the first given is located after the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.body[sourceCode.ast.body.length - 1], + sourceCode.ast.tokens[0] + ), + expected + ); + }); + }); + }); + }); + + describe("should return true when there is at least one whitespace character between a node and a token", () => { + leche.withData([ + ["let foo = bar;;", false], + ["let foo = bar;;;", false], + ["let foo = 1; let bar = 2;;", true], + ["let foo = bar;/**/;", false], + ["let foo = bar;/* */;", false], + ["let foo = bar;;;", false], + ["let foo = bar; ;", true], + ["let foo = bar; /**/;", true], + ["let foo = bar; /* */;", true], + ["let foo = bar;/**/ ;", true], + ["let foo = bar;/* */ ;", true], + ["let foo = bar; /**/ ;", true], + ["let foo = bar; /* */ ;", true], + ["let foo = bar;\t;", true], + ["let foo = bar;\t/**/;", true], + ["let foo = bar;\t/* */;", true], + ["let foo = bar;/**/\t;", true], + ["let foo = bar;/* */\t;", true], + ["let foo = bar;\t/**/\t;", true], + ["let foo = bar;\t/* */\t;", true], + ["let foo = bar;\n;", true], + ["let foo = bar;\n/**/;", true], + ["let foo = bar;\n/* */;", true], + ["let foo = bar;/**/\n;", true], + ["let foo = bar;/* */\n;", true], + ["let foo = bar;\n/**/\n;", true], + ["let foo = bar;\n/* */\n;", true] + ], (code, expected) => { + describe("when the first given is located before the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.body[0], + sourceCode.ast.tokens[sourceCode.ast.tokens.length - 1] + ), + expected + ); + }); + }); + + describe("when the first given is located after the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[sourceCode.ast.tokens.length - 1], + sourceCode.ast.body[0] + ), + expected + ); + }); + }); + }); + }); + + describe("should return true when there is at least one whitespace character between two nodes", () => { + leche.withData([ + ["let foo = bar;let baz = qux;", false], + ["let foo = bar;/**/let baz = qux;", false], + ["let foo = bar;/* */let baz = qux;", false], + ["let foo = bar; let baz = qux;", true], + ["let foo = bar; /**/let baz = qux;", true], + ["let foo = bar; /* */let baz = qux;", true], + ["let foo = bar;/**/ let baz = qux;", true], + ["let foo = bar;/* */ let baz = qux;", true], + ["let foo = bar; /**/ let baz = qux;", true], + ["let foo = bar; /* */ let baz = qux;", true], + ["let foo = bar;\tlet baz = qux;", true], + ["let foo = bar;\t/**/let baz = qux;", true], + ["let foo = bar;\t/* */let baz = qux;", true], + ["let foo = bar;/**/\tlet baz = qux;", true], + ["let foo = bar;/* */\tlet baz = qux;", true], + ["let foo = bar;\t/**/\tlet baz = qux;", true], + ["let foo = bar;\t/* */\tlet baz = qux;", true], + ["let foo = bar;\nlet baz = qux;", true], + ["let foo = bar;\n/**/let baz = qux;", true], + ["let foo = bar;\n/* */let baz = qux;", true], + ["let foo = bar;/**/\nlet baz = qux;", true], + ["let foo = bar;/* */\nlet baz = qux;", true], + ["let foo = bar;\n/**/\nlet baz = qux;", true], + ["let foo = bar;\n/* */\nlet baz = qux;", true], + ["let foo = 1;let foo2 = 2; let foo3 = 3;", true] + ], (code, expected) => { + describe("when the first given is located before the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.body[0], + sourceCode.ast.body[sourceCode.ast.body.length - 1] + ), + expected + ); + }); + }); + + describe("when the first given is located after the second", () => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.body[sourceCode.ast.body.length - 1], + sourceCode.ast.body[0] + ), + expected + ); + }); + }); + }); + }); + + describe("should return false either of the arguments' location is inside the other one", () => { + leche.withData([ + ["let foo = bar;", false] + ], (code, expected) => { + it(code, () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[0], + sourceCode.ast.body[0] + ), + expected + ); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[sourceCode.ast.tokens.length - 1], + sourceCode.ast.body[0] + ), + expected + ); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.body[0], + sourceCode.ast.tokens[0] + ), + expected + ); + + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.body[0], + sourceCode.ast.tokens[sourceCode.ast.tokens.length - 1] + ), + expected + ); + }); }); }); });