Fix: Improves pref of globbing by inheriting glob.GlobSync (fixes esl…

…int#6710)

lib/ignored-paths.js

@@ -41,7 +41,7 @@ var 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) {
@@ -192,34 +192,30 @@ 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() {
-    var dirs = DEFAULT_IGNORE_DIRS;
+IgnoredPaths.prototype.getIgnoredFoldersGlobChecker = function() {
 
-    if (this.options.ignore) {
-
-        /* eslint-disable no-underscore-dangle */
-
-        var patterns = this.ig.custom._rules.filter(function(rule) {
-            return rule.negative;
-        }).map(function(rule) {
-            return rule.origin;
-        });
+    /**
+     * 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"`
+     */
+    var dirs = DEFAULT_IGNORE_DIRS.map(function(dir) {
+        return dir + "*";
+    });
 
-        /* eslint-enable no-underscore-dangle */
+    var ig = ignore().add(dirs);
 
-        dirs = dirs.filter(function(dir) {
-            return patterns.every(function(p) {
-                return (p.indexOf("!" + dir) !== 0 && p.indexOf("!/" + dir) !== 0);
-            });
-        });
+    if (this.options.ignore) {
+        ig.add(this.ig.custom);
     }
 
+    var filter = ig.createFilter();
 
-    return dirs.map(function(dir) {
-        return dir + "**";
-    });
+    return function(filename) {
+        return !filter(filename);
+    };
 };
 
 module.exports = IgnoredPaths;

lib/util/glob-util.js

@@ -11,7 +11,7 @@
 var debug = require("debug"),
     fs = require("fs"),
     path = require("path"),
-    glob = require("glob"),
+    GlobSync = require("./glob"),
     shell = require("shelljs"),
 
     pathUtil = require("./path-util"),
@@ -108,7 +108,8 @@ function listFilesToProcess(globPatterns, options) {
     var ignoredPaths,
         files = [],
         added = {},
-        globOptions;
+        globOptions,
+        shouldIgnore;
 
     var cwd = (options && options.cwd) || process.cwd();
 
@@ -154,9 +155,9 @@ function listFilesToProcess(globPatterns, options) {
     ignoredPaths = new IgnoredPaths(options);
     globOptions = {
         nodir: true,
-        cwd: cwd,
-        ignore: ignoredPaths.getIgnoredFoldersGlobPatterns()
+        cwd: cwd
     };
+    shouldIgnore = ignoredPaths.getIgnoredFoldersGlobChecker();
 
     debug("Creating list of files to process.");
     globPatterns.forEach(function(pattern) {
@@ -165,7 +166,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,70 @@
+/**
+ * @fileoverview An inherited `glob.GlobSync` to support .gitignore patterns.
+ * @author Kael Zhang
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var Sync = require("glob").GlobSync,
+    path = require("path"),
+    util = require("util");
+
+//------------------------------------------------------------------------------
+// Private
+//------------------------------------------------------------------------------
+
+var 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()`
+     */
+    var marked = this._mark(abs);
+
+    /**
+     * `node-ignore` only accepts relative paths which are relative to the cwd,
+     * as well as .gitignore file does.
+     */
+    var relative = path.relative(this.cwd, marked);
+
+    if (relative && this[IGNORE](relative)) {
+        return null;
+    }
+
+    return Sync.prototype._readdir.call(this, abs, inGlobStar);
+};
+
+
+module.exports = GlobSync;

package.json

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

tests/lib/ignored-paths.js

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