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: --ignore-pattern in flat config mode should be relative to cwd #16425

Merged
merged 3 commits into from Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion eslint.config.js
Expand Up @@ -92,7 +92,7 @@ module.exports = [
"tmp/**",
"tools/internal-rules/node_modules/**",
"**/test.js",
"!**/.eslintrc.js"
"!.eslintrc.js"
]
},
{
Expand Down
10 changes: 4 additions & 6 deletions lib/cli.js
Expand Up @@ -143,12 +143,6 @@ async function translateOptions({
overrideConfig[0].plugins = plugins;
}

if (ignorePattern) {
overrideConfig.push({
ignores: ignorePattern
});
}

} else {
overrideConfigFile = config;

Expand Down Expand Up @@ -193,6 +187,10 @@ async function translateOptions({
options.useEslintrc = eslintrc;
options.extensions = ext;
options.ignorePath = ignorePath;
} else {
if (ignorePattern) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rearrange the conditionals in these blocks? It’s a bit hard to follow an else after a not equals. Maybe structure as “if configType is flat, then add ignorePatterns, else do the other stuff”?

Also, I don’t think you need to check for ignorePattern before adding ignorePatterns to options.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for the suggestion!

options.ignorePatterns = ignorePattern;
}
}

return options;
Expand Down
11 changes: 0 additions & 11 deletions lib/eslint/flat-eslint.js
Expand Up @@ -362,17 +362,6 @@ async function calculateConfigArray(eslint, {
const negated = pattern.startsWith("!");
const basePattern = negated ? pattern.slice(1) : pattern;

/*
* Ignore patterns are considered relative to a directory
* when the pattern contains a slash in a position other
* than the last character. If that's the case, we need to
* add the relative ignore path to the current pattern to
* get the correct behavior. Otherwise, no change is needed.
*/
if (!basePattern.includes("/") || basePattern.endsWith("/")) {
return pattern;
}

Comment on lines -365 to -375
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second test case was failing because of this conditional. I'm not sure why we had this special case. It could be a leftover from the version where patterns had .gitignore semantics (e.g, *.js matches at any level).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I’m pretty sure that’s left over from gitignore compatibility.

return (negated ? "!" : "") +
path.posix.join(relativeIgnorePath, basePattern);
});
Expand Down
3 changes: 3 additions & 0 deletions tests/fixtures/cli/ignore-pattern-relative/.eslintrc.js
@@ -0,0 +1,3 @@
module.exports = {
root: true
};
@@ -0,0 +1 @@
module.exports = [];
Empty file.
26 changes: 26 additions & 0 deletions tests/lib/cli.js
Expand Up @@ -798,6 +798,32 @@ describe("cli", () => {
assert.strictEqual(exit, 0);
});

it(`should interpret pattern that contains a slash as relative to cwd with configType:${configType}`, async () => {
process.cwd = () => getFixturePath("cli/ignore-pattern-relative/subdir");

/*
* The config file is in `cli/ignore-pattern-relative`, so this would fail
* if `subdir/**` ignore pattern is interpreted as relative to the config base path.
*/
const exit = await cli.execute("**/*.js --ignore-pattern subdir/**", null, useFlatConfig);

assert.strictEqual(exit, 0);

await stdAssert.rejects(
async () => await cli.execute("**/*.js --ignore-pattern subsubdir/*.js", null, useFlatConfig),
/All files matched by '\*\*\/\*\.js' are ignored/u
);
});

it(`should interpret pattern that doesn't contain a slash as relative to cwd with configType:${configType}`, async () => {
process.cwd = () => getFixturePath("cli/ignore-pattern-relative/subdir/subsubdir");

await stdAssert.rejects(
async () => await cli.execute("**/*.js --ignore-pattern *.js", null, useFlatConfig),
/All files matched by '\*\*\/\*\.js' are ignored/u
);
});

if (useFlatConfig) {
it("should ignore files if the pattern is a path to a directory (with trailing slash)", async () => {
const filePath = getFixturePath("cli/syntax-error.js");
Expand Down