From c3ce5212f672d95dde3465d7d3c4bf99ff665f8b Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 4 Nov 2022 07:02:42 -0700 Subject: [PATCH] fix: Ensure unmatched glob patterns throw an error (#16462) * fix: Ensure that any unmatched glob patterns throw an error. Fixes #16275 * Cleanup commented out code * Ensure missing ignored pattern throws error * Don't throw an error for ignored files with errorOnUnmatchedPattern: false * Fix error handling --- lib/eslint/eslint-helpers.js | 373 ++++++++++++++++++++++++-------- tests/lib/eslint/flat-eslint.js | 77 ++++++- 2 files changed, 348 insertions(+), 102 deletions(-) diff --git a/lib/eslint/eslint-helpers.js b/lib/eslint/eslint-helpers.js index ed4132cb621..9f2f5e8c31e 100644 --- a/lib/eslint/eslint-helpers.js +++ b/lib/eslint/eslint-helpers.js @@ -27,6 +27,17 @@ const isPathInside = require("is-path-inside"); const doFsWalk = util.promisify(fswalk.walk); const Minimatch = minimatch.Minimatch; +//----------------------------------------------------------------------------- +// Types +//----------------------------------------------------------------------------- + +/** + * @typedef {Object} GlobSearch + * @property {Array} patterns The normalized patterns to use for a search. + * @property {Array} rawPatterns The patterns as entered by the user + * before doing any normalization. + */ + //----------------------------------------------------------------------------- // Errors //----------------------------------------------------------------------------- @@ -47,6 +58,30 @@ class NoFilesFoundError extends Error { } } +/** + * The error type when a search fails to match multiple patterns. + */ +class UnmatchedSearchPatternsError extends Error { + + /** + * @param {Object} options The options for the error. + * @param {string} options.basePath The directory that was searched. + * @param {Array} options.unmatchedPatterns The glob patterns + * which were not found. + * @param {Array} options.patterns The glob patterns that were + * searched. + * @param {Array} options.rawPatterns The raw glob patterns that + * were searched. + */ + constructor({ basePath, unmatchedPatterns, patterns, rawPatterns }) { + super(`No files matching '${rawPatterns}' in '${basePath}' were found.`); + this.basePath = basePath; + this.patternsToCheck = unmatchedPatterns; + this.patterns = patterns; + this.rawPatterns = rawPatterns; + } +} + /** * The error type when there are files matched by a glob, but all of them have been ignored. */ @@ -107,6 +142,69 @@ function isGlobPattern(pattern) { return isGlob(path.sep === "\\" ? normalizeToPosix(pattern) : pattern); } + +/** + * 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.basePath The directory to search. + * @param {string} options.pattern A glob pattern to match. + * @returns {Promise} True if there is a glob match, false if not. + */ +function globMatch({ basePath, pattern }) { + + let found = false; + const patternToUse = path.isAbsolute(pattern) + ? normalizeToPosix(path.relative(basePath, pattern)) + : pattern; + + const matcher = new Minimatch(patternToUse); + + const fsWalkSettings = { + + deepFilter(entry) { + const relativePath = normalizeToPosix(path.relative(basePath, entry.path)); + + return !found && matcher.match(relativePath, true); + }, + + entryFilter(entry) { + if (found || entry.dirent.isDirectory()) { + return false; + } + + const relativePath = normalizeToPosix(path.relative(basePath, entry.path)); + + if (matcher.match(relativePath)) { + found = true; + return true; + } + + return false; + } + }; + + return new Promise(resolve => { + + // using a stream so we can exit early because we just need one match + const globStream = fswalk.walkStream(basePath, fsWalkSettings); + + globStream.on("data", () => { + globStream.destroy(); + resolve(true); + }); + + // swallow errors as they're not important here + globStream.on("error", () => { }); + + globStream.on("end", () => { + resolve(false); + }); + globStream.read(); + }); + +} + /** * Searches a directory looking for matching glob patterns. This uses * the config array's logic to determine if a directory or file should @@ -116,26 +214,62 @@ function isGlobPattern(pattern) { * @param {string} options.basePath The directory to search. * @param {Array} options.patterns An array of glob patterns * to match. + * @param {Array} options.rawPatterns An array of glob patterns + * as the user inputted them. Used for errors. * @param {FlatConfigArray} options.configs The config array to use for * determining what to ignore. + * @param {boolean} options.errorOnUnmatchedPattern Determines if an error + * should be thrown when a pattern is unmatched. * @returns {Promise>} An array of matching file paths * or an empty array if there are no matches. + * @throws {UnmatchedSearchPatternsErrror} If there is a pattern that doesn't + * match any files. */ -async function globSearch({ basePath, patterns, configs }) { +async function globSearch({ + basePath, + patterns, + rawPatterns, + configs, + errorOnUnmatchedPattern +}) { if (patterns.length === 0) { return []; } - const matchers = patterns.map(pattern => { + /* + * In this section we are converting the patterns into Minimatch + * instances for performance reasons. Because we are doing the same + * matches repeatedly, it's best to compile those patterns once and + * reuse them multiple times. + * + * To do that, we convert any patterns with an absolute path into a + * relative path and normalize it to Posix-style slashes. We also keep + * track of the relative patterns to map them back to the original + * patterns, which we need in order to throw an error if there are any + * unmatched patterns. + */ + const relativeToPatterns = new Map(); + const matchers = patterns.map((pattern, i) => { const patternToUse = path.isAbsolute(pattern) ? normalizeToPosix(path.relative(basePath, pattern)) : pattern; + relativeToPatterns.set(patternToUse, patterns[i]); + return new minimatch.Minimatch(patternToUse); }); - return (await doFsWalk(basePath, { + /* + * We track unmatched patterns because we may want to throw an error when + * they occur. To start, this set is initialized with all of the patterns. + * Every time a match occurs, the pattern is removed from the set, making + * it easy to tell if we have any unmatched patterns left at the end of + * search. + */ + const unmatchedPatterns = new Set([...relativeToPatterns.keys()]); + + const filePaths = (await doFsWalk(basePath, { deepFilter(entry) { const relativePath = normalizeToPosix(path.relative(basePath, entry.path)); @@ -151,94 +285,177 @@ async function globSearch({ basePath, patterns, configs }) { return false; } - const matchesPattern = matchers.some(matcher => matcher.match(relativePath)); + /* + * Optimization: We need to track when patterns are left unmatched + * and so we use `unmatchedPatterns` to do that. There is a bit of + * complexity here because the same file can be matched by more than + * one pattern. So, when we start, we actually need to test every + * pattern against every file. Once we know there are no remaining + * unmatched patterns, then we can switch to just looking for the + * first matching pattern for improved speed. + */ + const matchesPattern = unmatchedPatterns.size > 0 + ? matchers.reduce((previousValue, matcher) => { + const pathMatches = matcher.match(relativePath); + + /* + * We updated the unmatched patterns set only if the path + * matches and the file isn't ignored. If the file is + * ignored, that means there wasn't a match for the + * pattern so it should not be removed. + * + * Performance note: isFileIgnored() aggressively caches + * results so there is no performance penalty for calling + * it twice with the same argument. + */ + if (pathMatches && !configs.isFileIgnored(entry.path)) { + unmatchedPatterns.delete(matcher.pattern); + } + + return pathMatches || previousValue; + }, false) + : matchers.some(matcher => matcher.match(relativePath)); return matchesPattern && !configs.isFileIgnored(entry.path); } + })).map(entry => entry.path); + // now check to see if we have any unmatched patterns + if (errorOnUnmatchedPattern && unmatchedPatterns.size > 0) { + throw new UnmatchedSearchPatternsError({ + basePath, + unmatchedPatterns: [...unmatchedPatterns].map( + pattern => relativeToPatterns.get(pattern) + ), + patterns, + rawPatterns + }); + } + + return filePaths; +} + +/** + * Checks to see if there are any ignored results for a given search. This + * happens either when there are unmatched patterns during a search or if + * a search returns no results. + * @param {Object} options The options for this function. + * @param {string} options.basePath The directory to search. + * @param {Array} options.patterns An array of glob patterns + * that were used in the original search. + * @param {Array} options.rawPatterns An array of glob patterns + * as the user inputted them. Used for errors. + * @param {Array} options.patternsToCheck An array of glob patterns + * to use for this check. + * @returns {void} + * @throws {NoFilesFoundError} If there is a pattern that doesn't match + * any files and `errorOnUnmatchedPattern` is true. + * @throws {AllFilesIgnoredError} If there is a pattern that matches files + * when there are no ignores. + */ +async function checkForIgnoredResults({ + basePath, + patterns, + rawPatterns, + patternsToCheck = patterns +}) { + + for (const pattern of patternsToCheck) { + + const patternHasMatch = await globMatch({ + basePath, + pattern + }); + + if (patternHasMatch) { + throw new AllFilesIgnoredError( + rawPatterns[patterns.indexOf(pattern)] + ); + } + } + + // if we get here there are truly no matches + throw new NoFilesFoundError( + rawPatterns[patterns.indexOf(patternsToCheck[0])], + true + ); } /** * Performs multiple glob searches in parallel. * @param {Object} options The options for this function. - * @param {Array<{patterns:Array,rawPatterns:Array}>} options.searches + * @param {Map} options.searches * An array of glob patterns to match. * @param {FlatConfigArray} options.configs The config array to use for * determining what to ignore. + * @param {boolean} options.errorOnUnmatchedPattern Determines if an + * unmatched glob pattern should throw an error. * @returns {Promise>} An array of matching file paths * or an empty array if there are no matches. */ -async function globMultiSearch({ searches, configs }) { +async function globMultiSearch({ searches, configs, errorOnUnmatchedPattern }) { - const results = await Promise.all( - [...searches].map( - ([basePath, { patterns }]) => globSearch({ basePath, patterns, configs }) + /* + * For convenience, we normalized the search map into an array of objects. + * Next, we filter out all searches that have no patterns. This happens + * primarily for the cwd, which is prepopulated in the searches map as an + * optimization. However, if it has no patterns, it means all patterns + * occur outside of the cwd and we can safely filter out that search. + */ + const normalizedSearches = [...searches].map( + ([basePath, { patterns, rawPatterns }]) => ({ basePath, patterns, rawPatterns }) + ).filter(({ patterns }) => patterns.length > 0); + + const results = await Promise.allSettled( + normalizedSearches.map( + ({ basePath, patterns, rawPatterns }) => globSearch({ + basePath, + patterns, + rawPatterns, + configs, + errorOnUnmatchedPattern + }) ) ); - 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.basePath The directory to search. - * @param {string} options.pattern A glob pattern to match. - * @returns {Promise} True if there is a glob match, false if not. - */ -function globMatch({ basePath, pattern }) { - - let found = false; - const patternToUse = path.isAbsolute(pattern) - ? normalizeToPosix(path.relative(basePath, pattern)) - : pattern; + const filePaths = []; - const matcher = new Minimatch(patternToUse); + for (let i = 0; i < results.length; i++) { - const fsWalkSettings = { + const result = results[i]; + const currentSearch = normalizedSearches[i]; - deepFilter(entry) { - const relativePath = normalizeToPosix(path.relative(basePath, entry.path)); + if (result.status === "fulfilled") { - return !found && matcher.match(relativePath, true); - }, - - entryFilter(entry) { - if (found || entry.dirent.isDirectory()) { - return false; + // if the search was successful just add the results + if (result.value.length > 0) { + filePaths.push(...result.value); } - const relativePath = normalizeToPosix(path.relative(basePath, entry.path)); + continue; + } - if (matcher.match(relativePath)) { - found = true; - return true; - } + // if we make it here then there was an error + const error = result.reason; - return false; + // unexpected errors should be re-thrown + if (!error.basePath) { + throw error; } - }; - return new Promise(resolve => { + if (errorOnUnmatchedPattern) { - // using a stream so we can exit early because we just need one match - const globStream = fswalk.walkStream(basePath, fsWalkSettings); + await checkForIgnoredResults({ + ...currentSearch, + patternsToCheck: error.patternsToCheck + }); - globStream.on("data", () => { - globStream.destroy(); - resolve(true); - }); + } - // swallow errors as they're not important here - globStream.on("error", () => {}); + } - globStream.on("end", () => { - resolve(false); - }); - globStream.read(); - }); + return [...new Set(filePaths)]; } @@ -335,47 +552,17 @@ async function findFiles({ } }); - const globbyResults = await globMultiSearch({ - searches, - configs - }); - - // if there are no results, tell the user why - if (!results.length && !globbyResults.length) { - - for (const [basePath, { patterns: patternsToCheck, rawPatterns: patternsToReport }] of searches) { - - let index = 0; - - // 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); - } - - index++; - } - } - - } - // there were patterns that didn't match anything, tell the user if (errorOnUnmatchedPattern && missingPatterns.length) { throw new NoFilesFoundError(missingPatterns[0], globInputPaths); } + // now we are safe to do the search + const globbyResults = await globMultiSearch({ + searches, + configs, + errorOnUnmatchedPattern + }); return [ ...results, diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index b91ddd28c8f..1cbaedfb8b3 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -809,17 +809,69 @@ describe("FlatESLint", () => { }); // https://github.com/eslint/eslint/issues/16275 - it("should throw an error for a missing pattern when combined with a found pattern", async () => { - eslint = new FlatESLint({ - ignore: false, - cwd: getFixturePath("example-app2") + describe("Glob patterns without matches", () => { + + it("should throw an error for a missing pattern when combined with a found pattern", async () => { + eslint = new FlatESLint({ + ignore: false, + cwd: getFixturePath("example-app2") + }); + + await assert.rejects(async () => { + await eslint.lintFiles(["subdir1", "doesnotexist/*.js"]); + }, /No files matching 'doesnotexist\/\*\.js' were found/u); }); - const results = await eslint.lintFiles(["subdir1", "doesnotexist/*.js"]); - assert.strictEqual(results.length, 1); - assert.strictEqual(results[0].messages.length, 0); - assert.strictEqual(results[0].filePath, getFixturePath("example-app2/subdir1/a.js")); - assert.strictEqual(results[0].suppressedMessages.length, 0); + it("should throw an error for an ignored directory pattern when combined with a found pattern", async () => { + eslint = new FlatESLint({ + cwd: getFixturePath("example-app2"), + overrideConfig: { + ignores: ["subdir2"] + } + }); + + await assert.rejects(async () => { + await eslint.lintFiles(["subdir1/*.js", "subdir2/*.js"]); + }, /All files matched by 'subdir2\/\*\.js' are ignored/u); + }); + + it("should throw an error for an ignored file pattern when combined with a found pattern", async () => { + eslint = new FlatESLint({ + cwd: getFixturePath("example-app2"), + overrideConfig: { + ignores: ["subdir2/*.js"] + } + }); + + await assert.rejects(async () => { + await eslint.lintFiles(["subdir1/*.js", "subdir2/*.js"]); + }, /All files matched by 'subdir2\/\*\.js' are ignored/u); + }); + + it("should not throw an error for an ignored file pattern when errorOnUnmatchedPattern is false", async () => { + eslint = new FlatESLint({ + cwd: getFixturePath("example-app2"), + overrideConfig: { + ignores: ["subdir2/*.js"] + }, + errorOnUnmatchedPattern: false + }); + + const results = await eslint.lintFiles(["subdir2/*.js"]); + + assert.strictEqual(results.length, 0); + }); + + it("should not throw an error for a non-existing file pattern when errorOnUnmatchedPattern is false", async () => { + eslint = new FlatESLint({ + cwd: getFixturePath("example-app2"), + errorOnUnmatchedPattern: false + }); + + const results = await eslint.lintFiles(["doesexist/*.js"]); + + assert.strictEqual(results.length, 0); + }); }); // https://github.com/eslint/eslint/issues/16260 @@ -2715,6 +2767,13 @@ describe("FlatESLint", () => { await eslint.lintFiles(["console.js", "non-exist.js"]); }, /No files matching 'non-exist\.js' were found\./u); }); + + // https://github.com/eslint/eslint/issues/16275 + it("a mix of an existing glob pattern and a non-existing glob pattern", async () => { + await assert.rejects(async () => { + await eslint.lintFiles(["*.js", "non-exist/*.js"]); + }, /No files matching 'non-exist\/\*\.js' were found\./u); + }); }); describe("multiple processors", () => {