feat(cli): support multiple patterns with ignore pattern (#2356)

bin/prettier.js

@@ -3,9 +3,8 @@
 "use strict";
 
 const fs = require("fs");
-const path = require("path");
 const getStream = require("get-stream");
-const glob = require("glob");
+const globby = require("globby");
 const chalk = require("chalk");
 const minimist = require("minimist");
 const readline = require("readline");
@@ -69,8 +68,8 @@ const filepatterns = argv["_"];
 const write = argv["write"];
 const stdin = argv["stdin"] || (!filepatterns.length && !process.stdin.isTTY);
 const ignoreNodeModules = argv["with-node-modules"] === false;
+const ignoreNodeModulesGlobs = ["!**/node_modules/**", "!./node_modules/**"];
 const globOptions = {
-  ignore: ignoreNodeModules && ["**/node_modules/**", "./node_modules/**"],
   dot: true
 };
 
@@ -361,30 +360,30 @@ function writeOutput(result) {
 }
 
 function eachFilename(patterns, callback) {
-  patterns.forEach(pattern => {
-    if (!glob.hasMagic(pattern)) {
-      if (shouldIgnorePattern(pattern)) {
-        return;
-      }
-      callback(pattern);
-      return;
-    }
+  if (ignoreNodeModules) {
+    patterns = patterns.concat(ignoreNodeModulesGlobs);
+  }
 
-    glob(pattern, globOptions, (err, filenames) => {
-      if (err) {
-        console.error("Unable to expand glob pattern: " + pattern + "\n" + err);
-        // Don't exit the process if one pattern failed
+  return globby(patterns, globOptions)
+    .then(filePaths => {
+      if (filePaths.length === 0) {
+        console.error(
+          "No matching files. Patterns tried: " + patterns.join(" ")
+        );
         process.exitCode = 2;
         return;
       }
 
-      filenames.forEach(filename => {
-        callback(filename);
+      filePaths.forEach(filePath => {
+        return callback(filePath);
       });
+    })
+    .catch(err => {
+      console.error(
+        "Unable to expand glob patterns: " + patterns.join(" ") + "\n" + err
+      );
+      // Don't exit the process if one pattern failed
+      process.exitCode = 2;
+      return;
     });
-  });
-}
-
-function shouldIgnorePattern(pattern) {
-  return ignoreNodeModules && path.resolve(pattern).includes("/node_modules/");
 }

package.json

@@ -18,7 +18,7 @@
     "esutils": "2.0.2",
     "flow-parser": "0.47.0",
     "get-stream": "3.0.0",
-    "glob": "7.1.2",
+    "globby": "^6.1.0",
     "graphql": "0.10.1",
     "jest-validate": "20.0.3",
     "json-to-ast": "2.0.0-alpha1.2",

scripts/sync-flow-tests.js

@@ -2,7 +2,7 @@
 
 const fs = require("fs");
 const flowParser = require("flow-parser");
-const glob = require("glob");
+const globby = require("globby");
 const mkdirp = require("mkdirp");
 const path = require("path");
 const rimraf = require("rimraf");
@@ -29,8 +29,10 @@ function tryParse(file, content) {
 }
 
 function syncTests(syncDir) {
-  const specFiles = glob.sync(path.join(FLOW_TESTS_DIR, "**", SPEC_FILE_NAME));
-  const filesToCopy = glob.sync(path.join(syncDir, "**/*.js"));
+  const specFiles = globby.sync(
+    path.join(FLOW_TESTS_DIR, "**", SPEC_FILE_NAME)
+  );
+  const filesToCopy = globby.sync(path.join(syncDir, "**/*.js"));
 
   if (filesToCopy.length === 0) {
     throw new Error(

tests_integration/__tests__/__snapshots__/multiple-patterns.js.snap

@@ -0,0 +1,53 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`multiple patterns 1`] = `
+"directory/file.js
+directory/nested-directory/nested-directory-file.js
+other-directory/file.js
+other-directory/nested-directory/nested-directory-file.js
+"
+`;
+
+exports[`multiple patterns by with ignore pattern, doesn't ignore node_modules with --with-node-modules flag 1`] = `
+"node_modules/node-module.js
+other-directory/file.js
+other-directory/nested-directory/nested-directory-file.js
+other-regular-modules.js
+regular-module.js
+"
+`;
+
+exports[`multiple patterns by with ignore pattern, ignores node_modules by default 1`] = `
+"other-directory/file.js
+other-directory/nested-directory/nested-directory-file.js
+other-regular-modules.js
+regular-module.js
+"
+`;
+
+exports[`multiple patterns by with ignore pattern, ignores node_modules by with ./**/*.js 1`] = `
+"./other-directory/file.js
+./other-directory/nested-directory/nested-directory-file.js
+./other-regular-modules.js
+./regular-module.js
+"
+`;
+
+exports[`multiple patterns with ignore nested directories pattern 1`] = `
+"directory/file.js
+other-directory/file.js
+other-regular-modules.js
+regular-module.js
+"
+`;
+
+exports[`multiple patterns with non exists pattern 1`] = `
+"directory/file.js
+directory/nested-directory/nested-directory-file.js
+"
+`;
+
+exports[`multiple patterns, throw error and exit with non zero code on non existing files 1`] = `
+"No matching files. Patterns tried: non-existent.js other-non-existent.js !**/node_modules/** !./node_modules/**
+"
+`;

tests_integration/__tests__/multiple-patterns.js

@@ -0,0 +1,87 @@
+"use strict";
+
+const runPrettier = require("../runPrettier");
+
+test("multiple patterns", () => {
+  const result = runPrettier("cli/multiple-patterns", [
+    "directory/**/*.js",
+    "other-directory/**/*.js",
+    "-l"
+  ]);
+
+  expect(result.stdout).toMatchSnapshot();
+  expect(result.status).toEqual(1);
+});
+
+test("multiple patterns with non exists pattern", () => {
+  const result = runPrettier("cli/multiple-patterns", [
+    "directory/**/*.js",
+    "non-existent.js",
+    "-l"
+  ]);
+
+  expect(result.stdout).toMatchSnapshot();
+  expect(result.status).toEqual(1);
+});
+
+test("multiple patterns with ignore nested directories pattern", () => {
+  const result = runPrettier("cli/multiple-patterns", [
+    "**/*.js",
+    "!**/nested-directory/**",
+    "-l"
+  ]);
+
+  expect(result.stdout).toMatchSnapshot();
+  expect(result.status).toEqual(1);
+});
+
+test("multiple patterns by with ignore pattern, ignores node_modules by default", () => {
+  const result = runPrettier("cli/multiple-patterns", [
+    "**/*.js",
+    "!directory/**",
+    "-l"
+  ]);
+
+  expect(result.stdout).toMatchSnapshot();
+  expect(result.status).toEqual(1);
+});
+
+test("multiple patterns by with ignore pattern, ignores node_modules by with ./**/*.js", () => {
+  const result = runPrettier("cli/multiple-patterns", [
+    "./**/*.js",
+    "!./directory/**",
+    "-l"
+  ]);
+
+  expect(result.stdout).toMatchSnapshot();
+  expect(result.status).toEqual(1);
+});
+
+test("multiple patterns by with ignore pattern, doesn't ignore node_modules with --with-node-modules flag", () => {
+  const result = runPrettier("cli/multiple-patterns", [
+    "**/*.js",
+    "!directory/**",
+    "-l",
+    "--with-node-modules"
+  ]);
+
+  expect(result.stdout).toMatchSnapshot();
+  expect(result.status).toEqual(1);
+});
+
+test("no errors on empty patterns", () => {
+  const result = runPrettier("cli/multiple-patterns");
+
+  expect(result.status).toEqual(0);
+});
+
+test("multiple patterns, throw error and exit with non zero code on non existing files", () => {
+  const result = runPrettier("cli/multiple-patterns", [
+    "non-existent.js",
+    "other-non-existent.js",
+    "-l"
+  ]);
+
+  expect(result.stderr).toMatchSnapshot();
+  expect(result.status).toEqual(2);
+});

tests_integration/cli/multiple-patterns/directory/file.js

@@ -0,0 +1,2 @@
+/* eslint-disable */
+'use strict';

tests_integration/cli/multiple-patterns/directory/nested-directory/nested-directory-file.js

@@ -0,0 +1,2 @@
+/* eslint-disable */
+'use strict';

tests_integration/cli/multiple-patterns/node_modules/node-module.js

@@ -0,0 +1,2 @@
+/* eslint-disable */
+'use strict';

tests_integration/cli/multiple-patterns/other-directory/file.js

@@ -0,0 +1,2 @@
+/* eslint-disable */
+'use strict';

tests_integration/cli/multiple-patterns/other-directory/nested-directory/nested-directory-file.js

@@ -0,0 +1,2 @@
+/* eslint-disable */
+'use strict';

tests_integration/cli/multiple-patterns/other-regular-modules.js

@@ -0,0 +1,2 @@
+/* eslint-disable */
+'use strict';

tests_integration/cli/multiple-patterns/regular-module.js

@@ -0,0 +1,2 @@
+/* eslint-disable */
+'use strict';