Treat --errors-only as a disable, not a paired enable/disable

[jacobtylerwalls]

Type of Changes

	Type
✓	🐛 Bug fix

Description

Closes #6811

Now, --errors-only is interpreted as a disable of non-error/fatal messages, not a paired disable of non-errors and enable of every error. Prevents canceling out prior disables. (In other words, it was acting as "enable all and only errors", not "disable non-errors").

[DanielNoord]

https://github.com/PyCQA/pylint/blob/003b472d1814948f41da07995f4387eb2c642742/pylint/lint/base_options.py#L532-L535

I'm not really sure if this language implies that we enable all other messages.

Either way, this is a backwards incompatible change. With all the issues people had with their configuration files in the 2.14 bump I'm not sure we should include additional changes in 2.15... Should this be postponed to 3.x?

[Pierre-Sassoulas]

Thank you !

[coveralls]

Pull Request Test Coverage Report for Build 2499995642

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 95.552%

---

Totals
Change from base Build 2493047509:	0.01%
Covered Lines:	16411
Relevant Lines:	17175

---

💛 - Coveralls

[Pierre-Sassoulas]

I see where Daniel is coming from, but I think it could be considered a fix. I don't have strong opinion about it though.

[jacobtylerwalls]

I'm not really sure if this language implies that we enable all other messages.

I think I'm not following you. As in, I'm not sure what that has to do with the bug report or the PR. So I must be missing something :-)

Either way, this is a backwards incompatible change. With all the issues people had with their configuration files in the 2.14 bump I'm not sure we should include additional changes in 2.15... Should this be postponed to 3.x?

It's worth getting this right, for sure, but every bug fix is backwards incompatible. I think this is a bug, not behavior people would reasonably depend on. It's the equivalent of treating --errors-only as --all-errors. We know all and only are different things!

[DanielNoord]

I think I'm not following you. As in, I'm not sure what that has to do with the bug report or the PR. So I must be missing something :-)

That's the description of this option. I'm wondering if that implies that we turn on all error level messages. If that's the case this is not a bug but a "feature" 😄

It's worth getting this right, for sure, but every bug fix is backwards incompatible. I think this is a bug, not behavior people would reasonably depend on. It's the equivalent of treating --errors-only as --all-errors. We know all and only are different things!

Agreed that this is a bug. I'm only worried about people depending on this. I can see (new) users try and do pylint --disable=all --errors-only based on a combination of StackOverflow answers they found. Currently that would work and it has for a long time (probably since introduction of the flag).

[jacobtylerwalls]

Gotcha. Sorry for sounding annoying. Was dashing off a quick reply before stepping out -- but I follow you now 👍🏻

So let's take backporting off the table.

I feel like the flag is pretty surprising/unusable as is, though. We probably don't get issues reported about it because folks decide immediately not to use it when they see it ignores all their disables. You can't use this flag with a pylintrc that disables any errors, right?

Here is the README:

Pylint isn't smarter than you: it may warn you about things that you have
conscientiously done or check for some things that you don't care about.
During adoption, especially in a legacy project where pylint was never enforced,
it's best to start with the ``--errors-only`` flag, then disable
convention and refactor message with ``--disable=C,R`` and progressively
re-evaluate and re-enable messages as your priorities evolve.

Imagine adopting pylint on a legacy project and starting with a pylintc you know and like from other projects that disables, I don't know, no-member or something. Now you can't follow our advice, because --errors-only, instead of quieting things, is going to make things louder. But I get that it's undefined right now.

For things that are undefined and feel like bugs, I feel like we don't have to wait for 3.X but I'm not a zealous advocate here, so I'm open.

[DanielNoord]

Gotcha. Sorry for sounding annoying. Was dashing off a quick reply before stepping out -- but I follow you now 👍🏻

No worries!

I feel like the flag is pretty surprising/unusable as is, though. We probably don't get issues reported about it because folks decide immediately not to use it when they see it ignores all their disables. You can't use this flag with a pylintrc that disables any errors, right?

You could by doing --error-mode --disable=MyError. The disable needs to be placed after the --error-mode flag (options are parsed consecutively)

For things that are undefined and feel like bugs, I feel like we don't have to wait for 3.X but I'm not a zealous advocate here, so I'm open.

No I agree with you. I just wanted to make sure we were all on the same page and conscious of any potential breakage. Even though I have become more aware of it I keep surprising myself how much we can break with each minor version 😄

So let's take backporting off the table.

If we consider this a bug let's do the backport. It would also help take the load of explaining this decision from the 2.15 release which will undoubtedly have its own backwards incompatible changes we need to explain 😄

[Pierre-Sassoulas]

Even though I have become more aware of it I keep surprising myself how much we can break with each minor version

https://xkcd.com/1172/ 😄

We might be better off actually releasing a major and officially break things. There's quite a lot of changes to apply at this point.

[DanielNoord]

Even though I have become more aware of it I keep surprising myself how much we can break with each minor version

https://xkcd.com/1172/ 😄

We might be better off actually releasing a major and officially break things. There's quite a lot of changes to apply at this point.

I'd hold off on releasing 3.x until typing is complete (and potentially mypy can pass on astroid). We might run into other issues that need deprecations.

[jacobtylerwalls]

I'd hold off on releasing 3.x until typing is complete (and potentially mypy can pass on astroid).

+1

If we consider this a bug let's do the backport.

Sure.

You could by doing --error-mode --disable=MyError. The disable needs to be placed after the --error-mode flag (options are parsed consecutively)

Right, okay, so for my scenario with an existing config file being brought to a new project, that means having to explicitly re-disable everything. I think we're on the same page, but just mentioning it here for "posterity" i.e. next month.

[Pierre-Sassoulas]

LGTM, are you ok to backport in 2.14.2 @DanielNoord ?

[DanielNoord]

LGTM, are you ok to backport in 2.14.2 @DanielNoord ?

Yes!

[Pierre-Sassoulas]

Rebased on main because I think the CI issue might come from #6945

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit d16f765