Don't default parser to babylon (#4528)

* Don't default parser to babylon

* Different default values according to version

* Fix error logging

* Add tests specifically for parser inference

* Add another test case

* Add API test

index.js

@@ -42,12 +42,8 @@ module.exports = {
   },
 
   check: function(text, opts) {
-    try {
-      const formatted = formatWithCursor(text, opts).formatted;
-      return formatted === text;
-    } catch (e) {
-      return false;
-    }
+    const formatted = formatWithCursor(text, opts).formatted;
+    return formatted === text;
   },
 
   doc,

src/cli/util.js

@@ -52,6 +52,23 @@ function diff(a, b) {
 }
 
 function handleError(context, filename, error) {
+  if (error instanceof errors.UndefinedParserError) {
+    if (context.argv["write"] && process.stdout.isTTY) {
+      readline.clearLine(process.stdout, 0);
+      readline.cursorTo(process.stdout, 0, null);
+    }
+    if (!context.argv["list-different"]) {
+      process.exitCode = 2;
+    }
+    context.logger.error(error.message);
+    return;
+  }
+
+  if (context.argv["write"]) {
+    // Add newline to split errors from filename line.
+    process.stdout.write("\n");
+  }
+
   const isParseError = Boolean(error && error.loc);
   const isValidationError = /Validation Error/.test(error && error.message);
 
@@ -119,13 +136,15 @@ function listDifferent(context, input, options, filename) {
     return;
   }
 
-  options = Object.assign({}, options, { filepath: filename });
-
-  if (!prettier.check(input, options)) {
-    if (!context.argv["write"]) {
-      context.logger.log(filename);
+  try {
+    if (!prettier.check(input, options)) {
+      if (!context.argv["write"]) {
+        context.logger.log(filename);
+      }
+      process.exitCode = 1;
     }
-    process.exitCode = 1;
+  } catch (error) {
+    context.logger.error(error.message);
   }
 
   return true;
@@ -286,11 +305,11 @@ function formatStdin(context) {
 
     const options = getOptionsForFile(context, filepath);
 
-    if (listDifferent(context, input, options, "(stdin)")) {
-      return;
-    }
-
     try {
+      if (listDifferent(context, input, options, "(stdin)")) {
+        return;
+      }
+
       writeOutput(format(context, input, options), options);
     } catch (error) {
       handleError(context, "stdin", error);
@@ -329,7 +348,12 @@ function eachFilename(context, patterns, callback) {
       return;
     }
     filePaths.forEach(filePath =>
-      callback(filePath, getOptionsForFile(context, filePath))
+      callback(
+        filePath,
+        Object.assign(getOptionsForFile(context, filePath), {
+          filepath: filePath
+        })
+      )
     );
   } catch (error) {
     context.logger.error(
@@ -381,8 +405,6 @@ function formatFiles(context) {
       return;
     }
 
-    listDifferent(context, input, options, filename);
-
     const start = Date.now();
 
     let result;
@@ -396,13 +418,17 @@ function formatFiles(context) {
       );
       output = result.formatted;
     } catch (error) {
-      // Add newline to split errors from filename line.
-      process.stdout.write("\n");
-
       handleError(context, filename, error);
       return;
     }
 
+    const isDifferent = output !== input;
+
+    if (context.argv["list-different"] && isDifferent) {
+      context.logger.log(filename);
+      process.exitCode = 1;
+    }
+
     if (context.argv["write"]) {
       if (process.stdout.isTTY) {
         // Remove previously printed filename to log it with duration.
@@ -412,14 +438,8 @@ function formatFiles(context) {
 
       // Don't write the file if it won't change in order not to invalidate
       // mtime based caches.
-      if (output === input) {
+      if (isDifferent) {
         if (!context.argv["list-different"]) {
-          context.logger.log(`${chalk.grey(filename)} ${Date.now() - start}ms`);
-        }
-      } else {
-        if (context.argv["list-different"]) {
-          context.logger.log(filename);
-        } else {
           context.logger.log(`${filename} ${Date.now() - start}ms`);
         }
 
@@ -432,6 +452,8 @@ function formatFiles(context) {
           // Don't exit the process if one file failed
           process.exitCode = 2;
         }
+      } else if (!context.argv["list-different"]) {
+        context.logger.log(`${chalk.grey(filename)} ${Date.now() - start}ms`);
       }
     } else if (context.argv["debug-check"]) {
       if (output) {

src/common/errors.js

@@ -2,8 +2,10 @@
 
 class ConfigError extends Error {}
 class DebugError extends Error {}
+class UndefinedParserError extends Error {}
 
 module.exports = {
   ConfigError,
-  DebugError
+  DebugError,
+  UndefinedParserError
 };

src/main/core-options.js

@@ -85,7 +85,10 @@ const options = {
     since: "0.0.10",
     category: CATEGORY_GLOBAL,
     type: "choice",
-    default: "babylon",
+    default: [
+      { since: "0.0.10", value: "babylon" },
+      { since: "1.13.0", value: undefined }
+    ],
     description: "Which parser to use.",
     exception: value =>
       typeof value === "string" || typeof value === "function",

src/main/core.js

@@ -255,6 +255,7 @@ function format(text, opts) {
 
 module.exports = {
   formatWithCursor(text, opts) {
+    opts = normalizeOptions(opts);
     return format(text, normalizeOptions(opts));
   },
 
@@ -275,8 +276,8 @@ module.exports = {
 
   // Doesn't handle shebang for now
   formatDoc(doc, opts) {
-    opts = normalizeOptions(opts);
     const debug = printDocToDebug(doc);
+    opts = normalizeOptions(Object.assign({}, opts, { parser: "babylon" }));
     return format(debug, opts).formatted;
   },
 

src/main/multiparser.js

@@ -21,7 +21,7 @@ function textToDoc(text, partialNextOptions, parentOptions) {
       parentParser: parentOptions.parser,
       originalText: text
     }),
-    { passThrough: true, inferParser: false }
+    { passThrough: true }
   );
 
   const result = require("./parser").parse(text, nextOptions);

src/main/options.js

@@ -1,6 +1,7 @@
 "use strict";
 
 const path = require("path");
+const UndefinedParserError = require("../common/errors").UndefinedParserError;
 const getSupportInfo = require("../main/support").getSupportInfo;
 const normalizer = require("./options-normalizer");
 const resolveParser = require("./parser").resolveParser;
@@ -29,30 +30,27 @@ function normalize(options, opts) {
     Object.assign({}, hiddenDefaults)
   );
 
-  if (opts.inferParser !== false) {
-    if (
-      rawOptions.filepath &&
-      (!rawOptions.parser || rawOptions.parser === defaults.parser)
-    ) {
-      const inferredParser = inferParser(
-        rawOptions.filepath,
-        rawOptions.plugins
+  if (!rawOptions.parser) {
+    if (!rawOptions.filepath) {
+      throw new UndefinedParserError(
+        "No parser and no file path given, couldn't infer a parser."
+      );
+    }
+
+    rawOptions.parser = inferParser(rawOptions.filepath, rawOptions.plugins);
+    if (!rawOptions.parser) {
+      throw new UndefinedParserError(
+        `No parser could be inferred for file: ${rawOptions.filepath}`
       );
-      if (inferredParser) {
-        rawOptions.parser = inferredParser;
-      }
     }
   }
 
   const parser = resolveParser(
-    !rawOptions.parser
-      ? rawOptions
-      : // handle deprecated parsers
-        normalizer.normalizeApiOptions(
-          rawOptions,
-          [supportOptions.find(x => x.name === "parser")],
-          { passThrough: true, logger: false }
-        )
+    normalizer.normalizeApiOptions(
+      rawOptions,
+      [supportOptions.find(x => x.name === "parser")],
+      { passThrough: true, logger: false }
+    )
   );
   rawOptions.astFormat = parser.astFormat;
   rawOptions.locEnd = parser.locEnd;

src/main/parser.js

@@ -43,8 +43,6 @@ function resolveParser(opts, parsers) {
       throw new ConfigError(`Couldn't resolve parser "${opts.parser}"`);
     }
   }
-  /* istanbul ignore next */
-  return parsers.babylon;
 }
 
 function parse(text, opts) {

tests/cursor/jsfmt.spec.js

@@ -3,7 +3,9 @@ run_spec(__dirname, ["babylon", "typescript", "flow"]);
 const prettier = require("../../tests_config/require_prettier");
 
 test("translates cursor correctly in basic case", () => {
-  expect(prettier.formatWithCursor(" 1", { cursorOffset: 2 })).toEqual({
+  expect(
+    prettier.formatWithCursor(" 1", { parser: "babylon", cursorOffset: 2 })
+  ).toEqual({
     formatted: "1;\n",
     cursorOffset: 1
   });
@@ -11,7 +13,9 @@ test("translates cursor correctly in basic case", () => {
 
 test("positions cursor relative to closest node, not SourceElement", () => {
   const code = "return         15";
-  expect(prettier.formatWithCursor(code, { cursorOffset: 15 })).toEqual({
+  expect(
+    prettier.formatWithCursor(code, { parser: "babylon", cursorOffset: 15 })
+  ).toEqual({
     formatted: "return 15;\n",
     cursorOffset: 7
   });
@@ -19,7 +23,9 @@ test("positions cursor relative to closest node, not SourceElement", () => {
 
 test("keeps cursor inside formatted node", () => {
   const code = "return         15";
-  expect(prettier.formatWithCursor(code, { cursorOffset: 14 })).toEqual({
+  expect(
+    prettier.formatWithCursor(code, { parser: "babylon", cursorOffset: 14 })
+  ).toEqual({
     formatted: "return 15;\n",
     cursorOffset: 7
   });
@@ -30,7 +36,9 @@ test("doesn't insert second placeholder for nonexistent TypeAnnotation", () => {
 foo('bar', cb => {
   console.log('stuff')
 })`;
-  expect(prettier.formatWithCursor(code, { cursorOffset: 24 })).toEqual({
+  expect(
+    prettier.formatWithCursor(code, { parser: "babylon", cursorOffset: 24 })
+  ).toEqual({
     formatted: `foo("bar", cb => {
   console.log("stuff");
 });

tests_integration/__tests__/__snapshots__/debug-check.js.snap

@@ -30,10 +30,6 @@ exports[`show diff for 2+ error files with --debug-check (stderr) 1`] = `
 "
 `;
 
-exports[`show diff for 2+ error files with --debug-check (stdout) 1`] = `
-"
-
-"
-`;
+exports[`show diff for 2+ error files with --debug-check (stdout) 1`] = `""`;
 
 exports[`show diff for 2+ error files with --debug-check (write) 1`] = `Array []`;

tests_integration/__tests__/__snapshots__/early-exit.js.snap

@@ -296,8 +296,6 @@ Valid options:
   graphql          GraphQL
   markdown         Markdown
   vue              Vue
-
-Default: babylon
 "
 `;
 
@@ -597,7 +595,6 @@ Format options:
                            Defaults to false.
   --parser <flow|babylon|typescript|css|less|scss|json|json5|json-stringify|graphql|markdown|vue>
                            Which parser to use.
-                           Defaults to babylon.
   --print-width <int>      The line length where Prettier will try wrap.
                            Defaults to 80.
   --prose-wrap <always|never|preserve>
@@ -717,8 +714,6 @@ Valid options:
   graphql          GraphQL
   markdown         Markdown
   vue              Vue
-
-Default: babylon
 "
 `;
 
@@ -749,7 +744,6 @@ Format options:
                            Defaults to false.
   --parser <flow|babylon|typescript|css|less|scss|json|json5|json-stringify|graphql|markdown|vue>
                            Which parser to use.
-                           Defaults to babylon.
   --print-width <int>      The line length where Prettier will try wrap.
                            Defaults to 80.
   --prose-wrap <always|never|preserve>

tests_integration/__tests__/__snapshots__/infer-parser.js.snap

@@ -0,0 +1,98 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[` 1`] = `"No parser and no file path given, couldn't infer a parser."`;
+
+exports[`--list-different with unknown path and no parser multiple files (stderr) 1`] = `
+"[error] No parser could be inferred for file: FOO
+"
+`;
+
+exports[`--list-different with unknown path and no parser specific file (stderr) 1`] = `
+"[error] No parser could be inferred for file: FOO
+"
+`;
+
+exports[`--write and --list-different with unknown path and no parser multiple files (stderr) 1`] = `
+"[error] No parser could be inferred for file: FOO
+"
+`;
+
+exports[`--write and --list-different with unknown path and no parser multiple files (stdout) 1`] = `
+"foo.js
+"
+`;
+
+exports[`--write and --list-different with unknown path and no parser multiple files (write) 1`] = `
+Array [
+  Object {
+    "content": "foo();
+",
+    "filename": "foo.js",
+  },
+]
+`;
+
+exports[`--write and --list-different with unknown path and no parser specific file (stderr) 1`] = `
+"[error] No parser could be inferred for file: FOO
+"
+`;
+
+exports[`--write with unknown path and no parser multiple files (stderr) 1`] = `
+"[error] No parser could be inferred for file: FOO
+"
+`;
+
+exports[`--write with unknown path and no parser multiple files (stdout) 1`] = `
+"foo.js 0ms
+"
+`;
+
+exports[`--write with unknown path and no parser multiple files (write) 1`] = `
+Array [
+  Object {
+    "content": "foo();
+",
+    "filename": "foo.js",
+  },
+]
+`;
+
+exports[`--write with unknown path and no parser specific file (stderr) 1`] = `
+"[error] No parser could be inferred for file: FOO
+"
+`;
+
+exports[`stdin no path and no parser --list-different logs error but exits with 0 (stderr) 1`] = `
+"[error] No parser and no file path given, couldn't infer a parser.
+"
+`;
+
+exports[`stdin no path and no parser logs error and exits with 2 (stderr) 1`] = `
+"[error] No parser and no file path given, couldn't infer a parser.
+"
+`;
+
+exports[`stdin with unknown path and no parser --list-different logs error but exits with 0 (stderr) 1`] = `
+"[error] No parser could be inferred for file: foo
+"
+`;
+
+exports[`stdin with unknown path and no parser logs error and exits with 2 (stderr) 1`] = `
+"[error] No parser could be inferred for file: foo
+"
+`;
+
+exports[`unknown path and no parser multiple files (stderr) 1`] = `
+"[error] No parser could be inferred for file: FOO
+"
+`;
+
+exports[`unknown path and no parser multiple files (stdout) 1`] = `
+"foo();
+"
+`;
+
+exports[`unknown path and no parser specific file (stderr) 1`] = `
+"[error] No parser could be inferred for file: FOO
+"
+`;

tests_integration/__tests__/__snapshots__/plugin-options.js.snap

@@ -17,7 +17,7 @@ exports[` 1`] = `
                              Defaults to false.
     --parser <flow|babylon|typescript|css|less|scss|json|json5|json-stringify|graphql|markdown|vue>
                              Which parser to use.
-                             Defaults to babylon."
+    --print-width <int>      The line length where Prettier will try wrap."
 `;
 
 exports[`show detailed external option with \`--help foo-option\` (stderr) 1`] = `""`;

tests_integration/__tests__/config-invalid.js

@@ -40,14 +40,26 @@ describe("throw error with invalid config precedence option (configPrecedence)",
   });
 });
 
+// Tests below require --parser to prevent an error (no parser/filepath specified)
+
 describe("show warning with unknown option", () => {
-  runPrettier("cli/config/invalid", ["--config", "option/unknown"]).test({
+  runPrettier("cli/config/invalid", [
+    "--config",
+    "option/unknown",
+    "--parser",
+    "babylon"
+  ]).test({
     status: 0
   });
 });
 
 describe("show warning with kebab-case option key", () => {
-  runPrettier("cli/config/invalid", ["--config", "option/kebab-case"]).test({
+  runPrettier("cli/config/invalid", [
+    "--config",
+    "option/kebab-case",
+    "--parser",
+    "babylon"
+  ]).test({
     status: 0
   });
 });

tests_integration/__tests__/cursor-offset.js

@@ -3,13 +3,15 @@
 const runPrettier = require("../runPrettier");
 
 describe("write cursorOffset to stderr with --cursor-offset <int>", () => {
-  runPrettier("cli", ["--cursor-offset", "2"], { input: " 1" }).test({
+  runPrettier("cli", ["--cursor-offset", "2", "--parser", "babylon"], {
+    input: " 1"
+  }).test({
     status: 0
   });
 });
 
 describe("cursorOffset should not be affected by full-width character", () => {
-  runPrettier("cli", ["--cursor-offset", "21"], {
+  runPrettier("cli", ["--cursor-offset", "21", "--parser", "babylon"], {
     input: `const x = ["中文", "中文", "中文", "中文", "中文", "中文", "中文", "中文", "中文", "中文", "中文"];`
     //                              ^ offset = 21                              ^ width = 80
   }).test({

tests_integration/__tests__/debug-check.js

@@ -11,7 +11,7 @@ describe("doesn't crash when --debug-check is passed", () => {
 });
 
 describe("checks stdin with --debug-check", () => {
-  runPrettier("cli/with-shebang", ["--debug-check"], {
+  runPrettier("cli/with-shebang", ["--debug-check", "--parser", "babylon"], {
     input: "0"
   }).test({
     stdout: "(stdin)\n",

tests_integration/__tests__/debug-print-doc.js

@@ -3,9 +3,11 @@
 const runPrettier = require("../runPrettier");
 
 describe("prints doc with --debug-print-doc", () => {
-  runPrettier("cli/with-shebang", ["--debug-print-doc"], {
-    input: "0"
-  }).test({
+  runPrettier(
+    "cli/with-shebang",
+    ["--debug-print-doc", "--parser", "babylon"],
+    { input: "0" }
+  ).test({
     stdout: '["0", ";", hardline, breakParent];\n',
     stderr: "",
     status: 0

tests_integration/__tests__/infer-parser.js

@@ -0,0 +1,123 @@
+"use strict";
+
+const runPrettier = require("../runPrettier");
+const prettier = require("../../tests_config/require_prettier");
+
+describe("stdin no path and no parser", () => {
+  describe("logs error and exits with 2", () => {
+    runPrettier("cli/infer-parser/", ["--stdin"], { input: "foo" }).test({
+      status: 2,
+      stdout: "",
+      write: []
+    });
+  });
+
+  describe("--list-different logs error but exits with 0", () => {
+    runPrettier("cli/infer-parser/", ["--list-different", "--stdin"], {
+      input: "foo"
+    }).test({
+      status: 0,
+      stdout: "",
+      write: []
+    });
+  });
+});
+
+describe("stdin with unknown path and no parser", () => {
+  describe("logs error and exits with 2", () => {
+    runPrettier("cli/infer-parser/", ["--stdin", "--stdin-filepath", "foo"], {
+      input: "foo"
+    }).test({
+      status: 2,
+      stdout: "",
+      write: []
+    });
+  });
+
+  describe("--list-different logs error but exits with 0", () => {
+    runPrettier(
+      "cli/infer-parser/",
+      ["--list-different", "--stdin", "--stdin-filepath", "foo"],
+      { input: "foo" }
+    ).test({
+      status: 0,
+      stdout: "",
+      write: []
+    });
+  });
+});
+
+describe("unknown path and no parser", () => {
+  describe("specific file", () => {
+    runPrettier("cli/infer-parser/", ["FOO"]).test({
+      status: 2,
+      stdout: "",
+      write: []
+    });
+  });
+
+  describe("multiple files", () => {
+    runPrettier("cli/infer-parser/", ["*"]).test({
+      status: 2,
+      write: []
+    });
+  });
+});
+
+describe("--list-different with unknown path and no parser", () => {
+  describe("specific file", () => {
+    runPrettier("cli/infer-parser/", ["--list-different", "FOO"]).test({
+      status: 0,
+      stdout: "",
+      write: []
+    });
+  });
+
+  describe("multiple files", () => {
+    runPrettier("cli/infer-parser/", ["--list-different", "*"]).test({
+      status: 1,
+      stdout: "foo.js\n",
+      write: []
+    });
+  });
+});
+
+describe("--write with unknown path and no parser", () => {
+  describe("specific file", () => {
+    runPrettier("cli/infer-parser/", ["--write", "FOO"]).test({
+      status: 2,
+      stdout: "",
+      write: []
+    });
+  });
+
+  describe("multiple files", () => {
+    runPrettier("cli/infer-parser/", ["--write", "*"]).test({
+      status: 2
+    });
+  });
+});
+
+describe("--write and --list-different with unknown path and no parser", () => {
+  describe("specific file", () => {
+    runPrettier("cli/infer-parser/", [
+      "--list-different",
+      "--write",
+      "FOO"
+    ]).test({
+      status: 0,
+      stdout: "",
+      write: []
+    });
+  });
+
+  describe("multiple files", () => {
+    runPrettier("cli/infer-parser/", ["--list-different", "--write", "*"]).test(
+      { status: 1 }
+    );
+  });
+});
+
+describe("API with no path and no parser", () => {
+  expect(() => prettier.format("foo")).toThrowErrorMatchingSnapshot();
+});

tests_integration/__tests__/list-different.js

@@ -3,7 +3,7 @@
 const runPrettier = require("../runPrettier");
 
 describe("checks stdin with --list-different", () => {
-  runPrettier("cli/with-shebang", ["--list-different"], {
+  runPrettier("cli/with-shebang", ["--list-different", "--parser", "babylon"], {
     input: "0"
   }).test({
     stdout: "(stdin)\n",

tests_integration/__tests__/multiple-patterns.js

@@ -64,7 +64,8 @@ describe("multiple patterns by with ignore pattern, doesn't ignore node_modules
 });
 
 describe("no errors on empty patterns", () => {
-  runPrettier("cli/multiple-patterns").test({
+  // --parser is mandatory if no filepath is passed
+  runPrettier("cli/multiple-patterns", ["--parser", "babylon"]).test({
     status: 0
   });
 });

tests_integration/__tests__/syntax-error.js

@@ -3,7 +3,7 @@
 const runPrettier = require("../runPrettier");
 
 describe("exits with non-zero code when input has a syntax error", () => {
-  runPrettier("cli/with-shebang", ["--stdin"], {
+  runPrettier("cli/with-shebang", ["--stdin", "--parser", "babylon"], {
     input: "a.2"
   }).test({
     status: 2

tests_integration/cli/infer-parser/foo.js

@@ -0,0 +1 @@
+foo  (    )