From 95cefe7e01fad54c1c945399b2d3bdbacfc1a211 Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Thu, 24 Oct 2019 18:39:02 -0400 Subject: [PATCH 1/4] Fix: sourceCode#isSpaceBetweenTokens() checks non-adjacent tokens --- lib/source-code/source-code.js | 27 ++-- tests/lib/source-code/source-code.js | 177 +++++++++++++++++++++++++-- 2 files changed, 187 insertions(+), 17 deletions(-) diff --git a/lib/source-code/source-code.js b/lib/source-code/source-code.js index 86a56803ed7..c7df1aff508 100644 --- a/lib/source-code/source-code.js +++ b/lib/source-code/source-code.js @@ -411,19 +411,28 @@ 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. + * @param {ASTNode|Token} first The node or token to check after. + * @param {ASTNode|Token} second The node or token to check before. + * @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]); + let currentToken = this.getLastToken(first) || first; - return /\s/u.test(text.replace(/\/\*.*?\*\//gus, "")); + while (currentToken !== (this.getFirstToken(second) || second)) { + const nextToken = this.getTokenAfter(currentToken, { includeComments: true }); + + if (/\s/u.test(this.text.slice(currentToken.range[1], nextToken.range[0]))) { + return true; + } + + currentToken = nextToken; + } + + return false; } /** diff --git a/tests/lib/source-code/source-code.js b/tests/lib/source-code/source-code.js index 9e8da484428..ef9198c81bb 100644 --- a/tests/lib/source-code/source-code.js +++ b/tests/lib/source-code/source-code.js @@ -1790,29 +1790,190 @@ describe("SourceCode", () => { }); describe("isSpaceBetweenTokens()", () => { + leche.withData([ + ["let foo", true], + ["let foo", true], + ["let /**/ foo", true], + ["let/**/foo", false], + ["let/*\n*/foo", false] + ], (code, expected) => { + it("should return true when there is at least one whitespace character between two tokens", () => { + 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", true], ["a/**/+b", false], ["a/* */+b", false], ["a/**/ +b", true], ["a/**/ /**/+b", true], + ["a/* */ /* */+b", true], ["a/**/\n/**/+b", true], - ["a +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) => { + it("should return true when there is at least one whitespace character between two tokens", () => { + 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 + ); + }); + }); + + 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) => { + it("should return true when there is at least one whitespace character between a token and a node", () => { + const ast = espree.parse(code, DEFAULT_CONFIG), + sourceCode = new SourceCode(code, ast); - it("should return true when there is one space between tokens", () => { + assert.strictEqual( + sourceCode.isSpaceBetweenTokens( + sourceCode.ast.tokens[0], sourceCode.ast.body[sourceCode.ast.body.length - 1] + ), + expected + ); + }); + }); + + 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) => { + it("should return true when there is at least one whitespace character between a node and a token", () => { + 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 + ); + }); + }); + + 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) => { + it("should return true when there is at least one whitespace character between two nodes", () => { const ast = espree.parse(code, DEFAULT_CONFIG), sourceCode = new SourceCode(code, ast); assert.strictEqual( sourceCode.isSpaceBetweenTokens( - sourceCode.ast.tokens[0], sourceCode.ast.tokens[1] + sourceCode.ast.body[0], sourceCode.ast.body[sourceCode.ast.body.length - 1] ), expected ); From a3fd4fc98c64ccdca21198716fde19032759bd7d Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Thu, 24 Oct 2019 19:09:05 -0400 Subject: [PATCH 2/4] Incorporate feedback :) --- lib/source-code/source-code.js | 5 +- tests/lib/source-code/source-code.js | 360 ++++++++++++++------------- 2 files changed, 187 insertions(+), 178 deletions(-) diff --git a/lib/source-code/source-code.js b/lib/source-code/source-code.js index c7df1aff508..5d7ccdaf1e3 100644 --- a/lib/source-code/source-code.js +++ b/lib/source-code/source-code.js @@ -420,12 +420,13 @@ class SourceCode extends TokenStore { * @public */ isSpaceBetweenTokens(first, second) { + const finalToken = this.getFirstToken(second) || second; let currentToken = this.getLastToken(first) || first; - while (currentToken !== (this.getFirstToken(second) || second)) { + while (currentToken !== finalToken) { const nextToken = this.getTokenAfter(currentToken, { includeComments: true }); - if (/\s/u.test(this.text.slice(currentToken.range[1], nextToken.range[0]))) { + if (currentToken.range[1] !== nextToken.range[0]) { return true; } diff --git a/tests/lib/source-code/source-code.js b/tests/lib/source-code/source-code.js index ef9198c81bb..c7d67c70557 100644 --- a/tests/lib/source-code/source-code.js +++ b/tests/lib/source-code/source-code.js @@ -1790,193 +1790,201 @@ describe("SourceCode", () => { }); describe("isSpaceBetweenTokens()", () => { - leche.withData([ - ["let foo", true], - ["let foo", true], - ["let /**/ foo", true], - ["let/**/foo", false], - ["let/*\n*/foo", false] - ], (code, expected) => { - it("should return true when there is at least one whitespace character between two tokens", () => { - 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 - ); + 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) => { + 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([ - ["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) => { - it("should return true when there is at least one whitespace character between two tokens", () => { - 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 - ); + 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) => { + 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 + ); + }); }); }); - 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) => { - it("should return true when there is at least one whitespace character between a token and a node", () => { - 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("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) => { + 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 + ); + }); }); }); - 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) => { - it("should return true when there is at least one whitespace character between a node and a token", () => { - 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("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) => { + 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 + ); + }); }); }); - 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) => { - it("should return true when there is at least one whitespace character between two nodes", () => { - 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("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) => { + 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 + ); + }); }); }); }); From 3599b9e515cb99d1ca47d7c044368e27c5fb7009 Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Wed, 6 Nov 2019 20:54:39 -0500 Subject: [PATCH 3/4] Handle args in reverse order and when args overlap --- lib/source-code/source-code.js | 22 ++- tests/lib/source-code/source-code.js | 207 ++++++++++++++++++++++----- 2 files changed, 187 insertions(+), 42 deletions(-) diff --git a/lib/source-code/source-code.js b/lib/source-code/source-code.js index 5d7ccdaf1e3..3ebff96d236 100644 --- a/lib/source-code/source-code.js +++ b/lib/source-code/source-code.js @@ -412,16 +412,28 @@ class SourceCode extends TokenStore { /** * Determines if two nodes or tokens have at least one whitespace character - * between them. - * @param {ASTNode|Token} first The node or token to check after. - * @param {ASTNode|Token} second The node or token to check before. + * 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 finalToken = this.getFirstToken(second) || second; - let currentToken = this.getLastToken(first) || first; + + // Arguments are overlapping. + if (first.range[0] <= second.range[0] && first.range[1] >= second.range[0] || + second.range[0] <= first.range[0] && second.range[1] >= first.range[0]) { + return false; + } + + const nodesAreReversed = second.range[1] <= first.range[0]; + const startingNodeOrToken = nodesAreReversed ? second : first; + const endingNodeOrToken = nodesAreReversed ? first : second; + 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 }); diff --git a/tests/lib/source-code/source-code.js b/tests/lib/source-code/source-code.js index c7d67c70557..d4d1dd4dd27 100644 --- a/tests/lib/source-code/source-code.js +++ b/tests/lib/source-code/source-code.js @@ -1798,16 +1798,34 @@ describe("SourceCode", () => { ["let/**/foo", false], ["let/*\n*/foo", false] ], (code, expected) => { - it(code, () => { - const ast = espree.parse(code, DEFAULT_CONFIG), - sourceCode = new SourceCode(code, ast); + 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 + ); + }); + }); - assert.strictEqual( - sourceCode.isSpaceBetweenTokens( - sourceCode.ast.tokens[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.tokens[0] + ), + expected + ); + }); }); }); @@ -1844,16 +1862,34 @@ describe("SourceCode", () => { ["a/* */+` /*\n*/ ` /* */+c", true], ["a/* */+ ` /*\n*/ ` /* */+c", true] ], (code, expected) => { - it(code, () => { - const ast = espree.parse(code, DEFAULT_CONFIG), - sourceCode = new SourceCode(code, ast); + 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 + ); + }); + }); - 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 + ); + }); }); }); }); @@ -1888,16 +1924,34 @@ describe("SourceCode", () => { [";\n/**/\nlet foo = bar", true], [";\n/* */\nlet foo = bar", true] ], (code, expected) => { - it(code, () => { - const ast = espree.parse(code, DEFAULT_CONFIG), - sourceCode = new SourceCode(code, ast); + 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 + ); + }); + }); - 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 + ); + }); }); }); }); @@ -1932,16 +1986,34 @@ describe("SourceCode", () => { ["let foo = bar;\n/**/\n;", true], ["let foo = bar;\n/* */\n;", true] ], (code, expected) => { - it(code, () => { - const ast = espree.parse(code, DEFAULT_CONFIG), - sourceCode = new SourceCode(code, ast); + 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 + ); + }); + }); - 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 + ); + }); }); }); }); @@ -1973,6 +2045,42 @@ describe("SourceCode", () => { ["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), @@ -1980,7 +2088,32 @@ describe("SourceCode", () => { assert.strictEqual( sourceCode.isSpaceBetweenTokens( - sourceCode.ast.body[0], sourceCode.ast.body[sourceCode.ast.body.length - 1] + 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 ); From 8a7042f7966d241adbe6aeafa62a2386effbaa2a Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Wed, 6 Nov 2019 21:04:39 -0500 Subject: [PATCH 4/4] Extract helper function --- lib/source-code/source-code.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/source-code/source-code.js b/lib/source-code/source-code.js index 3ebff96d236..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 //------------------------------------------------------------------------------ @@ -421,16 +433,13 @@ class SourceCode extends TokenStore { * @public */ isSpaceBetweenTokens(first, second) { - - // Arguments are overlapping. - if (first.range[0] <= second.range[0] && first.range[1] >= second.range[0] || - second.range[0] <= first.range[0] && second.range[1] >= first.range[0]) { + if (nodesOrTokensOverlap(first, second)) { return false; } - const nodesAreReversed = second.range[1] <= first.range[0]; - const startingNodeOrToken = nodesAreReversed ? second : first; - const endingNodeOrToken = nodesAreReversed ? first : second; + 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;