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

Bug: Can't unignore file in flat config #17964

Closed
1 task
fisker opened this issue Jan 6, 2024 · 12 comments · Fixed by #18020
Closed
1 task

Bug: Can't unignore file in flat config #17964

fisker opened this issue Jan 6, 2024 · 12 comments · Fixed by #18020
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation

Comments

@fisker
Copy link
Contributor

fisker commented Jan 6, 2024

Environment

Node version: v18.18.0
npm version: v9.4.2
Local ESLint version: v8.56.0 (Currently used)
Global ESLint version: Not found
Operating System: linux 5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36

What parser are you using?

Default (Espree)

What did you do?

Configuration
export default [
  {
    ignores: ['files/**', '!files/should-be-linted.js'],
  },
  {
    rules: {
      'no-var': 'error',
    },
  },
];

var a = 1;

What did you expect to happen?

Should report error for files/should-be-linted.js file.

What actually happened?

You are linting "files", but all of the files matching the glob pattern "files" are ignored.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/stackblitz-starters-48wnlw?description=Starter%20project%20for%20Node.js,%20a%20JavaScript%20runtime%20built%20on%20Chrome%27s%20V8%20JavaScript%20engine&file=package.json,eslint.config.js,files%2Fshould-be-linted.js&title=node.new%20Starter

Participation

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

Additional comments

No response

@fisker fisker added bug ESLint is working incorrectly repro:needed labels Jan 6, 2024
@mdjermanovic
Copy link
Member

ignores: ['files/**', '!files/should-be-linted.js'],

This works as intended because, in flat config, these are minimatch patterns, and per the minimatch rules files/** also matches files/ directory (isaacs/minimatch#179), which is not unignored by a subsequent pattern so everything under it is still ignored.

The following patterns would ignore everything under foo/ except for files/should-be-linted.js:

ignores: ['files/*', '!files/should-be-linted.js']

I think we should explain this better in the documentation and migration guide.

@mdjermanovic mdjermanovic added documentation Relates to ESLint's documentation accepted There is consensus among the team that this change meets the criteria for inclusion and removed bug ESLint is working incorrectly repro:needed labels Jan 6, 2024
@fisker
Copy link
Contributor Author

fisker commented Jan 6, 2024

Question: How to unignore file in files/? I can't figure out, it blocks me switch to flat config in Prettier prettier/prettier#15869.

@mdjermanovic
Copy link
Member

mdjermanovic commented Jan 6, 2024

Question: How to unignore file in files/? I can't figure out, it blocks me switch to flat config in Prettier prettier/prettier#15869.

The problem is that the following doesn't unignore jsfmt.spec.js files?

tests/format/**/*
!tests/format/**/jsfmt.spec.js

It's most likely because tests/format/**/* matches directories where jsfmt.spec.js files are, can you try with:

tests/format/**/*.js
!tests/format/**/jsfmt.spec.js

fisker added a commit to fisker/prettier that referenced this issue Jan 6, 2024
@fisker
Copy link
Contributor Author

fisker commented Jan 6, 2024

Thanks, it seems works! I'm going to use

tests/format/**/*.*
!tests/format/**/jsfmt.spec.js

@fisker
Copy link
Contributor Author

fisker commented Jan 6, 2024

I have to say it's really odd..

I guess, my patterns won't work if directory contains . or the file want be unignored doesn't contains . (eg: bin files without extensions).

Another thing is that it seems we treat patterns starts with / as absolute path, which is also different as before, that's why I removed all leading / in https://github.com/prettier/prettier/pull/15869/files

@mdjermanovic
Copy link
Member

I guess, my patterns won't work if directory contains . or the file want be unignored doesn't contains . (eg: bin files without extensions).

I think this is the same problem as with .gitignore patterns, and the solution would be similar:

# first, ignore everything (files and directories)
tests/format/**/*

# then, uningore all directories
!tests/format/**/*/

# then, unignore specific files
!tests/format/**/jsfmt.spec.js

That said, some behavior of flat config ignore patterns does look odd, I'll double-check whether everything works as intended.

Another thing is that it seems we treat patterns starts with / as absolute path, which is also different as before, that's why I removed all leading / in https://github.com/prettier/prettier/pull/15869/files

This is intended. In flat config, patterns should not start with / .

This is different from .gitignore, where /a.js pattern matches only top-level a.js, while a.js pattern matches a.js at any level. In flat config, a.js pattern matches only top-level a.js, **/a.js should be used to match at any level, while /a.js is basically invalid. We should document this as well.

@fisker
Copy link
Contributor Author

fisker commented Jan 6, 2024

tests/format/**/*
!tests/format/**/*/
!tests/format/**/jsfmt.spec.js

Doesn't work as I tested, it will unignore all files.

@mdjermanovic
Copy link
Member

mdjermanovic commented Jan 6, 2024

tests/format/**/*
!tests/format/**/*/
!tests/format/**/jsfmt.spec.js

Doesn't work as I tested, it will unignore all files.

I'd expect this to ignore everything under tests/format/ except for jsfmt.spec.js files. I'll check why it doesn't work that way.

@mdjermanovic
Copy link
Member

tests/format/**/*
!tests/format/**/*/
!tests/format/**/jsfmt.spec.js

Doesn't work as I tested, it will unignore all files.

I'd expect this to ignore everything under tests/format/ except for jsfmt.spec.js files. I'll check why it doesn't work that way.

Looks like a bug to me, I'm working on this.

@nzakas
Copy link
Member

nzakas commented Jan 10, 2024

@mdjermanovic #17980 upgrades config-array, but you may want to add tests to cover this issue.

@mdjermanovic
Copy link
Member

@mdjermanovic #17980 upgrades config-array, but you may want to add tests to cover this issue.

Yes, I'll add test cases.

@mdjermanovic
Copy link
Member

Tests: #18018

I'm working on the docs now.

nzakas pushed a commit that referenced this issue Jan 22, 2024
* test: add more tests for ignoring files and directories

Refs #17964

* add one more test
nzakas added a commit that referenced this issue Jan 24, 2024
* docs: add more examples to flat config ignores docs

Fixes #17964

* Update docs/src/use/configure/ignore.md

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

* clarify second example

---------

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
snitin315 pushed a commit that referenced this issue Feb 1, 2024
* test: add more tests for ignoring files and directories

Refs #17964

* add one more test

(cherry picked from commit 8c1b8dd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants