Add default for the disable option

[DanielNoord]

• Add yourself to CONTRIBUTORS if you are a new contributor.
• Write a good description on what the PR does.

Type of Changes

	Type
✓	🔨 Refactoring

Description

Closes #3512.

Finally.

[coveralls]

Pull Request Test Coverage Report for Build 2601467310

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 95.358%

---

Totals
Change from base Build 2601041076:	0.0%
Covered Lines:	16702
Relevant Lines:	17515

---

💛 - Coveralls

[Pierre-Sassoulas]

The problem with this approach is that we're breaking some assumption about the way the disable work. I.e. when disabling one message the user would be in fact enabling all the messages we disable by default. I think the disabling need to take place in the checker code maybe the message definition directly; maybe a configuration in the spirit of templating the configuration (i.e. using google pylint template, or netflix pylint template...).

[DanielNoord]

The problem with this approach is that we're breaking some assumption about the way the disable work. I.e. when disabling one message the user would be in fact enabling all the messages we disable by default.

That's basically what #1353 is referring to, but we just closed that 😄
I understand that that is an issue, but as I said there: that should be solved by better documentation. Fixing this by appending disables to other disables creates numerous edge cases for the benefit of not having an user google for 5 minutes to see that they should just append the disable they want to the current list.
For most current users this won't be an issue anyway as they are likely disabling stuff already.

[github-actions]

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

This comment was generated for commit 33ceaba

[Pierre-Sassoulas]

I did not like #1353 because it would make this accessible to the user. I don't think our decision concerning what is a default check or not should be modifiable by the user. It's just something that is internal to pylint. The disable/enable option that the user have come on top of it and are sufficient imo. Although being able to handle a pylint template internally instead of having to define an additonal option in the message definition dict would be a better design and would permit to handle a template externally too (i.e. users would be able to distribute what they think should be pylint configuration without copy pasting 500 lines of toml).

[DanielNoord]

I did not like #1353 because it would make this accessible to the user. I don't think our decision concerning what is a default check or not should be modifiable by the user. It's just something that is internal to pylint. The disable/enable option that the user have come on top of it and are sufficient imo.

If we handle this value like that it would deviate from all other options. For example, for ignored-classes we also don't have a default list where users can append or subtract from. The default values are just that: suggested default values.
If you overwrite an option with your own value you can expect to no longer inherit from the default value that was previously being used. If you write ignored-classes= you'll lose:
https://github.com/PyCQA/pylint/blob/01ea163c48d759f86d75dfb47e00796b7409abe1/pylint/checkers/typecheck.py#L888-L892

I can understand why this feels not so user-friendly, but singling out disable to be the only option where the default value is not an overwritable default but an appendable default only further complicates the pylint configuration for both us and the users.

[jacobtylerwalls]

The problem with this approach is that we're breaking some assumption about the way the disable work. I.e. when disabling one message the user would be in fact enabling all the messages we disable by default.

I strongly agree and for that reason am -1 on the current proposal (sorry!)

If we handle this value like that it would deviate from all other options.

Sure, but if we find another implementation, then we're not exposing it to the user at all, and then there's no deviation to bother a user with.

[DanielNoord]

The problem with this approach is that we're breaking some assumption about the way the disable work. I.e. when disabling one message the user would be in fact enabling all the messages we disable by default.

I strongly agree and for that reason am -1 on the current proposal (sorry!)

I don't really understand how we're "breaking an assumption" here. Why is the assumption not: if you overwrite a default value you lose the default value? Imo, that's a much stronger assumption that's already being used in a number of other places.

Sure, but if we find another implementation, then we're not exposing it to the user at all, and then there's no deviation to bother a user with.

We already have that implementation: we can simply move the messages we don't want to be turned on by default to the extensions.

I thought the issue was about finding a sane default for disable which could serve as a suggestion to users. If the issue is about disabling messages because we don't think they should be in the "pylint stdlib" we can move them to extensions like we did with no-self-use.

[jacobtylerwalls]

I don't really understand how we're "breaking an assumption" here. Why is the assumption not: if you overwrite a default value you lose the default value? Imo, that's a much stronger assumption that's already being used in a number of other places.

There's no problem with that if you take for granted --disable having a default; the problem IMO is asking users to think of --disable as having a default value in the first place. That's profoundly weird. I don't think sending them to stackoverflow to learn that is a good usability paradigm.

I thought the issue was about finding a sane default for disable which could serve as a suggestion to users. If the issue is about disabling messages because we don't think they should be in the "pylint stdlib" we can move them to extensions like we did with no-self-use.

Yep, I would be much more in favor of that solution. And it matches the current documentation.

It might be laborious to extricate some of the messages from their current checkers, but that should be balanced I suppose against maintaining a more complicated solution in the checker/disabling implementation, which is what I'm hearing you express hesitation about (I agree!)

[DanielNoord]

Yep, I would be much more in favor of that solution. And it matches the current documentation.

Let's do that then 👍

[jacobtylerwalls]

I thought the issue was about finding a sane default for disable which could serve as a suggestion to users.

That's fine, if it's opt-in. This is kind of like Pierre's idea:

maybe a configuration in the spirit of templating the configuration (i.e. using google pylint template, or netflix pylint template...).

[jacobtylerwalls]

(I'll move this discussion to the issue.)