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

Override files globbing logic breaks with --config #11558

Closed
epmatsw opened this issue Mar 27, 2019 · 13 comments · Fixed by #12887
Closed

Override files globbing logic breaks with --config #11558

epmatsw opened this issue Mar 27, 2019 · 13 comments · Fixed by #12887
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features
Projects

Comments

@epmatsw
Copy link

epmatsw commented Mar 27, 2019

Tell us about your environment

  • ESLint Version: 5.15.3
  • Node Version: 10.15.0
  • npm Version: 6.9.0

What parser (default, Babel-ESLint, etc.) are you using?

Please show your full configuration:

Configuration
{
    "overrides": [{
        "files": ["**/blah.js"],
        "rules": {
            "prefer-const": 0
        }
    }],
    "parserOptions": {
        "ecmaVersion": 2018
    },
    "rules": {
        "prefer-const": 2
    }
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

In this case, I have a configuration that is inside of my node_modules, or another subdirectory. Using the --config flag, I use that file directly rather than having a config in the root of my project. In the file, as above, I've got a glob that should match some files. However, that glob is not matched correctly. In particular, in getConfigVector in config, we use config.baseDirectory to compute a relative path. In this case, because the config is in a sibling directory, that relative path starts with something like ../, which makes the glob always fail.

N/A
eslint --config ./configs/.eslintrc.json .

What did you expect to happen?
It seems likely that if someone is using --config, they probably want the globs to be computed relative to cwd rather than wherever that config happens to be on disk.

What actually happened? Please include the actual, raw output from ESLint.
Globs are computed relative to the config's location on disk, which makes them not work super well.

Are you willing to submit a pull request to fix this bug?

@epmatsw epmatsw added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Mar 27, 2019
@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Apr 27, 2019
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@epmatsw
Copy link
Author

epmatsw commented Apr 27, 2019

Never even triaged :(

@platinumazure
Copy link
Member

Apologies @epmatsw, the ESLint team has been very busy lately and things have slipped through the cracks. I'll reopen this.

@platinumazure platinumazure reopened this Apr 27, 2019
@platinumazure platinumazure added core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed auto closed The bot closed this issue triage An ESLint team member will look at this issue soon labels Apr 27, 2019
@platinumazure
Copy link
Member

@eslint/eslint-team Thoughts? This seems like undesirable behavior to me.

@aladdin-add
Copy link
Member

what if creating an eslint config in cwd?
maybe it's related to the glob matching, I've encountered a similar issue a while ago.

@mysticatea
Copy link
Member

ESLint makes the paths to source codes to the relative paths from the config file then compares with glob patterns.

By the way, how does --ignore-path option handle paths? I think that we should do a same.

@platinumazure platinumazure self-assigned this Apr 30, 2019
@epmatsw
Copy link
Author

epmatsw commented Apr 30, 2019

Wrote up a quick demo in https://github.com/epmatsw/eslintglobs.

npm test demonstrates this bug. I think src/a.js should not have no-var applied because the override should disable it, but it is flagged. src/b.js should be ignored, and it is.

npm run control runs a control test where it uses the same configuration but in a different location. I would expect the behavior to be the same, but in this case we get the expected behavior where src/a.js is correctly not flagged.

Basically, to answer @mysticatea's question, it looks like --ignore-path is computed relative to the cwd, while override globs are computed relative to the config file's location.

@mysticatea
Copy link
Member

Hmm.

The two indicate the paths are relative to the .eslintignore file.
I think our intention is that all paths are relative to the file which the path is written. But if a path is not written in any file, the path is relative to CWD.

However, .eslintignore may have a bug since #5627 because it adds the content of --ignore-path file into the default ignore data as well. (Honestly, I don't fully understand the IgnoredPaths class...)

On the other hand, "Ignoring Files and Directories" section says "Paths are relative to .eslintignore location or the current working directory." But this description is not changed since ESLint 2.0.0; I think this documentation is outdated.

@epmatsw
Copy link
Author

epmatsw commented Jun 10, 2019

Any more thoughts on this? Seems like it would probably be a breaking change to fix it, and it looks like the window for 6.0 is closing soon...

@mysticatea
Copy link
Member

mysticatea commented Jun 10, 2019

  • 6.0.0 has been feature-frozen (breaking change frozen) in April.
  • I think the current behavior is intentional.

@epmatsw
Copy link
Author

epmatsw commented Jun 10, 2019

6.0.0 has been feature-frozen (breaking change frozen) in April.

Ah, bummer.

I think the current behavior is intentional.

It might be, but I think it's not optimal. It makes it really difficult to use overrides and the --config option together, especially when the overrides can be in files in mostly-unpredictable locations in node_modules.

(Note: this is in the 5.x series, it's possible that the overrides changes in 6.x will fix this in some other way or make people write configs that are less likely to trigger this issue)

@gziolo
Copy link

gziolo commented Sep 18, 2019

I run into exactly the same issue as I learned after filing #12278 which was closed as a duplicate. You can find more details there. I also think that ESLint should scan from the project’s current directory when executing with —config option.

@mysticatea
Copy link
Member

I have posted an RFC eslint/rfcs#37.

@mysticatea mysticatea added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Nov 7, 2019
@mysticatea mysticatea moved this from Accepted, ready to implement to Issues which have PR in v7.0.0 Feb 8, 2020
v7.0.0 automation moved this from Issues which have PR to Done Mar 23, 2020
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 21, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features
Projects
No open projects
v7.0.0
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants