From 18fd0149859e98e051158db97dce321f8008c4a6 Mon Sep 17 00:00:00 2001 From: Joseph Frazier <1212jtraceur@gmail.com> Date: Mon, 8 May 2017 21:16:35 -0400 Subject: [PATCH] Run AST comparison tests on Travis (#1553) * Run AST comparison tests on Travis It looks like some of these currently fail, so we should probably also sort that out. Inspired by https://github.com/prettier/prettier/issues/1552 * tests: Use specified parser when AST_COMPARE=1 This fixes some of the tests with AST_COMPARE=1 * Move cleanAST() into prettier.__debug This makes it available for tests to use. * AST_COMPARE=1 uses cleanAst() instead of removeEmptyStatements() Ths fixes some of the tests with AST_COMPARE=1 * Export parse() from src/parser.js This makes it available for tests to use. * tests: Use specified parser more when AST_COMPARE=1 This is a continuation of commit 86437a66d326919897fe89891a25824870f5bb79 This fixes some of the tests with AST_COMPARE=1 * massageAST: remove leadingComments/trailingComments This fixes some of the tests with AST_COMPARE=1 * massageAST: remove `extra` This fixes some of the tests with AST_COMPARE=1 * tests_config/run_spec.js: Rename variables for clarity * AST_COMPARE=1 tests compare unstringified objects This makes the test error output shorter. * fixup! Export parse() from src/parser.js * Revert "Run AST comparison tests on Travis" See https://github.com/prettier/prettier/pull/1553#issuecomment-300027747 This reverts commit 49873a956c532f23fd216551a35ae35c1a18407e. * fixup! fixup! Export parse() from src/parser.js * parser: Require babel-code-frame only when needed This addresses: * https://github.com/prettier/prettier/pull/1553#discussion_r115386253 * https://github.com/prettier/prettier/pull/1553#discussion_r115386250 * parser: Don't export now-unused parseWith* functions Addresses https://github.com/prettier/prettier/pull/1553#discussion_r115386964 * Move cleanAST/massageAST into own file, don't export This addresses: * https://github.com/prettier/prettier/pull/1553#discussion_r115386993 * https://github.com/prettier/prettier/pull/1553#discussion_r115386611 * Don't destructure require() result (Node v4 compat.) * Fix copy/paste error --- bin/prettier.js | 70 +------------------------------------ index.js | 34 ++---------------- src/clean-ast.js | 75 ++++++++++++++++++++++++++++++++++++++++ src/parser.js | 30 +++++++++++++++- tests_config/run_spec.js | 33 +++++++----------- 5 files changed, 120 insertions(+), 122 deletions(-) create mode 100644 src/clean-ast.js diff --git a/bin/prettier.js b/bin/prettier.js index ac1dc4e74..5a3bb70e9 100755 --- a/bin/prettier.js +++ b/bin/prettier.js @@ -9,6 +9,7 @@ const chalk = require("chalk"); const minimist = require("minimist"); const readline = require("readline"); const prettier = require("../index"); +const cleanAST = require('../src/clean-ast.js').cleanAST; const argv = minimist(process.argv.slice(2), { boolean: [ @@ -139,75 +140,6 @@ function format(input) { } if (argv["debug-check"]) { - function massageAST(ast) { - if (Array.isArray(ast)) { - return ast.map(e => massageAST(e)).filter(e => e); - } - if (ast && typeof ast === "object") { - // We remove extra `;` and add them when needed - if (ast.type === "EmptyStatement") { - return undefined; - } - - // We move text around, including whitespaces and add {" "} - if (ast.type === "JSXText") { - return undefined; - } - if ( - ast.type === "JSXExpressionContainer" && - ast.expression.type === "Literal" && - ast.expression.value === " " - ) { - return undefined; - } - - const newObj = {}; - for (var key in ast) { - newObj[key] = massageAST(ast[key]); - } - - [ - "loc", - "range", - "raw", - "comments", - "start", - "end", - "tokens", - "flags" - ].forEach(name => { - delete newObj[name]; - }); - - // We convert
to
- if (ast.type === "JSXOpeningElement") { - delete newObj.selfClosing; - } - if (ast.type === "JSXElement") { - delete newObj.closingElement; - } - - // We change {'key': value} into {key: value} - if ( - (ast.type === "Property" || - ast.type === "MethodDefinition" || - ast.type === "ClassProperty") && - typeof ast.key === "object" && - ast.key && - (ast.key.type === "Literal" || ast.key.type === "Identifier") - ) { - delete newObj.key; - } - - return newObj; - } - return ast; - } - - function cleanAST(ast) { - return JSON.stringify(massageAST(ast), null, 2); - } - function diff(a, b) { return require("diff") .createTwoFilesPatch("", "", a, b, "", "", { context: 2 }); diff --git a/index.js b/index.js index 834896c21..e2464d1ae 100644 --- a/index.js +++ b/index.js @@ -1,6 +1,5 @@ "use strict"; -const codeFrame = require("babel-code-frame"); const comments = require("./src/comments"); const version = require("./package.json").version; const printAstToDoc = require("./src/printer").printAstToDoc; @@ -17,33 +16,6 @@ function guessLineEnding(text) { return "\n"; } -function parse(text, opts) { - let parseFunction; - - if (opts.parser === "flow") { - parseFunction = parser.parseWithFlow; - } else if (opts.parser === "typescript") { - parseFunction = parser.parseWithTypeScript; - } else { - parseFunction = parser.parseWithBabylon; - } - - try { - return parseFunction(text); - } catch (error) { - const loc = error.loc; - - if (loc) { - error.codeFrame = codeFrame(text, loc.line, loc.column + 1, { - highlightCode: true - }); - error.message += "\n" + error.codeFrame; - } - - throw error; - } -} - function attachComments(text, ast, opts) { const astComments = ast.comments; if (astComments) { @@ -77,7 +49,7 @@ function ensureAllCommentsPrinted(astComments) { } function format(text, opts) { - const ast = parse(text, opts); + const ast = parser.parse(text, opts); const astComments = attachComments(text, ast, opts); const doc = printAstToDoc(ast, opts); opts.newLine = guessLineEnding(text); @@ -115,7 +87,7 @@ module.exports = { version: version, __debug: { parse: function(text, opts) { - return parse(text, opts); + return parser.parse(text, opts); }, formatAST: function(ast, opts) { opts = normalizeOptions(opts); @@ -132,7 +104,7 @@ module.exports = { }, printToDoc: function(text, opts) { opts = normalizeOptions(opts); - const ast = parse(text, opts); + const ast = parser.parse(text, opts); attachComments(text, ast, opts); const doc = printAstToDoc(ast, opts); return doc; diff --git a/src/clean-ast.js b/src/clean-ast.js new file mode 100644 index 000000000..4ecd483bd --- /dev/null +++ b/src/clean-ast.js @@ -0,0 +1,75 @@ +"use strict"; + +function cleanAST(ast) { + return JSON.stringify(massageAST(ast), null, 2); +} + +function massageAST(ast) { + if (Array.isArray(ast)) { + return ast.map(e => massageAST(e)).filter(e => e); + } + if (ast && typeof ast === "object") { + // We remove extra `;` and add them when needed + if (ast.type === "EmptyStatement") { + return undefined; + } + + // We move text around, including whitespaces and add {" "} + if (ast.type === "JSXText") { + return undefined; + } + if ( + ast.type === "JSXExpressionContainer" && + ast.expression.type === "Literal" && + ast.expression.value === " " + ) { + return undefined; + } + + const newObj = {}; + for (var key in ast) { + newObj[key] = massageAST(ast[key]); + } + + [ + "loc", + "range", + "raw", + "comments", + "leadingComments", + "trailingComments", + "extra", + "start", + "end", + "tokens", + "flags" + ].forEach(name => { + delete newObj[name]; + }); + + // We convert
to
+ if (ast.type === "JSXOpeningElement") { + delete newObj.selfClosing; + } + if (ast.type === "JSXElement") { + delete newObj.closingElement; + } + + // We change {'key': value} into {key: value} + if ( + (ast.type === "Property" || + ast.type === "MethodDefinition" || + ast.type === "ClassProperty") && + typeof ast.key === "object" && + ast.key && + (ast.key.type === "Literal" || ast.key.type === "Identifier") + ) { + delete newObj.key; + } + + return newObj; + } + return ast; +} + +module.exports = { cleanAST, massageAST }; diff --git a/src/parser.js b/src/parser.js index 426fcbc40..4c3317ac0 100644 --- a/src/parser.js +++ b/src/parser.js @@ -7,6 +7,34 @@ function createError(message, line, column) { return error; } +function parse(text, opts) { + let parseFunction; + + if (opts.parser === "flow") { + parseFunction = parseWithFlow; + } else if (opts.parser === "typescript") { + parseFunction = parseWithTypeScript; + } else { + parseFunction = parseWithBabylon; + } + + try { + return parseFunction(text); + } catch (error) { + const loc = error.loc; + + if (loc) { + const codeFrame = require("babel-code-frame"); + error.codeFrame = codeFrame(text, loc.line, loc.column + 1, { + highlightCode: true + }); + error.message += "\n" + error.codeFrame; + } + + throw error; + } +} + function parseWithFlow(text) { // Inline the require to avoid loading all the JS if we don't use it const flowParser = require("flow-parser"); @@ -97,4 +125,4 @@ function isProbablyJsx(text) { ].join(""), "m").test(text); } -module.exports = { parseWithFlow, parseWithBabylon, parseWithTypeScript }; +module.exports = { parse }; diff --git a/tests_config/run_spec.js b/tests_config/run_spec.js index 2b2452d5d..6643cd1d9 100644 --- a/tests_config/run_spec.js +++ b/tests_config/run_spec.js @@ -5,6 +5,7 @@ const extname = require("path").extname; const prettier = require("../"); const types = require("../src/ast-types"); const parser = require("../src/parser"); +const massageAST = require('../src/clean-ast.js').massageAST; const RUN_AST_TESTS = process.env["AST_COMPARE"]; const VERIFY_ALL_PARSERS = process.env["VERIFY_ALL_PARSERS"] || false; @@ -12,27 +13,15 @@ const ALL_PARSERS = process.env["ALL_PARSERS"] ? JSON.parse(process.env["ALL_PARSERS"]) : ["flow", "babylon", "typescript"]; -// Ignoring empty statements that are added into the output removes a -// lot of noise from test failures and let's us focus on the real -// failures when comparing asts -function removeEmptyStatements(ast) { - return types.visit(ast, { - visitEmptyStatement: function(path) { - path.prune(); - return false; - } - }); -} - function run_spec(dirname, options, additionalParsers) { fs.readdirSync(dirname).forEach(filename => { const extension = extname(filename); if (/^\.[jt]sx?$/.test(extension) && filename !== "jsfmt.spec.js") { const path = dirname + "/" + filename; + const mergedOptions = mergeDefaultOptions(options || {}); if (!RUN_AST_TESTS) { const source = read(path).replace(/\r\n/g, "\n"); - const mergedOptions = mergeDefaultOptions(options || {}); const output = prettyprint(source, path, mergedOptions); test(`${mergedOptions.parser} - ${parser.parser}-verify`, () => { expect(raw(source + "~".repeat(80) + "\n" + output)).toMatchSnapshot( @@ -56,20 +45,22 @@ function run_spec(dirname, options, additionalParsers) { if (RUN_AST_TESTS) { const source = read(dirname + "/" + filename); - const ast = removeEmptyStatements(parse(source)); - let ppast; + const ast = parse(source, mergedOptions); + const astMassaged = massageAST(ast); + let ppastMassaged; let pperr = null; try { - ppast = removeEmptyStatements(parse(prettyprint(source, path))); + const ppast = parse(prettyprint(source, path, mergedOptions), mergedOptions) + ppastMassaged = massageAST(ppast); } catch (e) { pperr = e.stack; } test(path + " parse", () => { expect(pperr).toBe(null); - expect(ppast).toBeDefined(); - if (ast.errors.length === 0) { - expect(ast).toEqual(ppast); + expect(ppastMassaged).toBeDefined(); + if (!ast.errors || ast.errors.length === 0) { + expect(astMassaged).toEqual(ppastMassaged); } }); } @@ -97,8 +88,8 @@ function stripLocation(ast) { return ast; } -function parse(string) { - return stripLocation(parser.parseWithFlow(string)); +function parse(string, opts) { + return stripLocation(parser.parse(string, opts)); } function prettyprint(src, filename, options) {