Implement pylint's else-if-used rule (PLR5501)

[chanman3388]

Attempt to implement else-if-used
https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/else-if-used.html

Issue #970

[chanman3388]

Annoyingly, else-if-used is the name of the rule, but it's disallowed, suggestions? collapsible-else-if?

[charliermarsh]

Yeah collapsible-else-if seems reasonable to me.

[charliermarsh]

Can we not use the orelse passed into the function already?

[chanman3388]

Yes you're right, going to give that a go.

[charliermarsh]

Should this be orelse.len() == 1? I thought we'd be looking for orelse.len() == 1 && matches!(orelse[0].node, StmtKind::If { .. }), which typically indicates an elif (or an else: if: in this case, which we'd have to differentiate somehow).

[charliermarsh]

Ah, here stmt is the if in the else: if: .... I'd assumed stmt was the outer if in:

if ...:
  pass
else:
  if:
    ...

[charliermarsh]

Hmm, I think we do need to structure this such that it looks at this statement:

if ...:
  pass
else:
  if:
    ...

Rather than the nested if. Otherwise, how can you tell if you're in an else or not?

[chanman3388]

Yeah, this is more complicated than I thought, namely because you don't know where the else is.
Pylint does have separate elif tokens which makes it easier...the only way I can think of doing it might be a bit janky where you use the locator to see if 'elif' is in the line or something like that.

[chanman3388]

So this can be done via the text method...is that ok? It feels a bit...dirty...

[charliermarsh]

Yeah it's ok. The alternative would be to do it by lexing and looking at the token stream. But this is effectively equivalent. There's no way to detect this AFAIK from the AST alone.

[charliermarsh]

It looks like this is being flagged as an error in the fixture, when it should be considered ok.

[ngnpope]

Other suggestions for names: uncollapsed-else-if or unnecessary-nested-else-if? 🤔

[chanman3388]

Other suggestions for names: uncollapsed-else-if or unnecessary-nested-else-if? 🤔

I quite like the latter.

[charliermarsh]

Merged as-is since I'm totally fine with the name, but happy to tweak in a follow-up PR if you feel strongly.