eslint: Bug: [flat config] Global `ignores` matches differently from non-global `ignores`

Environment

Node version: v18.12.1 npm version: 8.19.2 Local ESLint version: v8.41.0 Global ESLint version: none Operating System: Linux or MacOS

What parser are you using?

Default (Espree)

What did you do?

Configuration

`eslint-global-ignore.config.js`

export default [
  {
    ignores: ['ignored/'],
  },
  {
    rules: {
      'no-undef': 'error',
    },
  },
];

`eslint-files-ignore.config.js`

export default [
  {
    files: ['**/*.js'],
    ignores: ['ignored/'],
    rules: {
      'no-undef': 'error',
    },
  },
];

`file.js` and `ignored/file.js`

console.log();

What did you expect to happen?

Not sure what is the correct intended behavior here, so raising this issue for the maintainers’ consideration. Based on the documentation (Globally ignoring files with ignores), it looks like ignores can match and ignore an entire directory tree:

{
  // Ignores directory tree
  ignores: ["ignored/"],
}

However, if the same ignores pattern is used in a configuration block which also has files, then the pattern is interpreted completely differently, and does not have the same effect:

{
  files: ["**/*.js"],
  // Does *not* ignore directory tree
  ignores: ["ignored/"],
}

Rather, to ignore the entire top-level ignored tree, the ignores pattern needs to be adjusted as follows:

 {
   files: ["**/*.js"],
-  ignores: ["ignored/"],
+  ignores: ["ignored/**"], 
 }

Is this difference in behavior between the two ignores (global and non-global) intentional? And if so, can it be explicitly documented, if not already? It seems like a surprising “gotcha”.

What actually happened?

$ ESLINT_USE_FLAT_CONFIG=true eslint -c eslint-global-ignore.config.js .

/repo/file.js
  1:1   error  'console' is not defined  no-undef

✖ 1 problems (1 errors, 0 warnings)

$ ESLINT_USE_FLAT_CONFIG=true eslint -c eslint-files-ignore.config.js .

/repo/ignored/file.js
  1:1   error  'console' is not defined  no-undef

/repo/file.js
  1:1   error  'console' is not defined  no-undef

✖ 2 problems (2 errors, 0 warnings)

Link to Minimal Reproducible Example

https://stackblitz.com/edit/eslint-ignore-pattern-inconsistency

Participation

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

Additional comments

No response

About this issue

Original URL
State: closed
Created a year ago
Comments: 17 (16 by maintainers)

Most upvoted comments

@Rec0iL99 @mdjermanovic please be sure to take a stab at priority and impact when triaging. 😃

nzakas on May 26, 2023

@mxxk @fasttime while I appreciate the enthusiasm, I think this is a pretty hairy change that I’d like to take myself. Definitely not a good first contribution to ESLint due to the complexity.

nzakas on May 29, 2023

@fasttime I would be happy to take a stab at it. Since I’m not familiar with the code, would you mind including some pointers to the following places in the flat config part of the codebase?

Thanks for your interest @mxxk! I haven’t looked into most of this code before, but I’ll try to point out what I think are the relevant parts. I will also do my best to answer any questions that may pop up while you progress.

Where entire directory trees are potentially ignored by global ignores during traversal

This is where a directory matched by a global ignore pattern is filtered out:

https://github.com/eslint/eslint/blob/4f5440db631707b17140c4e5cc7beb223afbd2b9/lib/eslint/eslint-helpers.js#L279

The implementation of isDirectoryIgnored is in a different repository: https://github.com/humanwhocodes/config-array/blob/v0.11.9/src/config-array.js#L865-L925

The configs object on which the method is called is the resolved FlatConfigArray object that contains information about all global ignores used.

Where individual files are potentially ignored by non-global ignores

This is also done in the config-array repo:

https://github.com/humanwhocodes/config-array/blob/v0.11.9/src/config-array.js#L302

The function shouldIgnorePath is very simple, in that it tries to match a given file path against a list of ignore patterns. But what we would actually like to do additionally is to match the parent directories of that file against the list of ignore patterns. A possible impementation could be similar to what is being done in the core of isDirectoryIgnored, except that in this case the list of ignore patterns would have to come directly from a config element (config.ignores), not from the global ignores of the resolved FlatConfigArray object. Also, I don’t think that we can reuse the caching logic of isDirectoryIgnored, because the results would need to be specific to the config element that contains the ignore patterns.

When you feel confident enough to start working on a change, this is what I would do next:

Fork both the eslint and config-array repos. Make sure you can test and build both projects locally.
Do any required changes to config-array in a local branch, including tests.
Test your updated version of config-array in eslint. You can copy your local config-array folder to eslint/node_modules/@humanwhocodes/config-array manually or using npm link.
Commit your changes to config-array and submit a PR.
When the PR is merged, bump the version of config-array in eslint, add unit tests and update the docs.
Commit your changes to eslint and submit a new PR.

fasttime on May 29, 2023

Is this difference in behavior between the two ignores (global and non-global) intentional?

It is intentional. Patterns in global ignores can match directories for performance reasons so that ESLint doesn’t traverse ignored directories. Patterns in non-global ignores are supposed to match in the same way as patterns in files, that is to match only files.

In my opinion, this is indeed confusing. I think we should update the non-global ignores matching to work the same as global ignores matching. @nzakas what do you think?

mdjermanovic on May 24, 2023

Hi @mxxk, thanks for the issue. ignores uses minimatch which is mentioned in the docs.

> minimatch('ignored/file.js', 'ignored/')
false

> minimatch('ignored/file.js', 'ignored/**')
true

You would need to specify ignored/** for any files in the ignored folder to match in the ignores key. This does not look like a bug.

Edit: I now see your point about the ignores globally working without ignored/**. I’m confused.

@mdjermanovic please verify. Thanks.

Rec0iL99 on May 24, 2023