Fix: Improve pref of globbing by inheriting glob.GlobSync (fixes #6710)…

… (#6783)

lib/ignored-paths.js

@@ -23,9 +23,15 @@ const debug = require("debug")("eslint:ignored-paths");
 //------------------------------------------------------------------------------
 
 const ESLINT_IGNORE_FILENAME = ".eslintignore";
+
+/**
+ * Adds `"*"` at the end of `"node_modules/"`,
+ * so that subtle directories could be re-included by .gitignore patterns
+ * such as `"!node_modules/should_not_ignored"`
+ */
 const DEFAULT_IGNORE_DIRS = [
-    "node_modules/",
-    "bower_components/"
+    "/node_modules/*",
+    "/bower_components/*"
 ];
 const DEFAULT_OPTIONS = {
     dotfiles: false,
@@ -40,7 +46,7 @@ const DEFAULT_OPTIONS = {
 
 /**
  * Find an ignore file in the current directory.
- * @param {stirng} cwd Current working directory
+ * @param {string} cwd Current working directory
  * @returns {string} Path of ignore file or an empty string.
  */
 function findIgnoreFile(cwd) {
@@ -96,9 +102,7 @@ function IgnoredPaths(options) {
         return ig.add(fs.readFileSync(filepath, "utf8"));
     }
 
-    this.defaultPatterns = DEFAULT_IGNORE_DIRS.map(function(dir) {
-        return "/" + dir + "*";
-    }).concat(options.patterns || []);
+    this.defaultPatterns = [].concat(DEFAULT_IGNORE_DIRS, options.patterns || []);
     this.baseDir = options.cwd;
 
     this.ig = {
@@ -191,34 +195,37 @@ IgnoredPaths.prototype.contains = function(filepath, category) {
 
 /**
  * Returns a list of dir patterns for glob to ignore
- * @returns {string[]} list of glob ignore patterns
+ * @returns {function()} method to check whether a folder should be ignored by glob.
  */
-IgnoredPaths.prototype.getIgnoredFoldersGlobPatterns = function() {
-    let dirs = DEFAULT_IGNORE_DIRS;
+IgnoredPaths.prototype.getIgnoredFoldersGlobChecker = function() {
 
-    if (this.options.ignore) {
+    const ig = ignore().add(DEFAULT_IGNORE_DIRS);
 
-        /* eslint-disable no-underscore-dangle */
+    if (this.options.ignore) {
+        ig.add(this.ig.custom);
+    }
 
-        const patterns = this.ig.custom._rules.filter(function(rule) {
-            return rule.negative;
-        }).map(function(rule) {
-            return rule.origin;
-        });
+    const filter = ig.createFilter();
 
-        /* eslint-enable no-underscore-dangle */
+    /**
+     * TODO
+     * 1.
+     * Actually, it should be `this.options.baseDir`, which is the base dir of `ignore-path`,
+     * as well as Line 177.
+     * But doing this leads to a breaking change and fails tests.
+     * Related to #6759
+     */
+    const base = this.options.cwd;
 
-        dirs = dirs.filter(function(dir) {
-            return patterns.every(function(p) {
-                return (p.indexOf("!" + dir) !== 0 && p.indexOf("!/" + dir) !== 0);
-            });
-        });
-    }
+    return function(absolutePath) {
+        const relative = pathUtil.getRelativePath(absolutePath, base);
 
+        if (!relative) {
+            return false;
+        }
 
-    return dirs.map(function(dir) {
-        return dir + "**";
-    });
+        return !filter(relative);
+    };
 };
 
 module.exports = IgnoredPaths;

lib/util/glob-util.js

@@ -10,7 +10,7 @@
 
 const fs = require("fs"),
     path = require("path"),
-    glob = require("glob"),
+    GlobSync = require("./glob"),
     shell = require("shelljs"),
 
     pathUtil = require("./path-util"),
@@ -113,9 +113,9 @@ function listFilesToProcess(globPatterns, options) {
     const ignoredPaths = new IgnoredPaths(options);
     const globOptions = {
         nodir: true,
-        cwd,
-        ignore: ignoredPaths.getIgnoredFoldersGlobPatterns()
+        cwd
     };
+    const shouldIgnore = ignoredPaths.getIgnoredFoldersGlobChecker();
 
     /**
      * Executes the linter on a file defined by the `filename`. Skips
@@ -162,7 +162,7 @@ function listFilesToProcess(globPatterns, options) {
         if (shell.test("-f", file)) {
             addFile(fs.realpathSync(file), !shell.test("-d", file));
         } else {
-            glob.sync(pattern, globOptions).forEach(function(globMatch) {
+            new GlobSync(pattern, globOptions, shouldIgnore).found.forEach(function(globMatch) {
                 addFile(path.resolve(cwd, globMatch), false);
             });
         }

lib/util/glob.js

@@ -0,0 +1,63 @@
+/**
+ * @fileoverview An inherited `glob.GlobSync` to support .gitignore patterns.
+ * @author Kael Zhang
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const Sync = require("glob").GlobSync,
+    util = require("util");
+
+//------------------------------------------------------------------------------
+// Private
+//------------------------------------------------------------------------------
+
+const IGNORE = typeof Symbol === "function" ? Symbol("ignore") : "_shouldIgnore";
+
+/**
+ * Subclass of `glob.GlobSync`
+ * @param {string}     pattern      Pattern to be matched.
+ * @param {Object}     options      `options` for `glob`
+ * @param {function()} shouldIgnore Method to check whether a directory should be ignored.
+ * @constructor
+ */
+function GlobSync(pattern, options, shouldIgnore) {
+
+    /**
+     * We don't put this thing to argument `options` to avoid
+     * further problems, such as `options` validation.
+     *
+     * Use `Symbol` as much as possible to avoid confliction.
+     */
+    this[IGNORE] = shouldIgnore;
+
+    Sync.call(this, pattern, options);
+}
+
+util.inherits(GlobSync, Sync);
+
+/* eslint no-underscore-dangle: ["error", { "allow": ["_readdir", "_mark"] }] */
+
+GlobSync.prototype._readdir = function(abs, inGlobStar) {
+
+    /**
+     * `options.nodir` makes `options.mark` as `true`.
+     * Mark `abs` first
+     * to make sure `"node_modules"` will be ignored immediately with ignore pattern `"node_modules/"`.
+
+     * There is a built-in cache about marked `File.Stat` in `glob`, so that we could not worry about the extra invocation of `this._mark()`
+     */
+    const marked = this._mark(abs);
+
+    if (this[IGNORE](marked)) {
+        return null;
+    }
+
+    return Sync.prototype._readdir.call(this, abs, inGlobStar);
+};
+
+
+module.exports = GlobSync;

package.json

@@ -45,7 +45,7 @@
     "file-entry-cache": "^1.3.1",
     "glob": "^7.0.3",
     "globals": "^9.2.0",
-    "ignore": "^3.1.2",
+    "ignore": "^3.1.5",
     "imurmurhash": "^0.1.4",
     "inquirer": "^0.12.0",
     "is-my-json-valid": "^2.10.0",

tests/lib/ignored-paths.js

@@ -529,50 +529,93 @@ describe("IgnoredPaths", function() {
 
     });
 
-    describe("getIgnoredFoldersGlobPatterns", function() {
-        it("should return default ignores glob dir patterns when there is no eslintignore file", function() {
-            const ignoredPaths = new IgnoredPaths({ ignore: true, cwd: getFixturePath("no-ignore-file") });
-
-            const expected = ["node_modules/**", "bower_components/**"];
-            const actual = ignoredPaths.getIgnoredFoldersGlobPatterns();
-
-            assert.sameMembers(actual, expected);
-        });
-
-        it("should return default glob dir patterns when there is an ignore file without unignored defaults", function() {
-            const ignoredPaths = new IgnoredPaths({ ignore: true, ignorePath: getFixturePath(".eslintignore"), cwd: getFixturePath() });
-
-            const expected = ["node_modules/**", "bower_components/**"];
-            const actual = ignoredPaths.getIgnoredFoldersGlobPatterns();
-
-            assert.sameMembers(actual, expected);
-        });
-
-        it("should not return dirs with unignored defaults in ignore file", function() {
-            const ignoredPaths = new IgnoredPaths({ ignore: true, ignorePath: getFixturePath(".eslintignoreWithUnignoredDefaults"), cwd: getFixturePath() });
-
-            const actual = ignoredPaths.getIgnoredFoldersGlobPatterns();
-
-            assert.notInclude(actual, "node_modules/**");
-            assert.notInclude(actual, "bower_components/**");
-        });
-
-        it("should return dirs with unignored defaults in ignore file when ignore option is disabled", function() {
-            const ignoredPaths = new IgnoredPaths({ ignore: false, ignorePath: getFixturePath(".eslintignoreWithUnignoredDefaults"), cwd: getFixturePath() });
-
-            const expected = ["node_modules/**", "bower_components/**"];
-            const actual = ignoredPaths.getIgnoredFoldersGlobPatterns();
-
-            assert.includeMembers(actual, expected);
-        });
-
-        it("should not return dirs unignored by ignorePattern", function() {
-            const ignoredPaths = new IgnoredPaths({ ignore: true, cwd: getFixturePath("no-ignore-file"), ignorePattern: "!/node_modules/package" });
-
-            const actual = ignoredPaths.getIgnoredFoldersGlobPatterns();
-
-            assert.notInclude(actual, "node_modules/**");
-            assert.include(actual, "bower_components/**");
+    describe("getIgnoredFoldersGlobChecker", function() {
+
+        /**
+         * Creates a function to resolve the given relative path according to the `cwd`
+         * @param {path} cwd The cwd of `ignorePaths`
+         * @returns {function()} the function described above.
+         */
+        function createResolve(cwd) {
+            return function(relative) {
+                return path.join(cwd, relative);
+            };
+        }
+
+        it("should ignore default folders when there is no eslintignore file", function() {
+            const cwd = getFixturePath("no-ignore-file");
+            const ignoredPaths = new IgnoredPaths({ ignore: true, cwd: cwd });
+
+            const shouldIgnore = ignoredPaths.getIgnoredFoldersGlobChecker();
+            const resolve = createResolve(cwd);
+
+            assert.isTrue(shouldIgnore(resolve("node_modules/a")));
+            assert.isTrue(shouldIgnore(resolve("node_modules/a/b")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a/b")));
+            assert.isFalse(shouldIgnore(resolve(".hidden")));
+        });
+
+        it("should ignore default folders there is an ignore file without unignored defaults", function() {
+            const cwd = getFixturePath();
+            const ignoredPaths = new IgnoredPaths({ ignore: true, ignorePath: getFixturePath(".eslintignore"), cwd: cwd });
+
+            const shouldIgnore = ignoredPaths.getIgnoredFoldersGlobChecker();
+            const resolve = createResolve(cwd);
+
+            assert.isTrue(shouldIgnore(resolve("node_modules/a")));
+            assert.isTrue(shouldIgnore(resolve("node_modules/a/b")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a/b")));
+            assert.isFalse(shouldIgnore(resolve(".hidden")));
+        });
+
+        it("should not ignore things which are re-included in ignore file", function() {
+            const cwd = getFixturePath();
+            const ignoredPaths = new IgnoredPaths({ ignore: true, ignorePath: getFixturePath(".eslintignoreWithUnignoredDefaults"), cwd: cwd });
+
+            const shouldIgnore = ignoredPaths.getIgnoredFoldersGlobChecker();
+            const resolve = createResolve(cwd);
+
+            assert.isTrue(shouldIgnore(resolve("node_modules/a")));
+            assert.isTrue(shouldIgnore(resolve("node_modules/a/b")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a/b")));
+            assert.isFalse(shouldIgnore(resolve(".hidden")));
+            assert.isFalse(shouldIgnore(resolve("node_modules/package")));
+            assert.isFalse(shouldIgnore(resolve("bower_components/package")));
+        });
+
+        it("should ignore files which we try to re-include in ignore file when ignore option is disabled", function() {
+            const cwd = getFixturePath();
+            const ignoredPaths = new IgnoredPaths({ ignore: false, ignorePath: getFixturePath(".eslintignoreWithUnignoredDefaults"), cwd: cwd });
+
+            const shouldIgnore = ignoredPaths.getIgnoredFoldersGlobChecker();
+            const resolve = createResolve(cwd);
+
+            assert.isTrue(shouldIgnore(resolve("node_modules/a")));
+            assert.isTrue(shouldIgnore(resolve("node_modules/a/b")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a/b")));
+            assert.isFalse(shouldIgnore(resolve(".hidden")));
+            assert.isTrue(shouldIgnore(resolve("node_modules/package")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/package")));
+        });
+
+        it("should not ignore dirs which are re-included by ignorePattern", function() {
+            const cwd = getFixturePath("no-ignore-file");
+            const ignoredPaths = new IgnoredPaths({ ignore: true, cwd: cwd, ignorePattern: "!/node_modules/package" });
+
+            const shouldIgnore = ignoredPaths.getIgnoredFoldersGlobChecker();
+            const resolve = createResolve(cwd);
+
+            assert.isTrue(shouldIgnore(resolve("node_modules/a")));
+            assert.isTrue(shouldIgnore(resolve("node_modules/a/b")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/a/b")));
+            assert.isFalse(shouldIgnore(resolve(".hidden")));
+            assert.isFalse(shouldIgnore(resolve("node_modules/package")));
+            assert.isTrue(shouldIgnore(resolve("bower_components/package")));
         });
     });