eslint: Remove warnings when linting ignored files

I was putting this off for a long time, sorry about that. Finished the RFC: https://github.com/eslint/rfcs/pull/90

This feature is ready to implement per https://github.com/eslint/rfcs/tree/main/designs/2022-supress-ignored-file-warnings.

IMHO, it does not make sense to be just logged, as in most cases it was an error to pass ignored files to eslint.

Does it really cause any errors? If I understand correctly, if a file is ignored and passed in a file list, it’s just not linted. In that case, it’s good to inform that File ignored because of a matching ignore pattern, but does that really warrant a warning?

Changing all rules to error would probably work, but I don’t think that’s a good solution – some warnings are warnings for a reason. For example, no-console is set as warn in my codebase, because it’s not really an error – it has no effect on functionality. However, I have max-warnings 0 on my lint-staged command, because I don’t want to leave unnecessary console.logs when commiting code.

Additionaly, if I’m extending a config, I’d need to override all the warn rules.

I have noticed lib/cli uses log.info sometimes Example. Maybe log.info should be used instead of pushing warning messages in createIgnoreResult function?

As of now, eslint mix linting rules and config related problems to the output. But you just want to disallow the former (please correct if I’m misunderstanding).

If you want to disallow some linting warnings, you can just config the rules to "error", and no --max-warnings 0 needed. so, I’m soft 👎 to add something like this in eslint core.

I solved this issue by excluding the files from glob pattern like this:

  "lint-staged": {
    "**/*.{ts,tsx,js} !(*.generated.ts)": [
      "eslint --fix --max-warnings=0",
      "nx format:write"
    ]
  },

@domnantas @sgronblo

lint-staged is not necessarily used with eslint (it may be used with prettier, test runners, etc), therefore it shouldn’t care about .eslintignore. So the issue is not on lint-staged end.

Right now the only solution is creating .lintstagedrc.js and using eslint CLIEngine.isPathIgnored as outlined in lint-staged README. If there was a CLI option, users of lint-staged or pre-commit could add the flag to the eslint command to achieve this:

"lint-staged": {
	"*.js": [
		"eslint --max-warnings=0 --no-warn-ignored"
	]
},

Based on TSC meeting a couple of years back, it was decided an additional flag is not worth adding. However, it seems they were considering that only integrations that use ESLint CLI are affected. Based on the activity in this issue, past issues, and issues on projects like lint-staged, it clearly affects end-users of eslint too. I’m in progress of writing the RFC for this

Would be great to see a new CLI parameter that disables this warning sometime soon, glad that there’s an RFC in progress 😃 Even if you would like lint-staged to work differently, lint-staged isn’t the only tool or use case for when you’d want to run ESLint on an ignored file, so this param would definitely be very useful

We decided in the TSC meeting that we’d like to provide a solution to this. We can discuss possible options here on the issue. One option is replicating eslint.lintText()'s warnIgnored option on eslint.lintFiles() and exposing it via a CLI option.

That’s why I support an info message - inform the user that the file they passed is ignored. But I don’t understand why should it be a warning or an error. Is it breaking any linting rules? No. Does eslint CLI breaks in any way? No, the files are just ignored. Agree, adding an option to disable the warnings (just like --no-error-on-unmatched-pattern) is another way to fix this issue.

the more cases are users misconfigured the eslintignore.

Imagine this: the file foo.js was ignored, users ran eslint foo.js output 0 warning/error. they might think it was linted successfully, but it was never linted! To catch this kind of cases, it should be an error (at least a warning). A solution is to add an option to disable these warnings.

@nzakas Yes, I have explicitly ignored the files in .eslintignore, but eslint is throwing warnings. I know I can run eslint --max-warnings 0 . and it will work fine, but lint-staged provides a list of all staged files to eslint, so it’s out of my control – eslint --max-warnings 0 ignoredFile.js shows the warning and exits the process with code 1.

Here’s a reproduction sandbox: https://codesandbox.io/s/eslint-ignored-warning-w5wsm?file=/ignoredFile.js You’ll need to click + to open a new terminal and run yarn lint

@aladdin-add I just tried that flag, thanks for pointing it out, but it does not fix this issue (it prevents errors when no files match with given pattern)

IMHO, it does not make sense to be just logged, as in most cases it was an error to pass ignored files to eslint.

It’s not only a lint-staged issue. We get changed files with git in CI and pass the output to eslint, to be able to lint only the files changed in a PR. Prettier handles it perfectly with its cli flag ignore-unknown.

Since I’m authoring this issue, I’ll take care of the RFC Edit: Quite busy lately, might take 1-2 weeks 😅

