Pylint: no-else-return improperly triggered with elif statement

[kchen]

Steps to reproduce

Run pylint on the following:

"""no-else-return test module"""

def qqq():
    """no-else-return test function"""

    aaa = 5

    if aaa == 3:
        return 1
    elif aaa == 4:
        return 2

    return None

Current behavior

*** Module no_else_return
no_else_return.py:8:4: R1705: Unnecessary "else" after "return" (no-else-return)

Expected behavior

No issues returned.

pylint --version output

pylint 2.0.0.dev2
astroid 2.0.0.dev4
Python 3.7.0 (default, Jul 13 2018, 22:37:47)
[GCC 5.4.0 20160609]

[PCManticore]

So what this check does is that it verifies if you actually need to explicitly write elif or else when in a previous branch you had a return. So instead of elif aaa == 4, it expects you to write if aaa == 4:.

[kchen]

Ah, thanks for the clarification. I'm not sure why "if" is preferred here over "elif", but if that's pylint's preference, that seems fine. If the pylint behavior remains unchanged, can you clarify the explanation in the message? It's currently confusing since it mentions an "else" even though none exists.

[mbkupfer]

@PCManticore , sorry to tack on to this so late, but I'm also confused about the preference of if over elif in these situations. I'm sure there is a good reason, but I think an explanation of _"why"_ this is the case would help justify the switch and also improve my knowledge of python at a fundamental level.

[graingert]

@mbkupfer https://refactoring.guru/replace-nested-conditional-with-guard-clauses

[mbkupfer]

@graingert thanks for the reply. Still not sure how that explains the preference of if over multiple elif's.

[graingert]

@mbkupfer here's the python example https://refactoring.guru/replace-nested-conditional-with-guard-clauses#python

elif x: is just short for:

else:
    if x:

[nachtkinder]

There may be an argument to be made for preference of elif over another plain if when dealing with object properties:

Example 1 Regular cat:

if not cat.alive:
    return do_not_pet(cat)
elif not cat.dead:
    return pet(cat)

Example 2 Schroedinger's / Zombie cat?

if not cat.alive:
    return do_not_pet(cat)

if not cat.dead:
    return pet(cat)

In Example 1 the elif reinforces the idea that regular cats are expected to be either alive (not dead) or dead (not alive) and that the code was written with this understanding. Cats should be petted if they're not dead, not petted if they are and undead cats are not expected.

With Example 2 it is unclear whether the code was written to expect zombie cats and pet them regardless or this is an error.

IMHO Zombie cats should not be petted as you're likely to get bitten and acquire an appetite for brains! ;)

NB: The examples provided are tailored towards cats, zombies and humour.

Regards.
Nachtkinder

[graingert]

@nachtkinder In those cases, ternery is preferable:

return (
    pet(cat)
    if (cat.alive and not cat.dead)
    else do_not_pet(cat)
)

You could even name the condition:

cat_is_alive_and_non_zombie = cat.alive and not cat.dead

[mbkupfer]

@graingert , sorry for long delay I was on a business trip and just had a chance to review. Your explanation really cleared things up for me and it actually makes a lot more sense now!

[dlee1j1]

Reading through the article:
https://refactoring.guru/replace-nested-conditional-with-guard-clauses

The elif construct (or syntactic sugar if you will), actually more elegantly solves the issue that the article points out.

It is quite difficult to understand:

if aaa == 3:
    return 1
else:
     if aaa == 4:
          return 2
     else:
            if aaa == 5:
                  return 3
             else:
                   if aaa == 6:
                         return 4
 ....

However, once you lift the else with the if via the elif construct, it's easier to understand (it reads more like a switch):

if aaa == 3:
    return 1
elif aaa == 4:
    return 2
 elif aaa == 5:
     return 3
  elif aaa == 6:
     return 4
 ....

and more straightforward than a series of if's because there's not a hint that there is a logical grouping

[graingert]

@dlee1j1 that article and this lint rule recommends the following:

if aaa == 3:
    return 1
if aaa == 4:
    return 2
if aaa == 5:
    return 3
if aaa == 6:
    return 4