C1901 (compare-to-empty-string) message is not explicit about the problem and the solution

[kfsone]

Bug description

Consider

   val_type, val_value = get_type(name)  # returns str, Any
   if val_type is Environment:
     if val_value is None or val_value is "":  # or ==

The last line produces a pylint error, with the common guidance (e.g https://vald-phoenix.github.io/pylint-errors/plerr/errors/compare-to-empty-string/C1901.html) suggesting that you should use:

    if val_value is None or not len(val_value):

The user used val_value is "" to explicitly say: is it a string, and is it empty?

To avoid doing this is or == comparison, the user would need to write:

    if val_value is None or (isinstance(val_value, str) and not len(val_value)):

example of following the guidance too literally:

    def fn(v):
      if len(v):
          return "empty"
      return "other"

    fn(None)    # OK: returns "other"
    fn("a")     # OK: returns "other"
    fn([None])  # OK: returns "other"
    fn("")      # OK: returns "empty"
    fn([])      # WRONG: returns "other"
    fn(0)       # ERROR: int has no len

Configuration

No response

Command used

pylint a.py

Pylint output

a.py.3.1: [C1901(compare-to-empty-string), a.py] Avoid comparisons to empty string [229,1]

Expected behavior

This should either be disabled by default or it should provide better guidance. Why is x is "" bad?

Pylint version

pylint 1.9.5,
astroid 1.6.6
Python 2.7.18 (v2.7.18:8d21aa21f2, Apr 20 2020, 13:25:05) [MSC v.1500 64 bit (AMD64)]

OS / Environment

No response

Additional dependencies

No response

[Pierre-Sassoulas]

Hi, thank you for opening the issue. We're not supporting python 2, anymore, does the problem still exists in latest versions ?

[Pierre-Sassoulas]

Btw I would use bool, not len personally here.

[DudeNr33]

Pylint encourages Truth Value Testing, which also applies to strings. That's why x is "" is considered "bad", as it is redundant - at least as long as you don't want to explicitly check the type.

Disabling it by default would lead to a lot of redundant and (arguably) "unpythonic" comparisons to go unnoticed, while this here seems like a rather special case.

[Pierre-Sassoulas]

The message is now (in 2.11) a.py:3:25: R0123: Comparison to literal (literal-comparison), which I think could be made more explicit. Maybe something like a.py:3:25: R0123: 'val_value is ""' could be replaced by 'val_value' (literal-comparison) (or bool(val_value) if it's not a context where we have an auto-cast to boolean ?)

[DudeNr33]

Maybe we should include the words "Truth Value Testing" as this is the official wording in the Python docs, and people who want to know more can therefore easily find the page linked above.

'if val_value is ""' could be replaced by 'if val_value' (Truth Value Testing) (literal-comparison)

[Pierre-Sassoulas]

Flake8 says : F632 use ==/!= to compare constant literals (str, bytes, int, float, tuple). we can also piggy back on the documentation for this flake8 check when pylint users search for the error messages.

[DudeNr33]

I think we might be discussing separate things here. 🤔
The original title and original problem description was about C1901 compare-to-empty-string.

The title is now changed to refer to R0123 literal-comparison (I also mixed those up in my last response).

I agree that flake8's help message for the R0123 equivalent is better and pylint could adopt it.
But that would not solve the original problem, or am I missing something?
C1901 is in the optional CompareToEmptyStringChecker, and if enabled fires alongside R0123 for this code.

[Pierre-Sassoulas]

I ran pylint 2.12.0-dev0 on:

val_type, val_value = get_type(name)  # returns str, Any
if val_type is Environment:
    if val_value is None or val_value is "":  # or ==
        pass  

And the closest I got to something unclear was R0123. So I'm assuming the original issue with C1901 does not exists in the latest pylint and we're not supporting pylint 1.9 for python 2.7 anymore.

[DudeNr33]

It exists in pylint.extensions.emptystring.CompareToEmptyStringChecker and is an optional checker. If enabled through --load-plugins=pylint.extensions.emptystring the message C1901 fires.