Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
180 changes: 86 additions & 94 deletions lib/eslint/eslint-helpers.js
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
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
@@ -0,0 +1 @@
console.log("Running");
1 change: 1 addition & 0 deletions tests/fixtures/example-app/eslint.config.js
@@ -0,0 +1 @@
module.exports = [];
1 change: 1 addition & 0 deletions tests/fixtures/example-app/subdir/util.js
@@ -0,0 +1 @@
module.exports = {};
3 changes: 3 additions & 0 deletions tests/fixtures/example-app2/eslint.config.js
@@ -0,0 +1,3 @@
module.exports = [{
files: ["subdir*/**/*.js"]
}];
Empty file.
Empty file.
2 changes: 1 addition & 1 deletion tests/lib/cli.js
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