fix: Ensure that glob patterns are matched correctly. (#16449)

nzakas · web-flow · commit 33668ee9d22e · 2022-10-21T22:26:36.000+02:00
* fix: Ensure that glob patterns are matched correctly. Fixes #16413 Fixes #16299 * Fix cli tests * Upgrade config-array to fix globbing issues
diff --git a/lib/eslint/eslint-helpers.js b/lib/eslint/eslint-helpers.js
@@ -17,6 +17,8 @@ const hash = require("../cli-engine/hash");
 const minimatch = require("minimatch");
 const util = require("util");
 const fswalk = require("@nodelib/fs.walk");
+const globParent = require("glob-parent");
+const isPathInside = require("is-path-inside");
 
 //-----------------------------------------------------------------------------
 // Fixup references
@@ -111,38 +113,38 @@ function isGlobPattern(pattern) {
  * be ignored, so it is consistent with how ignoring works throughout
  * ESLint.
  * @param {Object} options The options for this function.
- * @param {string} options.cwd The directory to search.
+ * @param {string} options.basePath The directory to search.
  * @param {Array<string>} options.patterns An array of glob patterns
  *      to match.
  * @param {FlatConfigArray} options.configs The config array to use for
  *      determining what to ignore.
  * @returns {Promise<Array<string>>} An array of matching file paths
  *      or an empty array if there are no matches.
  */
-async function globSearch({ cwd, patterns, configs }) {
+async function globSearch({ basePath, patterns, configs }) {
 
     if (patterns.length === 0) {
         return [];
     }
 
     const matchers = patterns.map(pattern => {
         const patternToUse = path.isAbsolute(pattern)
-            ? normalizeToPosix(path.relative(cwd, pattern))
+            ? normalizeToPosix(path.relative(basePath, pattern))
             : pattern;
 
         return new minimatch.Minimatch(patternToUse);
     });
 
-    return (await doFsWalk(cwd, {
+    return (await doFsWalk(basePath, {
 
         deepFilter(entry) {
-            const relativePath = normalizeToPosix(path.relative(cwd, entry.path));
+            const relativePath = normalizeToPosix(path.relative(basePath, entry.path));
             const matchesPattern = matchers.some(matcher => matcher.match(relativePath, true));
 
             return matchesPattern && !configs.isDirectoryIgnored(entry.path);
         },
         entryFilter(entry) {
-            const relativePath = normalizeToPosix(path.relative(cwd, entry.path));
+            const relativePath = normalizeToPosix(path.relative(basePath, entry.path));
 
             // entries may be directories or files so filter out directories
             if (entry.dirent.isDirectory()) {
@@ -157,27 +159,48 @@ async function globSearch({ cwd, patterns, configs }) {
 
 }
 
+/**
+ * Performs multiple glob searches in parallel.
+ * @param {Object} options The options for this function.
+ * @param {Array<{patterns:Array<string>,rawPatterns:Array<string>}>} options.searches
+ *      An array of glob patterns to match.
+ * @param {FlatConfigArray} options.configs The config array to use for
+ *      determining what to ignore.
+ * @returns {Promise<Array<string>>} An array of matching file paths
+ *      or an empty array if there are no matches.
+ */
+async function globMultiSearch({ searches, configs }) {
+
+    const results = await Promise.all(
+        [...searches].map(
+            ([basePath, { patterns }]) => globSearch({ basePath, patterns, configs })
+        )
+    );
+
+    return [...new Set(results.flat())];
+}
+
 /**
  * Determines if a given glob pattern will return any results.
  * Used primarily to help with useful error messages.
  * @param {Object} options The options for the function.
- * @param {string} options.cwd The directory to search.
+ * @param {string} options.basePath The directory to search.
  * @param {string} options.pattern A glob pattern to match.
  * @returns {Promise<boolean>} True if there is a glob match, false if not.
  */
-function globMatch({ cwd, pattern }) {
+function globMatch({ basePath, pattern }) {
 
     let found = false;
     const patternToUse = path.isAbsolute(pattern)
-        ? normalizeToPosix(path.relative(cwd, pattern))
+        ? normalizeToPosix(path.relative(basePath, pattern))
         : pattern;
 
     const matcher = new Minimatch(patternToUse);
 
     const fsWalkSettings = {
 
         deepFilter(entry) {
-            const relativePath = normalizeToPosix(path.relative(cwd, entry.path));
+            const relativePath = normalizeToPosix(path.relative(basePath, entry.path));
 
             return !found && matcher.match(relativePath, true);
         },
@@ -187,7 +210,7 @@ function globMatch({ cwd, pattern }) {
                 return false;
             }
 
-            const relativePath = normalizeToPosix(path.relative(cwd, entry.path));
+            const relativePath = normalizeToPosix(path.relative(basePath, entry.path));
 
             if (matcher.match(relativePath)) {
                 found = true;
@@ -201,7 +224,7 @@ function globMatch({ cwd, pattern }) {
     return new Promise(resolve => {
 
         // using a stream so we can exit early because we just need one match
-        const globStream = fswalk.walkStream(cwd, fsWalkSettings);
+        const globStream = fswalk.walkStream(basePath, fsWalkSettings);
 
         globStream.on("data", () => {
             globStream.destroy();
@@ -242,8 +265,10 @@ async function findFiles({
 }) {
 
     const results = [];
-    const globbyPatterns = [];
     const missingPatterns = [];
+    let globbyPatterns = [];
+    let rawPatterns = [];
+    const searches = new Map([[cwd, { patterns: globbyPatterns, rawPatterns: [] }]]);
 
     // check to see if we have explicit files and directories
     const filePaths = patterns.map(filePath => path.resolve(cwd, filePath));
@@ -271,109 +296,76 @@ async function findFiles({
             // directories need extensions attached
             if (stat.isDirectory()) {
 
-                // filePatterns are all relative to cwd
-                const filePatterns = configs.files
-                    .filter(filePattern => {
-
-                        // can only do this for strings, not functions
-                        if (typeof filePattern !== "string") {
-                            return false;
-                        }
-
-                        // patterns starting with ** always apply
-                        if (filePattern.startsWith("**")) {
-                            return true;
-                        }
-
-                        // patterns ending with * are not used for file search
-                        if (filePattern.endsWith("*")) {
-                            return false;
-                        }
-
-                        // not sure how to handle negated patterns yet
-                        if (filePattern.startsWith("!")) {
-                            return false;
-                        }
-
-                        // check if the pattern would be inside the config base path or not
-                        const fullFilePattern = path.join(cwd, filePattern);
-                        const patternRelativeToConfigBasePath = path.relative(configs.basePath, fullFilePattern);
-
-                        if (patternRelativeToConfigBasePath.startsWith("..")) {
-                            return false;
-                        }
-
-                        // check if the pattern matches
-                        if (minimatch(filePath, path.dirname(fullFilePattern), { partial: true })) {
-                            return true;
-                        }
-
-                        // check if the pattern is inside the directory or not
-                        const patternRelativeToFilePath = path.relative(filePath, fullFilePattern);
-
-                        if (patternRelativeToFilePath.startsWith("..")) {
-                            return false;
-                        }
-
-                        return true;
-                    })
-                    .map(filePattern => {
-                        if (filePattern.startsWith("**")) {
-                            return path.join(pattern, filePattern);
-                        }
-
-                        // adjust the path to be relative to the cwd
-                        return path.relative(
-                            cwd,
-                            path.join(configs.basePath, filePattern)
-                        );
-                    })
-                    .map(normalizeToPosix);
-
-                if (filePatterns.length) {
-                    globbyPatterns.push(...filePatterns);
+                // group everything in cwd together and split out others
+                if (isPathInside(filePath, cwd)) {
+                    ({ patterns: globbyPatterns, rawPatterns } = searches.get(cwd));
+                } else {
+                    if (!searches.has(filePath)) {
+                        searches.set(filePath, { patterns: [], rawPatterns: [] });
+                    }
+                    ({ patterns: globbyPatterns, rawPatterns } = searches.get(filePath));
                 }
 
+                globbyPatterns.push(`${normalizeToPosix(filePath)}/**`);
+                rawPatterns.push(pattern);
             }
 
             return;
         }
 
         // save patterns for later use based on whether globs are enabled
         if (globInputPaths && isGlobPattern(filePath)) {
-            globbyPatterns.push(pattern);
+
+            const basePath = globParent(filePath);
+
+            // group in cwd if possible and split out others
+            if (isPathInside(basePath, cwd)) {
+                ({ patterns: globbyPatterns, rawPatterns } = searches.get(cwd));
+            } else {
+                if (!searches.has(basePath)) {
+                    searches.set(basePath, { patterns: [], rawPatterns: [] });
+                }
+                ({ patterns: globbyPatterns, rawPatterns } = searches.get(basePath));
+            }
+
+            globbyPatterns.push(filePath);
+            rawPatterns.push(pattern);
         } else {
             missingPatterns.push(pattern);
         }
     });
 
-    // note: globbyPatterns can be an empty array
-    const globbyResults = await globSearch({
-        cwd,
-        patterns: globbyPatterns,
-        configs,
-        shouldIgnore: true
+    const globbyResults = await globMultiSearch({
+        searches,
+        configs
     });
 
     // if there are no results, tell the user why
     if (!results.length && !globbyResults.length) {
 
-        // try globby without ignoring anything
-        for (const globbyPattern of globbyPatterns) {
+        for (const [basePath, { patterns: patternsToCheck, rawPatterns: patternsToReport }] of searches) {
 
-            // check if there are any matches at all
-            const patternHasMatch = await globMatch({
-                cwd,
-                pattern: globbyPattern
-            });
+            let index = 0;
 
-            if (patternHasMatch) {
-                throw new AllFilesIgnoredError(globbyPattern);
-            }
+            // try globby without ignoring anything
+            for (const patternToCheck of patternsToCheck) {
+
+                // check if there are any matches at all
+                const patternHasMatch = await globMatch({
+                    basePath,
+                    pattern: patternToCheck
+                });
+
+                if (patternHasMatch) {
+                    throw new AllFilesIgnoredError(patternsToReport[index]);
+                }
+
+                // otherwise no files were found
+                if (errorOnUnmatchedPattern) {
+                    throw new NoFilesFoundError(patternsToReport[index], globInputPaths);
+                }
 
-            // otherwise no files were found
-            if (errorOnUnmatchedPattern) {
-                throw new NoFilesFoundError(globbyPattern, globInputPaths);
+                index++;
             }
         }
 
diff --git a/package.json b/package.json
@@ -56,7 +56,7 @@
   "bugs": "https://github.com/eslint/eslint/issues/",
   "dependencies": {
     "@eslint/eslintrc": "^1.3.3",
-    "@humanwhocodes/config-array": "^0.11.5",
+    "@humanwhocodes/config-array": "^0.11.6",
     "@humanwhocodes/module-importer": "^1.0.1",
     "@nodelib/fs.walk": "^1.2.8",
     "ajv": "^6.10.0",
@@ -74,13 +74,14 @@
     "fast-deep-equal": "^3.1.3",
     "file-entry-cache": "^6.0.1",
     "find-up": "^5.0.0",
-    "glob-parent": "^6.0.1",
+    "glob-parent": "^6.0.2",
     "globals": "^13.15.0",
     "grapheme-splitter": "^1.0.4",
     "ignore": "^5.2.0",
     "import-fresh": "^3.0.0",
     "imurmurhash": "^0.1.4",
     "is-glob": "^4.0.0",
+    "is-path-inside": "^3.0.3",
     "js-sdsl": "^4.1.4",
     "js-yaml": "^4.1.0",
     "json-stable-stringify-without-jsonify": "^1.0.1",
diff --git a/tests/fixtures/example-app/app.js b/tests/fixtures/example-app/app.js
@@ -0,0 +1 @@
+console.log("Running");
diff --git a/tests/fixtures/example-app/eslint.config.js b/tests/fixtures/example-app/eslint.config.js
@@ -0,0 +1 @@
+module.exports = [];
diff --git a/tests/fixtures/example-app/subdir/util.js b/tests/fixtures/example-app/subdir/util.js
@@ -0,0 +1 @@
+module.exports = {};
diff --git a/tests/fixtures/example-app2/eslint.config.js b/tests/fixtures/example-app2/eslint.config.js
@@ -0,0 +1,3 @@
+module.exports = [{
+    files: ["subdir*/**/*.js"]
+}];
diff --git a/tests/fixtures/example-app2/subdir1/a.js b/tests/fixtures/example-app2/subdir1/a.js
diff --git a/tests/fixtures/example-app2/subdir2/b.js b/tests/fixtures/example-app2/subdir2/b.js
diff --git a/tests/lib/cli.js b/tests/lib/cli.js
@@ -746,7 +746,7 @@ describe("cli", () => {
                             : `--ignore-path ${getFixturePath(".eslintignore")}`;
                         const filePath = getFixturePath("cli");
                         const expectedMessage = useFlatConfig
-                            ? `All files matched by '${filePath.replace(/\\/gu, "/")}/**/*.js' are ignored.`
+                            ? `All files matched by '${filePath.replace(/\\/gu, "/")}' are ignored.`
                             : `All files matched by '${filePath}' are ignored.`;
 
                         await stdAssert.rejects(async () => {
diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+module.exports = [{`
	`2`	`+ files: ["subdir//.js"]`
	`3`	`+}];`