From 3960f4de647e3c81b3c0d3020de8ef156c188944 Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Thu, 20 Feb 2020 11:31:57 -0500 Subject: [PATCH] Breaking: align babel-eslint-parser & espree (#11137) * Chore: align babel-eslint-parser & espree * Start program at beginning of comment when no tokens exist * Import correct version of Espree for tests * Remove hasOwnProperty guard --- eslint/babel-eslint-parser/package.json | 1 - .../src/convert/convertAST.js | 27 ++-- .../src/convert/convertComments.js | 3 +- .../test/helpers/assert-implements-ast.js | 40 ----- eslint/babel-eslint-parser/test/index.js | 138 +++++++++++------- 5 files changed, 102 insertions(+), 107 deletions(-) delete mode 100644 eslint/babel-eslint-parser/test/helpers/assert-implements-ast.js diff --git a/eslint/babel-eslint-parser/package.json b/eslint/babel-eslint-parser/package.json index 2d219e6a33..20580eb5d5 100644 --- a/eslint/babel-eslint-parser/package.json +++ b/eslint/babel-eslint-parser/package.json @@ -31,7 +31,6 @@ "@babel/core": "^7.2.0", "@babel/eslint-shared-fixtures": "*", "eslint": "^6.0.1", - "espree": "^6.0.0", "lodash.clonedeep": "^4.5.0" } } diff --git a/eslint/babel-eslint-parser/src/convert/convertAST.js b/eslint/babel-eslint-parser/src/convert/convertAST.js index 7dfcd9a6df..4cee65d0f6 100644 --- a/eslint/babel-eslint-parser/src/convert/convertAST.js +++ b/eslint/babel-eslint-parser/src/convert/convertAST.js @@ -1,14 +1,10 @@ import { types as t, traverse } from "@babel/core"; function convertNodes(ast, code) { - const state = { source: code }; const astTransformVisitor = { noScope: true, enter(path) { - const node = path.node; - - // private var to track original node type - node._babelType = node.type; + const { node } = path; if (node.innerComments) { delete node.innerComments; @@ -23,7 +19,16 @@ function convertNodes(ast, code) { } }, exit(path) { - const node = path.node; + const { node } = path; + + // Used internally by @babel/parser. + if (node.extra) { + delete node.extra; + } + + if (node?.loc.identifierName) { + delete node.loc.identifierName; + } if (path.isTypeParameter()) { node.type = "Identifier"; @@ -74,6 +79,7 @@ function convertNodes(ast, code) { } }, }; + const state = { source: code }; // Monkey patch visitor keys in order to be able to traverse the estree nodes t.VISITOR_KEYS.Property = t.VISITOR_KEYS.ObjectProperty; @@ -95,19 +101,14 @@ function convertNodes(ast, code) { function convertProgramNode(ast) { ast.type = "Program"; ast.sourceType = ast.program.sourceType; - ast.directives = ast.program.directives; ast.body = ast.program.body; delete ast.program; + delete ast.errors; if (ast.comments.length) { const lastComment = ast.comments[ast.comments.length - 1]; - if (!ast.tokens.length) { - // if no tokens, the program starts at the end of the last comment - ast.start = lastComment.end; - ast.loc.start.line = lastComment.loc.end.line; - ast.loc.start.column = lastComment.loc.end.column; - } else { + if (ast.tokens.length) { const lastToken = ast.tokens[ast.tokens.length - 1]; if (lastComment.end > lastToken.end) { diff --git a/eslint/babel-eslint-parser/src/convert/convertComments.js b/eslint/babel-eslint-parser/src/convert/convertComments.js index 3a675a3385..854c3440c5 100644 --- a/eslint/babel-eslint-parser/src/convert/convertComments.js +++ b/eslint/babel-eslint-parser/src/convert/convertComments.js @@ -1,6 +1,5 @@ export default function convertComments(comments) { - for (let i = 0; i < comments.length; i++) { - const comment = comments[i]; + for (const comment of comments) { if (comment.type === "CommentBlock") { comment.type = "Block"; } else if (comment.type === "CommentLine") { diff --git a/eslint/babel-eslint-parser/test/helpers/assert-implements-ast.js b/eslint/babel-eslint-parser/test/helpers/assert-implements-ast.js deleted file mode 100644 index 3c435940a4..0000000000 --- a/eslint/babel-eslint-parser/test/helpers/assert-implements-ast.js +++ /dev/null @@ -1,40 +0,0 @@ -// Checks if the source ast implements the target ast. Ignores extra keys on source ast -export default function assertImplementsAST(target, source, path) { - if (!path) { - path = []; - } - - function error(text) { - const err = new Error(`At ${path.join(".")}: ${text}:`); - err.depth = path.length + 1; - throw err; - } - - const typeA = target === null ? "null" : typeof target; - const typeB = source === null ? "null" : typeof source; - if (typeA !== typeB) { - error( - `have different types (${typeA} !== ${typeB}) (${target} !== ${source})`, - ); - } else if ( - typeA === "object" && - ["RegExp"].indexOf(target.constructor.name) !== -1 && - target.constructor.name !== source.constructor.name - ) { - error( - `object have different constructors (${target.constructor.name} !== ${source.constructor.name}`, - ); - } else if (typeA === "object") { - const keysTarget = Object.keys(target); - for (const i in keysTarget) { - const key = keysTarget[i]; - path.push(key); - assertImplementsAST(target[key], source[key], path); - path.pop(); - } - } else if (target !== source) { - error( - `are different (${JSON.stringify(target)} !== ${JSON.stringify(source)})`, - ); - } -} diff --git a/eslint/babel-eslint-parser/test/index.js b/eslint/babel-eslint-parser/test/index.js index a5158b4aa6..1c14ec3f4f 100644 --- a/eslint/babel-eslint-parser/test/index.js +++ b/eslint/babel-eslint-parser/test/index.js @@ -1,54 +1,88 @@ -import assert from "assert"; -import espree from "espree"; +import path from "path"; import escope from "eslint-scope"; import unpad from "dedent"; import { parseForESLint } from "../src"; -import assertImplementsAST from "./helpers/assert-implements-ast"; -const babelOptions = { +const BABEL_OPTIONS = { configFile: require.resolve( "@babel/eslint-shared-fixtures/config/babel.config.js", ), }; +const ALLOWED_PROPERTIES = [ + "importKind", + "exportKind", + "variance", + "typeArguments", +]; -function parseAndAssertSame(code) { - code = unpad(code); - const esAST = espree.parse(code, { - ecmaFeatures: { - // enable JSX parsing - jsx: true, - // enable return in global scope - globalReturn: true, - // enable implied strict mode (if ecmaVersion >= 5) - impliedStrict: true, - // allow experimental object rest/spread - experimentalObjectRestSpread: true, - }, - tokens: true, - loc: true, - range: true, - comment: true, - ecmaVersion: 2020, - sourceType: "module", - }); - const babylonAST = parseForESLint(code, { - eslintVisitorKeys: true, - eslintScopeManager: true, - babelOptions, - }).ast; - assertImplementsAST(esAST, babylonAST); +function deeplyRemoveProperties(obj, props) { + for (const [k, v] of Object.entries(obj)) { + if (typeof v === "object") { + if (Array.isArray(v)) { + for (const el of v) { + if (el != null) deeplyRemoveProperties(el, props); + } + } + + if (props.includes(k)) delete obj[k]; + else if (v != null) deeplyRemoveProperties(v, props); + continue; + } + + if (props.includes(k)) delete obj[k]; + } } -describe("babylon-to-espree", () => { +describe("Babel and Espree", () => { + let espree; + + function parseAndAssertSame(code) { + code = unpad(code); + const espreeAST = espree.parse(code, { + ecmaFeatures: { + // enable JSX parsing + jsx: true, + // enable return in global scope + globalReturn: true, + // enable implied strict mode (if ecmaVersion >= 5) + impliedStrict: true, + // allow experimental object rest/spread + experimentalObjectRestSpread: true, + }, + tokens: true, + loc: true, + range: true, + comment: true, + ecmaVersion: 2020, + sourceType: "module", + }); + const babelAST = parseForESLint(code, { + eslintVisitorKeys: true, + eslintScopeManager: true, + babelOptions: BABEL_OPTIONS, + }).ast; + deeplyRemoveProperties(babelAST, ALLOWED_PROPERTIES); + expect(babelAST).toEqual(espreeAST); + } + + beforeAll(async () => { + // Use the version of Espree that is a dependency of + // the version of ESLint we are testing against. + const espreePath = require.resolve("espree", { + paths: [path.dirname(require.resolve("eslint"))], + }); + espree = await import(espreePath); + }); + describe("compatibility", () => { it("should allow ast.analyze to be called without options", function() { - const esAST = parseForESLint("`test`", { + const ast = parseForESLint("`test`", { eslintScopeManager: true, eslintVisitorKeys: true, - babelOptions, + babelOptions: BABEL_OPTIONS, }).ast; expect(() => { - escope.analyze(esAST); + escope.analyze(ast); }).not.toThrow(new TypeError("Should allow no options argument.")); }); }); @@ -244,9 +278,9 @@ describe("babylon-to-espree", () => { const babylonAST = parseForESLint(code, { eslintVisitorKeys: true, eslintScopeManager: true, - babelOptions, + babelOptions: BABEL_OPTIONS, }).ast; - assert.strictEqual(babylonAST.tokens[1].type, "Punctuator"); + expect(babylonAST.tokens[1].type).toEqual("Punctuator"); }); // Espree doesn't support the nullish coalescing operator yet @@ -255,9 +289,9 @@ describe("babylon-to-espree", () => { const babylonAST = parseForESLint(code, { eslintVisitorKeys: true, eslintScopeManager: true, - babelOptions, + babelOptions: BABEL_OPTIONS, }).ast; - assert.strictEqual(babylonAST.tokens[1].type, "Punctuator"); + expect(babylonAST.tokens[1].type).toEqual("Punctuator"); }); // Espree doesn't support the pipeline operator yet @@ -266,9 +300,9 @@ describe("babylon-to-espree", () => { const babylonAST = parseForESLint(code, { eslintVisitorKeys: true, eslintScopeManager: true, - babelOptions, + babelOptions: BABEL_OPTIONS, }).ast; - assert.strictEqual(babylonAST.tokens[1].type, "Punctuator"); + expect(babylonAST.tokens[1].type).toEqual("Punctuator"); }); // Espree doesn't support private fields yet @@ -277,19 +311,17 @@ describe("babylon-to-espree", () => { const babylonAST = parseForESLint(code, { eslintVisitorKeys: true, eslintScopeManager: true, - babelOptions, + babelOptions: BABEL_OPTIONS, }).ast; - assert.strictEqual(babylonAST.tokens[3].type, "Punctuator"); - assert.strictEqual(babylonAST.tokens[3].value, "#"); + expect(babylonAST.tokens[3].type).toEqual("Punctuator"); + expect(babylonAST.tokens[3].value).toEqual("#"); }); - // eslint-disable-next-line jest/no-disabled-tests - it.skip("empty program with line comment", () => { + it("empty program with line comment", () => { parseAndAssertSame("// single comment"); }); - // eslint-disable-next-line jest/no-disabled-tests - it.skip("empty program with block comment", () => { + it("empty program with block comment", () => { parseAndAssertSame(" /* multiline\n * comment\n*/"); }); @@ -448,9 +480,13 @@ describe("babylon-to-espree", () => { }); it("do not allow import export everywhere", () => { - assert.throws(() => { + expect(() => { parseAndAssertSame('function F() { import a from "a"; }'); - }, /SyntaxError: 'import' and 'export' may only appear at the top level/); + }).toThrow( + new SyntaxError( + "'import' and 'export' may only appear at the top level", + ), + ); }); it("return outside function", () => { @@ -458,9 +494,9 @@ describe("babylon-to-espree", () => { }); it("super outside method", () => { - assert.throws(() => { + expect(() => { parseAndAssertSame("function F() { super(); }"); - }, /SyntaxError: 'super' keyword outside a method/); + }).toThrow(new SyntaxError("'super' keyword outside a method")); }); it("StringLiteral", () => {