feat: Add suppress ignored file warnings RFC

[domnantas]

Summary

When ignored files are explicitly linted, ESLint shows a warning. This causes problems when using tools which pass the file list to ESLint automatically together with --max-warnings 0 option.

Related Issues

Main issue:

eslint/eslint#15010

Related issues:

eslint/eslint#9977

eslint/eslint#12206

eslint/eslint#12249

[eslint-github-bot]

Hi @domnantas!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

• The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
• There should be a space following the initial tag and colon, for example 'feat: Message'.
• The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[linux-foundation-easycla]

• ✅ login: domnantas / name: Domantas Petrauskas (8369e36)
• ❌ The email address for the commit (7401dff, 36722ac) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please submit a support request ticket.

[Kurt-von-Laven]

Thank you very much for writing this up!

[eslint-github-bot]

Hi @domnantas!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

• The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
• There should be a space following the initial tag and colon, for example 'feat: Message'.
• The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[eslint-github-bot]

Hi @domnantas!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

• The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
• There should be a space following the initial tag and colon, for example 'feat: Message'.
• The first letter of the tag should be in lowercase

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[nzakas]

Thanks for putting this together. It seems simple enough. Just left a few comments.

[domnantas]

Thanks for the comments! I'm working on a PR to the https://github.com/eslint/eslint repo, because it's difficult to discuss the implementation details without having a clear picture of how this flag might work and relate to other pieces of the codebase

[nzakas]

A note for the team: if we move forward with this, it will be one more thing we will need to reconcile with FlatESLint going forward. I’m a little uneasy with continuing to add features into ESLint before FlatESLint is finished, so we may want to talk timelines here.

[domnantas]

I have created an example PR and updated the RFC accordingly. Making the default value of warnIgnored consistent is a breaking change. We could keep default overrides and avoid the breaking change.

I'm not exactly sure what's the scope of FlatESLint and how it is related. As you can see in the PR, there are very little changes in terms of functionality and I'm willing to help with integration of this RFC with FlatESLint. If you do decide to put this RFC on hold for the time being, I respect that decision too.

[nzakas]

Thanks! This update LGTM. Would like feedback from @mdjermanovic before moving to final commenting.

[nzakas]

My vote is no, let's keep it as a CLI only option.

[mdjermanovic]

Agreed, this doesn't seem like an option that should be available in eslint config files.

[mdjermanovic]

• Default values are not consistent. FlatESLint lintFiles gets warnIgnored = true, because that's how old ESLint constructor worked by default. lintText has warnIgnored = false by default, but cli.js overrides it to true when CLI is used with stdin.
• The RFC suggests to not modify lintText to maintain backwards compatibility, therefore --no-warn-ignored will not work when stdin is used.

I think we were in favor of consistency over maintaining backwards compatibility. When warnIgnored isn't passed to lintText, lintText should use constructor's warnIgnored, as in the previous version of this RFC.

[domnantas]

I think we were in favor of consistency over maintaining backwards compatibility. When warnIgnored isn't passed to lintText, lintText should use constructor's warnIgnored, as in the previous version of this RFC.

@mdjermanovic That was the initial idea, which I still agree with. Since releasing the breaking change is taking a really long time (1.5 years already), and people are continuously requesting this feature, I think it makes sense to release this feature as part of FlatESLint, just like @nzakas suggested. Minor downside - the flag will only be available if you're using FlatESLint.

Consistency and backporting to non-FlatESLint can be addressed as a separate issue/RFC

[nzakas]

I think we were in favor of consistency over maintaining backwards compatibility. When warnIgnored isn't passed to lintText, lintText should use constructor's warnIgnored, as in the previous version of this RFC.

Since you both are in favor of this, that's what we should do. No need to continue discussion there. :)

Since releasing the breaking change is taking a really long time (1.5 years already)

I'm very sorry about that. We are incredibly shorthanded and considering breaking changes takes a lot of extra effort. We really appreciate you sticking with us! (I myself had an RFC go over two years, so I understand the struggle.)

Minor downside - the flag will only be available if you're using FlatESLint.

That's fine. In the next major release FlatESLint will be the default.

[mdjermanovic]

I think there's confusion about the timing. My understanding is that since the FlatESLint class is still exported as an experimental API, we can release a change that is breaking for API users of the FlatESLint class in a minor 8.x version of ESLint. @nzakas is that correct?

[nzakas]

Yes, that's correct. Because FlatESLint is not yet the default, we can still making breaking changes even before a major release.

Ideally we'd do a major release this summer as we wrap up all the flat config work.

[domnantas]

If the major release is planned for this summer, could the breaking change (consistent warnIgnored: true default everywhere) be released together in it as well? In that case, I can update the RFC to reflect that change

[mdjermanovic]

If the major release is planned for this summer, could the breaking change (consistent warnIgnored: true default everywhere) be released together in it as well? In that case, I can update the RFC to reflect that change

We plan to implement this change only for flat config.

We can release this change in 8.x, that is, as soon as it is implemented. Even though it's a breaking change for FlatESLint class API users, it doesn't constitute a breaking change for eslint v8.

So basically this RFC should be reverted to the initial idea (warnIgnored of lintText() should default to constructor's warnIgnored) and just updated to target flat config (flat config CLI options, FlatESLint class, etc.).

[domnantas]

So basically this RFC should be reverted to the initial idea (warnIgnored of lintText() should default to constructor's warnIgnored) and just updated to target flat config (flat config CLI options, FlatESLint class, etc.).

Ah, I think I now understand what you're suggesting. Since we can do breaking changes when targeting flat config, we can do the consistency fix for the FlatESLint, but leave the warnIgnored as is for non-FlatESLint. Will update RFC accordingly

[nzakas]

@domnantas exactly! Once this update is made, I think we can formally approve this and move forward. We really appreciate your patience.

[domnantas]

Apologies for the inactivity. I am traveling with a sparse internet connection. I will be away until June. If anyone would like to take over updating the RFC before I return, feel free to do so.

[linux-foundation-easycla]

• ✅ login: domnantas / name: Domantas Petrauskas (8369e36, c06cf5c)
• ❌ The email address for the commit (7401dff, 36722ac) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please submit a support request ticket.

[linux-foundation-easycla]

• ✅ login: domnantas / name: Domantas Petrauskas (8369e36, c06cf5c, 4314e53)
• ❌ The email address for the commit (7401dff, 36722ac) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please submit a support request ticket.

[domnantas]

@nzakas @mdjermanovic Updated the RFC. Non-flat ESLint should not be affected, while FlatESLint lintFiles and lintText will behave consistently with warnIgnored: true as the default.

Had to force-push, because the first 2 commits were signed using my old email, which is no longer associated with my Github account, and that caused the CLA error.

[nzakas]

LGTM. Thanks!

Moving to Final Commenting as this was the last update requested.

[nzakas]

Final commenting period has been completed so this is approved! 🎉

Changes have been made.