@jacobrask It looks like this functionality requires either a new CLI option or introducing a breaking change, so per our policies it has to go through the RFC process. Please feel free to start an RFC, that would be great.

Based on the discussion on this issue, it seems that the preferred design choice is to add a new CLI option, but in the RFC we can certainly discuss alternatives.

TSC Summary: We currently output a linter warning when a file is passed on the command line that is ignored due to an ignore file. Because it’s reported as a linter warning, it counts towards the limit set by --max-warnings, which can in turn cause the process to exit with code 1.

TSC Question: Should we change this warning in some way? We could just output it to stderr so it doesn’t count as a linter warning, but then that information will not be available to formatters.

Maybe log.info should be used instead of pushing warning messages in createIgnoreResult function?

I think this is the decision here: does this warning need to be an ESLint warning so it can be caught and handled as such, or should it be just console output that is separate and doesn’t need to be caught? I can see arguments both ways, so we need the team to chime in.

Lint-staged should embrace what was already ignored by .eslintignore. It’s totally a lint-staged problem. The way this is set up, I have to pass a separate lint command to lint-staged. This is extremely confusing. We are using Lerna, so there’s a base eslint config, which can be overridden at the directory level, and then overridden by lint commands in package.json. NOW we have an additional command that we pass only to lint-staged (just for dev workflow). There should not need to be additional configuration. It should not pass ignored files to the linter, even if they’re staged.

@EightArmCode see this reply above.

the more cases are users misconfigured the eslintignore. Imagine this: the file foo.js was ignored, users ran eslint foo.js output 0 warning/error. they might think it was linted successfully, but it was never linted! To catch this kind of cases, it should be an error (at least a warning). A solution is to add an option to disable these warnings.

The warning seems reasonable to me in the context of manual use of the command line, for example. It’s hard to design a single behavior that works well in all cases. The main cases I am aware of where the current behavior doesn’t work well are with lint-staged and pre-commit. Hence, the consensus is to make the discussed behavior configurable.

Hi @Kurt-von-Laven

Now that I understand the problem a little better, I think the problem is on the lint-staged side. Cheers! Thanks for the quick reply.

We could just output it to stderr so it doesn’t count as a linter warning, but then that information will not be available to formatters.

Does it really need to be available to formatters? For me this sounds more like a problem related with eslint runtime (just like missing arguments, invalid argument value, those are not available for formatters, right?), not a code lint warning/error

I share the opinion that this doesn’t look like a code lint warning, but the problem is that it already is a code lint warning and correlates with other features as such. We would like to make a change that supports the lint-staged use case in a way that wouldn’t break other use cases. Removing the warning by default feels like a breaking change for a couple of reasons:

• Exit with status code 1 on eslint --max-warnings 0 ignored-file.js might be desirable behavior for other use cases.
• Formatters - for end users who read the output from the console it’s seemingly same if we output the message to stderr instead of passing it to the formatter, but formatters are used for integrations as well.

A new CLI option seems like the most straightforward way to support lint-staged and similar use cases while not breaking anything else.

have you tried –no-error-on-unmatched-pattern (it was introduced in eslint v6.8.0)?

@EightArmCode see this reply above.

the more cases are users misconfigured the eslintignore.

Imagine this: the file foo.js was ignored, users ran eslint foo.js output 0 warning/error. they might think it was linted successfully, but it was never linted! To catch this kind of cases, it should be an error (at least a warning). A solution is to add an option to disable these warnings.

The warning seems reasonable to me in the context of manual use of the command line, for example. It’s hard to design a single behavior that works well in all cases. The main cases I am aware of where the current behavior doesn’t work well are with lint-staged and pre-commit. Hence, the consensus is to make the discussed behavior configurable.

Would you be willing to accept a PR for a --no-warning-on-ignored-file flag?

Or could --no-error-on-unmatched-pattern be made to disable these warnings as well? It seems like the use cases for the both flags are the same - passing all changed files to eslint, expecting eslint to just lint the files it would have if given a glob pattern. For reference, Prettier’s related CLI flags are --no-error-on-unmatched-pattern that I believe came from eslint, and --ignore-unknown which is used to ignore files prettier doesn’t understand. Anything in .prettierignore is ignored by default though, without errors.

@okonet and @iiroj should be aware of this discussion since lint-staged is the predominant reported use case driving this feature request, and it implies an update to lint-staged’s README regarding ESLint use.

The reason it’s a warning is because silently ignoring the request to lint a specific file is confusing. You can’t tell the difference between a file with no lint errors and a file that was ignored.

We don’t have the concept of “info” severity, so that’s not an option.