``singleton comparison`` - ==/is true message improvement

[orSolocate]

Current problem

I was wondering about the singleton-comparison checker I worked on a bit a while ago.
it suggests you to compare singleton using is instead of == which is fine, for example for none.
But why should we use it with True and False? since comparison returns a Boolean anyway.

Desired solution

For the following code:

if expr == True or if expr is True

The checker should suggest to modify it to if expr.

for false, both if expr == False and if expr is False should be modified to if not expr.

Additional context

There is some controversy about this matter as can be seen in this thread:
https://stackoverflow.com/questions/27276610/boolean-identity-true-vs-is-true

Since it assumes an object has its __bool__ implemented although I believe that this is most probably the case in any class, since it comes with a default method that returns True.
See this thread for some (maybe engineered) corner cases:
https://stackoverflow.com/questions/64913140/is-there-a-way-to-remove-bool-method-from-a-class

[jacobtylerwalls]

Do you think we should use inference? Emit a message for x is True just if we infer x only takes boolean values?

The "controversy" you point out makes me think this should be a distinct message.

Also, I probably wouldn't want this in the context of an assertion. assert x is True seems fine in the context of a regression test: if x is unexpectedly foo, that should fail. A response on the SO post suggests checking isinstance(x, bool) and x for this case, but although that's terrifically pythonic, it strikes me as overly precise to warrant a pylint message when assert x is True is practical.

Interestingly, isinstance() doesn't even help with the numpy-bool-is-not-a-bool problem:

>>> type(numpy.ndarray([]).all()).mro()
[<class 'numpy.bool_'>, <class 'numpy.generic'>, <class 'object'>]

[Pierre-Sassoulas]

The checker should suggest to modify it to if expr.

This would have disastrous consequences if followed when expr is not a bool. Some value can be falsey or truethey, list, tuple, int, string, numpy object, pandas dataframe etc.

As the stackoverflow answers say, it's possible to override __eq__ or __bool__ and the assumption we take would now be false. This is what happens in pandas/numpy and why we need to check the implementation for the use-implicit-booleaness... checks (when we warn about if len(x) == 0 !). But we must also remind ourselves too that a lot of python code have dynamic typing, what comes from a json have ambiguous type sometime... Using implicit booleaness all the time produce subtle bugs that are hard to diagnose : A team member once lost literal days of debugging because of an overzealous use of implicit booleaness -- because a float is falsey when it's equal to zero. See the caveat in the doc for use-implicit-booleaness-not... checks: https://pylint.readthedocs.io/en/latest/user_guide/messages/convention/use-implicit-booleaness-not-comparison-to-zero.html

[orSolocate]

Ok, so @Pierre-Sassoulas, viewing this thread again, I believe we decide not to pursue this issue, am I right?

[Pierre-Sassoulas]

Not necessarily, but we'd need to reuse the logic from the implicit-booleaness checks.