Fix a crash on psycopg2 for elif used

[Pierre-Sassoulas]

Type of Changes

	Type
✓	🐛 Bug fix
✓	🔨 Refactoring

Description

https://github.com/psycopg/psycopg/blob/master/psycopg/psycopg/_queries.py#L281

Exception on node <If l.281 at 0x7f2f9141a550>

pylint crashed with a IndexError and with the following stacktrace:

Traceback (most recent call last):
  File "/home/pierre/pylint/pylint/lint/pylinter.py", line 1035, in _check_files
    self._check_file(get_ast, check_astroid_module, file)
  File "/home/pierre/pylint/pylint/lint/pylinter.py", line 1070, in _check_file
    check_astroid_module(ast_node)
  File "/home/pierre/pylint/pylint/lint/pylinter.py", line 1204, in check_astroid_module
    retval = self._check_astroid_module(
  File "/home/pierre/pylint/pylint/lint/pylinter.py", line 1251, in _check_astroid_module
    walker.walk(node)
  File "/home/pierre/pylint/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  File "/home/pierre/pylint/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  File "/home/pierre/pylint/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  [Previous line repeated 1 more time]
  File "/home/pierre/pylint/pylint/utils/ast_walker.py", line 72, in walk
    callback(astroid)
  File "/home/pierre/pylint/pylint/extensions/check_elif.py", line 69, in visit_if
    if not self._elifs[self._if_counter]:
IndexError: list index out of range

The hot fix would be:

--- a/pylint/extensions/check_elif.py
+++ b/pylint/extensions/check_elif.py
@@ -66,7 +66,7 @@ class ElseifUsedChecker(BaseTokenChecker):
             orelse = node.parent.orelse
             # current if node must directly follow an "else"
             if orelse and orelse == [node]:
-                if not self._elifs[self._if_counter]:
+                if self._if_counter < len(self._elifs) and not self._elifs[self._if_counter]:
                     self.add_message("else-if-used", node=node)
         self._if_counter += 1

It seem to me that counting the if/elif properly would take some time. I created a minimal reproduction example:

def _if_in_fstring_comprehension():
    order = {}
    if not "false":
        raise TypeError(
            f" {', '.join(sorted(i for i in order or () if i not in vars))}"
        )
    elif "true":
        raise TypeError("d")

The issue come from the f string with a comprehension inside it that is not separated for the token processing.

Detected in #5173

[coveralls]

Pull Request Test Coverage Report for Build 1498507098

• 9 of 9 (100.0%) changed or added relevant lines in 2 files are covered.
• 64 unchanged lines in 10 files lost coverage.
• Overall coverage increased (+0.007%) to 93.46%

---

Files with Coverage Reduction	New Missed Lines	%
pylint/config/option.py	1	83.02%
pylint/checkers/utils.py	2	95.4%
pylint/extensions/overlapping_exceptions.py	2	84.09%
pylint/message/message.py	2	86.67%
pylint/testutils/reporter_for_tests.py	3	93.48%
pylint/checkers/exceptions.py	5	97.27%
pylint/testutils/lint_module_test.py	9	74.7%
pylint/checkers/variables.py	11	95.6%
pylint/checkers/similar.py	12	96.34%
pylint/lint/pylinter.py	17	94.23%

Totals
Change from base Build 1493950870:	0.007%
Covered Lines:	13919
Relevant Lines:	14893

---

💛 - Coveralls

[Pierre-Sassoulas]

New warnings are false positives changing to draft.

[Pierre-Sassoulas]

@DanielNoord @cdce8p this is ready for review.

[DanielNoord]

Looks good! Small changes here and there which we could do, I'll leave that decision to you!

[DanielNoord]

😄

[Pierre-Sassoulas]

Not having an off by one error when we miscount an If increased the confidence 😄 But really the other option are:

Confidence = namedtuple("Confidence", ["name", "description"])
# Warning Certainties
HIGH = Confidence("HIGH", "No false positive possible.")
INFERENCE = Confidence("INFERENCE", "Warning based on inference result.")
INFERENCE_FAILURE = Confidence(
    "INFERENCE_FAILURE", "Warning based on inference with failures."
)
UNDEFINED = Confidence("UNDEFINED", "Warning without any associated confidence level.")

CONFIDENCE_LEVELS = [HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED]

There's no inference here, so it should probably by HIGH. But also, we should probably refactor this (?) It was not used before so we might as well make it make sense. I'm not confident enough to say there will never be any false positive just because there was no inference.

[DanielNoord]

Seems like we could use a NO_INFERENCE level. Maybe add the to the growing list of todo's for 2.13?

[Pierre-Sassoulas]

Updated #5317

[Pierre-Sassoulas]

I don't understand the 64 unchanged lines in 10 files lost coverage. from coverall. Maybe I need to rebase on main ?

[DanielNoord]

I don't understand the 64 unchanged lines in 10 files lost coverage. from coverall. Maybe I need to rebase on main ?

The report on coveralls is different, see:
https://coveralls.io/builds/44507140