Skip to content

Commit

Permalink
fix: Ensure that glob patterns are matched correctly. (#16449)
Browse files Browse the repository at this point in the history 
* fix: Ensure that glob patterns are matched correctly.

Fixes #16413
Fixes #16299

* Fix cli tests

* Upgrade config-array to fix globbing issues
  • Loading branch information
@nzakas
nzakas committed Oct 21, 2022
1 parent 651649b commit 33668ee
Show file tree
Hide file tree
Showing 10 changed files with 147 additions and 103 deletions.
180 changes: 86 additions & 94 deletions lib/eslint/eslint-helpers.js
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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()) {
Expand All @@ -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);
},
Expand All @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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++;
}
}

Expand Down
5 changes: 3 additions & 2 deletions package.json
View file
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/example-app/app.js
View file
@@ -0,0 +1 @@
console.log("Running");
1 change: 1 addition & 0 deletions tests/fixtures/example-app/eslint.config.js
View file
@@ -0,0 +1 @@
module.exports = [];
1 change: 1 addition & 0 deletions tests/fixtures/example-app/subdir/util.js
View file
@@ -0,0 +1 @@
module.exports = {};
3 changes: 3 additions & 0 deletions tests/fixtures/example-app2/eslint.config.js
View file
@@ -0,0 +1,3 @@
module.exports = [{
files: ["subdir*/**/*.js"]
}];
Empty file added tests/fixtures/example-app2/subdir1/a.js
View file
Empty file.
Empty file added tests/fixtures/example-app2/subdir2/b.js
View file
Empty file.
2 changes: 1 addition & 1 deletion tests/lib/cli.js
View file
Expand Up @@ -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 () => {
Expand Down

0 comments on commit 33668ee

Please sign in to comment.