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

@@ -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 <div></div> to <div />
-        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 });

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;

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 <div></div> to <div />
+    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 };

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 };

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));
+        const ast = parse(source, mergedOptions);
-        let ppast;
+        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();
+          expect(ppastMassaged).toBeDefined();
-          if (ast.errors.length === 0) {
+          if (!ast.errors || ast.errors.length === 0) {
-            expect(ast).toEqual(ppast);
+            expect(astMassaged).toEqual(ppastMassaged);
           }
         });
       }
@@ -97,8 +88,8 @@ function stripLocation(ast) {
   return ast;
 }
 
-function parse(string) {
+function parse(string, opts) {
-  return stripLocation(parser.parseWithFlow(string));
+  return stripLocation(parser.parse(string, opts));
 }
 
 function prettyprint(src, filename, options) {