diff --git a/changelog_unreleased/cli/pr-7587.md b/changelog_unreleased/cli/pr-7587.md new file mode 100644 index 000000000..001af15c5 --- /dev/null +++ b/changelog_unreleased/cli/pr-7587.md @@ -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. diff --git a/docs/cli.md b/docs/cli.md index 86f5feced..c64f5f80d 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -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` diff --git a/src/cli/util.js b/src/cli/util.js index afcb58b01..84708325b 100644 --- a/src/cli/util.js +++ b/src/cli/util.js @@ -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); } } diff --git a/tests_integration/__tests__/__snapshots__/patterns-glob.js.snap b/tests_integration/__tests__/__snapshots__/patterns-glob.js.snap new file mode 100644 index 000000000..5cd8b1338 --- /dev/null +++ b/tests_integration/__tests__/__snapshots__/patterns-glob.js.snap @@ -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 []`; diff --git a/tests_integration/__tests__/patterns-glob.js b/tests_integration/__tests__/patterns-glob.js new file mode 100644 index 000000000..8edd4c488 --- /dev/null +++ b/tests_integration/__tests__/patterns-glob.js @@ -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 + }); +}); diff --git a/tests_integration/cli/patterns-glob/fixtures-1/!file.js b/tests_integration/cli/patterns-glob/fixtures-1/!file.js new file mode 100644 index 000000000..0b118ad36 --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-1/!file.js @@ -0,0 +1,2 @@ +foo. +bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-1/a.js b/tests_integration/cli/patterns-glob/fixtures-1/a.js new file mode 100644 index 000000000..0b118ad36 --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-1/a.js @@ -0,0 +1,2 @@ +foo. +bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-1/b.js b/tests_integration/cli/patterns-glob/fixtures-1/b.js new file mode 100644 index 000000000..0b118ad36 --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-1/b.js @@ -0,0 +1,2 @@ +foo. +bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-2/!b.js b/tests_integration/cli/patterns-glob/fixtures-2/!b.js new file mode 100644 index 000000000..0b118ad36 --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-2/!b.js @@ -0,0 +1,2 @@ +foo. +bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-2/!dir.js/1.css b/tests_integration/cli/patterns-glob/fixtures-2/!dir.js/1.css new file mode 100644 index 000000000..687c7f1db --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-2/!dir.js/1.css @@ -0,0 +1,2 @@ +.foo +{} diff --git a/tests_integration/cli/patterns-glob/fixtures-2/!dir.js/2.css b/tests_integration/cli/patterns-glob/fixtures-2/!dir.js/2.css new file mode 100644 index 000000000..687c7f1db --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-2/!dir.js/2.css @@ -0,0 +1,2 @@ +.foo +{} diff --git a/tests_integration/cli/patterns-glob/fixtures-2/a.js b/tests_integration/cli/patterns-glob/fixtures-2/a.js new file mode 100644 index 000000000..0b118ad36 --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-2/a.js @@ -0,0 +1,2 @@ +foo. +bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-3/dir/.svn/in-svn.js b/tests_integration/cli/patterns-glob/fixtures-3/dir/.svn/in-svn.js new file mode 100644 index 000000000..11d77723b --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-3/dir/.svn/in-svn.js @@ -0,0 +1,2 @@ +foo +.bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-3/dir/inside.js b/tests_integration/cli/patterns-glob/fixtures-3/dir/inside.js new file mode 100644 index 000000000..11d77723b --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-3/dir/inside.js @@ -0,0 +1,2 @@ +foo +.bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-3/dir/node_modules/in-node_modules.js b/tests_integration/cli/patterns-glob/fixtures-3/dir/node_modules/in-node_modules.js new file mode 100644 index 000000000..11d77723b --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-3/dir/node_modules/in-node_modules.js @@ -0,0 +1,2 @@ +foo +.bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-3/outside.js b/tests_integration/cli/patterns-glob/fixtures-3/outside.js new file mode 100644 index 000000000..11d77723b --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-3/outside.js @@ -0,0 +1,2 @@ +foo +.bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-4/0/1/2/level-3.js b/tests_integration/cli/patterns-glob/fixtures-4/0/1/2/level-3.js new file mode 100644 index 000000000..11d77723b --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-4/0/1/2/level-3.js @@ -0,0 +1,2 @@ +foo +.bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-4/0/1/level-2.js b/tests_integration/cli/patterns-glob/fixtures-4/0/1/level-2.js new file mode 100644 index 000000000..11d77723b --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-4/0/1/level-2.js @@ -0,0 +1,2 @@ +foo +.bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-4/0/level-1.js b/tests_integration/cli/patterns-glob/fixtures-4/0/level-1.js new file mode 100644 index 000000000..11d77723b --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-4/0/level-1.js @@ -0,0 +1,2 @@ +foo +.bar() diff --git a/tests_integration/cli/patterns-glob/fixtures-4/level-0.js b/tests_integration/cli/patterns-glob/fixtures-4/level-0.js new file mode 100644 index 000000000..11d77723b --- /dev/null +++ b/tests_integration/cli/patterns-glob/fixtures-4/level-0.js @@ -0,0 +1,2 @@ +foo +.bar()