Check pattern existence before glob (#7587)

2020-02-19 12:57:39 +02:00
1 parent 3cef19c73e
commit 647c041b57
@@ -0,0 +1,3 @@
+#### Test whether passed globs are names of existing files before treating them as globs ([#7587](https://github.com/prettier/prettier/pull/7587) by [@fisker](https://github.com/fisker))
+
+Since file names in Linux can contain almost any characters, strings like `foo*.js` and `[bar].css` are valid file names. Previously, if the user needed to format a file named `[bar].css`, a glob escaping syntax had to be used: `prettier "\[bar].css"` (this one doesn't work on Windows, where backslashes are treated as path separators) or `prettier "[[]bar].css"`. Because of this, important use cases were broken. E.g. [lint-staged](https://github.com/okonet/lint-staged) passes absolute paths and knows nothing about the escaping syntax. Now, when Prettier CLI gets a glob, it first tries to resolve it as a literal file name.
@@ -3,7 +3,7 @@ id: cli
 title: CLI
 ---

-Run Prettier through the CLI with this script. Run it without any arguments to see the [options](options.md).
+Use the `prettier` command to run Prettier from the command line. Run it without any arguments to see the [options](options.md).

 To format a file in-place, use `--write`. You may want to consider committing your code before doing that, just in case.

@@ -17,9 +17,13 @@ In practice, this may look something like:
 prettier --single-quote --trailing-comma es5 --write "{app,__{tests,mocks}__}/**/*.js"
 ```

-Don't forget the quotes around the globs! The quotes make sure that Prettier expands the globs rather than your shell, for cross-platform usage. The [glob syntax from the glob module](https://github.com/isaacs/node-glob/blob/master/README.md#glob-primer) is used.
+> Don't forget the **quotes** around the globs! The quotes make sure that Prettier CLI expands the globs rather than your shell, which is important for cross-platform usage.

-Prettier CLI will ignore files located in `node_modules` directory. To opt-out from this behavior use `--with-node-modules` flag.
+Prettier CLI uses the [glob syntax from the `fast-glob` module](https://github.com/mrmlnc/fast-glob#pattern-syntax). To escape special glob characters, one of the two escaping syntaxes can be used: `prettier "\[my-dir]/*.js"` or `prettier "[[]my-dir]/*.js"`. Both match all JS files in a directory named `[my-dir]`, however the latter syntax is preferable as the former doesn't work on Windows, where backslashes are treated as path separators.
+
+Prettier CLI will ignore files located in `node_modules` directory. To opt out from this behavior use `--with-node-modules` flag.
+
+Since file names in Linux can contain almost any characters, before resolving a glob pattern, Prettier CLI first treats it as a literal file name. If a file with such a name is found, Prettier CLI proceeds with that file and doesn't resolve the pattern as a glob.

 ## `--check`

@@ -11,6 +11,7 @@ const stringify = require("json-stable-stringify");
 const fromPairs = require("lodash/fromPairs");
 const pick = require("lodash/pick");
 const groupBy = require("lodash/groupBy");
+const uniq = require("lodash/uniq");

 const minimist = require("./minimist");
 const prettier = require("../../index");
@@ -401,51 +402,106 @@ function createIgnorerFromContextOrDie(context) {
  }
 }

-function eachFilename(context, patterns, callback) {
-  // workaround for fast-glob on Windows ref:
-  // https://github.com/mrmlnc/fast-glob#how-to-write-patterns-on-windows
-  if (path.sep === "\\") {
-    patterns = patterns.map(path => path.replace(/\\/g, "/"));
-  }
-
-  // The '!./' globs are due to https://github.com/prettier/prettier/issues/2110
-  const ignoreNodeModules = context.argv["with-node-modules"] !== true;
-  if (ignoreNodeModules) {
-    patterns = patterns.concat(["!**/node_modules/**", "!./node_modules/**"]);
-  }
-  patterns = patterns.concat(["!**/.{git,svn,hg}/**", "!./.{git,svn,hg}/**"]);
-
+/**
+ * Get stats of a given path.
+ * @param {string} filePath The path to target file.
+ * @returns {fs.Stats|undefined} The stats.
+ * @private
+ */
+function statSafeSync(filePath) {
  try {
-    // `dot pattern` and `expand directories` support need handle differently
-    // for backward compatibility reason temporary remove `.` and set `expandDirectories=false`
-    // see https://github.com/prettier/prettier/pull/6639#issuecomment-548949954
-    const filePaths = globby.sync(
-      patterns.filter(pattern => pattern !== "."),
-      {
-        dot: true,
-        expandDirectories: false
-      }
-    );
-
-    if (filePaths.length === 0) {
-      context.logger.error(
-        `No matching files. Patterns tried: ${patterns.join(" ")}`
-      );
-      process.exitCode = 2;
-      return;
-    }
-
-    filePaths
-      .map(filePath => path.relative(process.cwd(), filePath))
-      // keeping file orders for backward compatibility
-      .sort((a, b) => a.localeCompare(b))
-      .forEach(filePath => callback(filePath));
+    return fs.statSync(filePath);
  } catch (error) {
+    /* istanbul ignore next */
+    if (error.code !== "ENOENT") {
+      throw error;
+    }
+  }
+}
+
+// `dot pattern` and `expand directories` support need handle differently
+// for backward compatibility reason temporary remove `.` and set `expandDirectories=false`
+// see https://github.com/prettier/prettier/pull/6639#issuecomment-548949954
+const isWindows = path.sep === "\\";
+const globbyOptions = { dot: true, expandDirectories: false, absolute: true };
+function eachFilename(context, maybePatterns, callback) {
+  const withNodeModules = context.argv["with-node-modules"] === true;
+  // TODO: use `ignore` option for `globby`
+  const extraPatterns = [
+    // The '!./' globs are due to https://github.com/prettier/prettier/issues/2110
+    ...(withNodeModules ? [] : ["!**/node_modules/**", "!./node_modules/**"]),
+    "!**/.{git,svn,hg}/**",
+    "!./.{git,svn,hg}/**"
+  ];
+
+  // Ignores files in version control systems directories and `node_modules`
+  const ignoredDirectories = [".git", ".svn", ".hg"];
+  if (!withNodeModules) {
+    ignoredDirectories.push("node_modules");
+  }
+
+  let files = [];
+  const cwd = process.cwd();
+  const patterns = [];
+
+  for (const pattern of maybePatterns) {
+    const absolutePath = path.resolve(cwd, pattern);
+    const stat = statSafeSync(absolutePath);
+    if (
+      stat &&
+      stat.isFile() &&
+      !path
+        .relative(cwd, absolutePath)
+        .split(path.sep)
+        .some(directory => ignoredDirectories.includes(directory))
+    ) {
+      files.push(absolutePath);
+    } else {
+      // Ignore `dot pattern` for now
+      if (pattern !== ".") {
+        // workaround for fast-glob on Windows ref:
+        // https://github.com/mrmlnc/fast-glob#how-to-write-patterns-on-windows
+        patterns.push(isWindows ? pattern.replace(/\\/g, "/") : pattern);
+      }
+    }
+  }
+
+  if (patterns.length > 0) {
+    try {
+      // Don't use `files.push(...)`, there is limitation on the number of arguments.
+      files = [
+        ...files,
+        ...globby.sync([...patterns, ...extraPatterns], globbyOptions)
+      ];
+    } catch (error) {
+      context.logger.error(
+        `Unable to expand glob patterns: ${[
+          ...maybePatterns,
+          ...extraPatterns
+        ].join(" ")}\n${error.message}`
+      );
+      // Don't exit the process if one pattern failed
+      process.exitCode = 2;
+    }
+  }
+
+  if (files.length === 0) {
    context.logger.error(
-      `Unable to expand glob patterns: ${patterns.join(" ")}\n${error.message}`
+      `No matching files. Patterns tried: ${[
+        ...maybePatterns,
+        ...extraPatterns
+      ].join(" ")}`
    );
-    // Don't exit the process if one pattern failed
    process.exitCode = 2;
+    return;
+  }
+
+  files = uniq(files.map(file => path.relative(cwd, file)))
+    // keeping file order for backward compatibility
+    .sort((a, b) => a.localeCompare(b));
+
+  for (const file of files) {
+    callback(file);
  }
 }

@@ -0,0 +1,108 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`fixtures-1: Should match all files (stderr) 1`] = `""`;
+
+exports[`fixtures-1: Should match all files (stdout) 1`] = `
+"!file.js
+a.js
+b.js
+"
+`;
+
+exports[`fixtures-1: Should match all files (write) 1`] = `Array []`;
+
+exports[`fixtures-1: Should match files except \`a.js\` (stderr) 1`] = `""`;
+
+exports[`fixtures-1: Should match files except \`a.js\` (stdout) 1`] = `
+"!file.js
+b.js
+"
+`;
+
+exports[`fixtures-1: Should match files except \`a.js\` (write) 1`] = `Array []`;
+
+exports[`fixtures-2: Should match \`a.js\` and \`!b.js\` (stderr) 1`] = `""`;
+
+exports[`fixtures-2: Should match \`a.js\` and \`!b.js\` (stdout) 1`] = `
+"!b.js
+a.js
+"
+`;
+
+exports[`fixtures-2: Should match \`a.js\` and \`!b.js\` (write) 1`] = `Array []`;
+
+exports[`fixtures-2: Should match all js files (stderr) 1`] = `""`;
+
+exports[`fixtures-2: Should match all js files (stdout) 1`] = `
+"!b.js
+a.js
+"
+`;
+
+exports[`fixtures-2: Should match all js files (write) 1`] = `Array []`;
+
+exports[`fixtures-2: Should only match \`!b.js\` (stderr) 1`] = `""`;
+
+exports[`fixtures-2: Should only match \`!b.js\` (stdout) 1`] = `
+"!b.js
+"
+`;
+
+exports[`fixtures-2: Should only match \`!b.js\` (write) 1`] = `Array []`;
+
+exports[`fixtures-3: Should exclude \`.svn\` (stderr) 1`] = `""`;
+
+exports[`fixtures-3: Should exclude \`.svn\` (stdout) 1`] = `
+"outside.js
+"
+`;
+
+exports[`fixtures-3: Should exclude \`.svn\` (write) 1`] = `Array []`;
+
+exports[`fixtures-3: Should match \`outside.js\`, \`dir/inside.js\` and \`dir/node_modules/in-node_modules.js\` (stderr) 1`] = `""`;
+
+exports[`fixtures-3: Should match \`outside.js\`, \`dir/inside.js\` and \`dir/node_modules/in-node_modules.js\` (stdout) 1`] = `
+"dir/inside.js
+dir/node_modules/in-node_modules.js
+outside.js
+"
+`;
+
+exports[`fixtures-3: Should match \`outside.js\`, \`dir/inside.js\` and \`dir/node_modules/in-node_modules.js\` (write) 1`] = `Array []`;
+
+exports[`fixtures-3: Should only match \`outside.js\` and \`dir/inside.js\` (stderr) 1`] = `""`;
+
+exports[`fixtures-3: Should only match \`outside.js\` and \`dir/inside.js\` (stdout) 1`] = `
+"dir/inside.js
+outside.js
+"
+`;
+
+exports[`fixtures-3: Should only match \`outside.js\` and \`dir/inside.js\` (write) 1`] = `Array []`;
+
+exports[`fixtures-4: Should match \`level-1.js\` #2 (stderr) 1`] = `""`;
+
+exports[`fixtures-4: Should match \`level-1.js\` #2 (stdout) 1`] = `
+"0/level-1.js
+"
+`;
+
+exports[`fixtures-4: Should match \`level-1.js\` #2 (write) 1`] = `Array []`;
+
+exports[`fixtures-4: Should match \`level-1.js\` #3 (stderr) 1`] = `""`;
+
+exports[`fixtures-4: Should match \`level-1.js\` #3 (stdout) 1`] = `
+"0/level-1.js
+"
+`;
+
+exports[`fixtures-4: Should match \`level-1.js\` #3 (write) 1`] = `Array []`;
+
+exports[`fixtures-4: Should match \`level-1.js\` (stderr) 1`] = `""`;
+
+exports[`fixtures-4: Should match \`level-1.js\` (stdout) 1`] = `
+"0/level-1.js
+"
+`;
+
+exports[`fixtures-4: Should match \`level-1.js\` (write) 1`] = `Array []`;
@@ -0,0 +1,123 @@
+"use strict";
+
+const runPrettier = require("../runPrettier");
+
+expect.addSnapshotSerializer(require("../path-serializer"));
+
+/*
+fixtures-1/
+├─ !file.js
+├─ a.js
+└─ b.js
+*/
+
+describe("fixtures-1: Should match all files", () => {
+  runPrettier("cli/patterns-glob/fixtures-1", ["*.js", "!file.js", "-l"]).test({
+    status: 1
+  });
+});
+
+describe("fixtures-1: Should match files except `a.js`", () => {
+  runPrettier("cli/patterns-glob/fixtures-1", ["*.js", "!a.js", "-l"]).test({
+    status: 1
+  });
+});
+
+/*
+fixtures-2/
+├─ a.js
+├─ !b.js
+└─ !dir.js/
+  ├─ 1.css
+  └─ 2.css
+*/
+
+describe("fixtures-2: Should match all js files", () => {
+  runPrettier("cli/patterns-glob/fixtures-2", ["*.js", "!dir.js", "-l"]).test({
+    status: 1
+  });
+});
+
+describe("fixtures-2: Should match `a.js` and `!b.js`", () => {
+  runPrettier("cli/patterns-glob/fixtures-2", ["*.js", "!b.js", "-l"]).test({
+    status: 1
+  });
+});
+
+describe("fixtures-2: Should only match `!b.js`", () => {
+  runPrettier("cli/patterns-glob/fixtures-2", ["*.js", "!a.js", "-l"]).test({
+    status: 1
+  });
+});
+
+/*
+fixtures-3/
+├─ outside.js
+└─ dir
+  ├─ inside.js
+  ├─ node_modules/
+  │ └─in-node_modules.js
+  └─ .svn/
+    └─in-svn.js
+*/
+
+describe("fixtures-3: Should match `outside.js`, `dir/inside.js` and `dir/node_modules/in-node_modules.js`", () => {
+  runPrettier("cli/patterns-glob/fixtures-3", [
+    "**/*.js",
+    "-l",
+    "--with-node-modules"
+  ]).test({
+    status: 1
+  });
+});
+
+describe("fixtures-3: Should only match `outside.js` and `dir/inside.js`", () => {
+  runPrettier("cli/patterns-glob/fixtures-3", ["**/*.js", "-l"]).test({
+    status: 1
+  });
+});
+
+describe("fixtures-3: Should exclude `.svn`", () => {
+  runPrettier("cli/patterns-glob/fixtures-3", [
+    "*.js",
+    ".svn/in-svn.js",
+    "-l"
+  ]).test({
+    status: 1
+  });
+});
+
+/*
+fixtures-4/
+├─ level-0.js
+└─ 0
+  ├─ level-1.js
+  └─ 1/
+    ├─ level-2.js
+    └─ 2/
+      └─ level-3.js
+*/
+
+describe("fixtures-4: Should match `level-1.js`", () => {
+  runPrettier("cli/patterns-glob/fixtures-4", ["./0/./level-1.js", "-l"]).test({
+    status: 1
+  });
+});
+
+describe("fixtures-4: Should match `level-1.js` #2", () => {
+  runPrettier("cli/patterns-glob/fixtures-4", [
+    "./0/1/2/../../level-1.js",
+    "-l"
+  ]).test({
+    status: 1
+  });
+});
+
+describe("fixtures-4: Should match `level-1.js` #3", () => {
+  runPrettier("cli/patterns-glob/fixtures-4", [
+    "./0/non-exists-dir/2/../../level-1.js",
+    "-l"
+  ]).test({
+    status: 1
+  });
+});
@@ -0,0 +1,2 @@
+foo.
+bar()
@@ -0,0 +1,2 @@
+foo.
+bar()
@@ -0,0 +1,2 @@
+foo.
+bar()
@@ -0,0 +1,2 @@
+foo.
+bar()
@@ -0,0 +1,2 @@
+.foo
+{}
@@ -0,0 +1,2 @@
+.foo
+{}
@@ -0,0 +1,2 @@
+foo.
+bar()
@@ -0,0 +1,2 @@
+foo
+.bar()
@@ -0,0 +1,2 @@
+foo
+.bar()
@@ -0,0 +1,2 @@
+foo
+.bar()
@@ -0,0 +1,2 @@
+foo
+.bar()
@@ -0,0 +1,2 @@
+foo
+.bar()
@@ -0,0 +1,2 @@
+foo
+.bar()
@@ -0,0 +1,2 @@
+foo
+.bar()
@@ -0,0 +1,2 @@
+foo
+.bar()