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

Docs: Unexpected file matching behavior when using ignores without files #18475

Open
1 task
MansurAliKoroglu opened this issue May 19, 2024 · 12 comments · May be fixed by #18539
Open
1 task

Docs: Unexpected file matching behavior when using ignores without files #18475

MansurAliKoroglu opened this issue May 19, 2024 · 12 comments · May be fixed by #18539
Labels
documentation Relates to ESLint's documentation repro:yes

Comments

@MansurAliKoroglu
Copy link

Docs page(s)

https://eslint.org/docs/latest/use/configure/configuration-files#excluding-files-with-ignores

What documentation issue do you want to solve?

I was checking the flat config and was checking the ignores / files patterns.

Documentation specifies that

If ignores is used without files and there are other keys (such as rules), then the configuration object applies to all files except the ones specified in ignores

So using the config below should match all the files except the ignored one. Because,

Effectively, this is like having files set to **/*.

export default [
    {
        ignores: ["**/*.config.js"],
        rules: {
            semi: "error"
        }
    }
];

To test this I created a fail.txt in the root. Apparently with the configuration file above it didn't match fail.txt. Maybe it's because files default to **/*.js, **/*.cjs and **/*.mjs.

If that's the case docs confused me because it says if an ignore pattern used without files it should match all files except the ignored ones.

What confused me most is, I also tested removing ignores and setting a files pattern like below:

export default [
    {
        files: ["**/*"],
        rules: {
            semi: "error"
        }
    }
];

Eslint didn't match fail.txt again.

With files set to ['**/*'] when I run npx eslint --inspect-config I can see fail.txt matches with 1 config object.

image

So, I am not sure if I completely misunderstood the concept, or docs are incorrect or is there a bug.

What do you think is the correct solution?

  • Fix the docs if they are incorrect or make it clear.
  • Fix the bug if there is.

Participation

  • I am willing to submit a pull request for this change.

Additional comments

OS: Kubuntu 22.04
node version: 20.11.1
eslint version: 9.3.0

@MansurAliKoroglu MansurAliKoroglu added the documentation Relates to ESLint's documentation label May 19, 2024
@fasttime
Copy link
Member

Thanks for the issue @MansurAliKoroglu.

Documentation specifies that

If ignores is used without files and there are other keys (such as rules), then the configuration object applies to all files except the ones specified in ignores

Note that the documentation refers to the configuration object, not the config file. In your first example, the configuration is an array consisting of just one configuration object:

export default [
    {
        ignores: ["**/*.config.js"],
        rules: {
            semi: "error"
        }
    }
];

This configuration object does match all files except "**/*.config.js", but it does not cause any new files to be enumerated. It still applies to files that are enumerated by the files property of other configs, if any. For example:

export default [
    {
        files: ["fail.txt"]
    },
    {
        ignores: ["**/*.config.js"],
        rules: {
            semi: "error"
        }
    }
];

With the above config, the semi rule applies to fail.txt, because it is mached by the files property in the first configuration object. I agree that the documentation isn't clear about this.

What confused me most is, I also tested removing ignores and setting a files pattern like below:

export default [
    {
        files: ["**/*"],
        rules: {
            semi: "error"
        }
    }
];

Eslint didn't match fail.txt again.

"**/*" only matches files and directories without extension (e.g. LICENSE). To match files with any extension, including .txt, one could use "**/*.*". To match all files with or without extension, one could use a pattern like "**".

With files set to ['**/*'] when I run npx eslint --inspect-config I can see fail.txt matches with 1 config object.

I think that both the documentation (here) and the config inspector are incorrect in considering that "**/*" should match all files. This is not how the current implementation behaves. I would like someone else from the team to verify. @eslint/eslint-team

@nzakas
Copy link
Member

nzakas commented May 20, 2024

I put this together regarding **/*.config.js:
https://stackblitz.com/edit/stackblitz-starters-zrom9q?file=index.js

It behaves as the documentation indicates. The eslint.config.js file is ignored while index.js is not.

Before going any further here, I'd like to see some repros of other behavior that seems incorrect.

@fasttime
Copy link
Member

@MansurAliKoroglu would you be able to create a small repo or a StackBlitz repro to show all files required to reproduce the issue (eslint.config.js, fail.txt, package.json, etc.?). This would help to avoid confusion and reduce arbitrariness for reviewers who try to look into the problem.

@MansurAliKoroglu
Copy link
Author

There were 2 problems. All explained by @fasttime

  1. files: ["**/*"] does not match to fail.txt.

Explanation

"/*" only matches files and directories without extension (e.g. LICENSE). To match files with any extension, including .txt, one could use "/.". To match all files with or without extension, one could use a pattern like "**".

  1. If ignores is used without files and there are other keys configuration object applies to all files except the ignores ones

Explanation

This configuration object does match all files except "**/*.config.js", but it does not cause any new files to be enumerated. It still applies to files that are enumerated by the files property of other configs, if any.

So there is no actual bug with ignores or files.

But I think the documentation here should be clear about this. Ignores without files is not targeting all files in the project. It targets all files targeted by other configuration objects + default target (.js, .cjs, .mjs)

But config inspector might have a bug because it shows file.txt is matched by **/*

Here is the stackblitz. Run npx eslint --inspect-config and test matching file path in UI using: fail.txt or check the files tab. You will see all files are listed.

https://stackblitz.com/edit/stackblitz-starters-99hktx

@mdjermanovic
Copy link
Member

**/* matches all files but doesn't make them "lintable". By default, ESLint lints only .js, .cjs, and .mjs files. Other files must be matched by at least one config object with a files pattern that doesn't end with /** or /*.

@mdjermanovic
Copy link
Member

mdjermanovic commented May 27, 2024

But config inspector might have a bug because it shows file.txt is matched by **/*

Here is the stackblitz. Run npx eslint --inspect-config and test matching file path in UI using: fail.txt or check the files tab. You will see all files are listed.

https://stackblitz.com/edit/stackblitz-starters-99hktx

I think the bug here is that config inspector doesn't show that file.txt is "not included" (effectively ignored). It is matched by **/* but isn't matched by any patterns that don't end with /** or /*.

@nzakas
Copy link
Member

nzakas commented May 27, 2024

Shall we move this issue to the config-inspector repo?

@mdjermanovic
Copy link
Member

If there's no need to update the docs, we could just move this issue to eslint/config-inspector.

If we want to update the docs, it might be better to keep this issue to track that task, and open a separate issue on eslint/config-inspector.

@MansurAliKoroglu
Copy link
Author

I think it would be nice to clarify the documentation for better understanding.

@fasttime
Copy link
Member

I'd be also in favor of clarifying the current behavior. It looks undocumented.

@nzakas
Copy link
Member

nzakas commented May 28, 2024

@fasttime do you want to open a PR for that?

@mdjermanovic can you open an issue for config-inspector?

@fasttime
Copy link
Member

@fasttime do you want to open a PR for that?

Sure, I'll open a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Relates to ESLint's documentation repro:yes
Projects
Status: Feedback Needed
Development

Successfully merging a pull request may close this issue.

4 participants