Skip to content

Commit

Permalink
Check pattern existence before glob (#7587)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker authored Feb 19, 2020
1 parent 3cef19c commit 647c041
Show file tree
Hide file tree
Showing 20 changed files with 358 additions and 34 deletions.
3 changes: 3 additions & 0 deletions changelog_unreleased/cli/pr-7587.md
Original file line number Diff line number Diff line change
@@ -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.
10 changes: 7 additions & 3 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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`

Expand Down
118 changes: 87 additions & 31 deletions src/cli/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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, "/"));
/**
* 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 {
return fs.statSync(filePath);
} catch (error) {
/* istanbul ignore next */
if (error.code !== "ENOENT") {
throw error;
}
}
}

// 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/**"]);
// `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");
}
patterns = patterns.concat(["!**/.{git,svn,hg}/**", "!./.{git,svn,hg}/**"]);

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
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 (filePaths.length === 0) {
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(
`No matching files. Patterns tried: ${patterns.join(" ")}`
`Unable to expand glob patterns: ${[
...maybePatterns,
...extraPatterns
].join(" ")}\n${error.message}`
);
// Don't exit the process if one pattern failed
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));
} catch (error) {
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);
}
}

Expand Down
108 changes: 108 additions & 0 deletions tests_integration/__tests__/__snapshots__/patterns-glob.js.snap
Original file line number Diff line number Diff line change
@@ -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 []`;
Loading

0 comments on commit 647c041

Please sign in to comment.