Skip to content

Commit

Permalink
Fix a crash on psycopg2 for elif used (#5369)
Browse files Browse the repository at this point in the history
* Fix a crash in the ``check_elif`` extensions where an undetected if in a comprehension
  with an if statement within a f-string resulted in an out of range error. The checker no
  longer relies on counting if statements anymore and uses known if statements locations instead.
  It should not crash on badly parsed if statements anymore. specify the confidence of the message

* Remove disable for else-if-used in pylint/checkers/classes.py

Co-authored-by: Daniël van Noord <[email protected]>
  • Loading branch information
Pierre-Sassoulas and DanielNoord authored Nov 24, 2021
1 parent e3a1e35 commit e8fa469
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 41 deletions.
5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ Release date: TBA

* Fix ``install graphiz`` message which isn't needed for puml output format.

* Fix a crash in the ``check_elif`` extensions where an undetected if in a comprehension
with an if statement within a f-string resulted in an out of range error. The checker no
longer relies on counting if statements anymore and uses known if statements locations instead.
It should not crash on badly parsed if statements anymore.

* Fix ``simplify-boolean-expression`` when condition can be inferred as False.

Closes #5200
Expand Down
26 changes: 12 additions & 14 deletions pylint/checkers/classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1848,20 +1848,18 @@ def _check_first_arg_for_type(self, node, metaclass=0):
"bad-mcs-method-argument",
node.name,
)
# regular class
else: # pylint: disable=else-if-used
# class method
if node.type == "classmethod" or node.name == "__class_getitem__":
self._check_first_arg_config(
first,
self.config.valid_classmethod_first_arg,
node,
"bad-classmethod-argument",
node.name,
)
# regular method without self as argument
elif first != "self":
self.add_message("no-self-argument", node=node)
# regular class with class method
elif node.type == "classmethod" or node.name == "__class_getitem__":
self._check_first_arg_config(
first,
self.config.valid_classmethod_first_arg,
node,
"bad-classmethod-argument",
node.name,
)
# regular class with regular method without self as argument
elif first != "self":
self.add_message("no-self-argument", node=node)

def _check_first_arg_config(self, first, config, node, message, method_name):
if first not in config:
Expand Down
38 changes: 14 additions & 24 deletions pylint/extensions/check_elif.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from pylint.checkers import BaseTokenChecker
from pylint.checkers.utils import check_messages
from pylint.interfaces import IAstroidChecker, ITokenChecker
from pylint.interfaces import HIGH, IAstroidChecker, ITokenChecker


class ElseifUsedChecker(BaseTokenChecker):
Expand All @@ -38,37 +38,27 @@ def __init__(self, linter=None):
self._init()

def _init(self):
self._elifs = []
self._if_counter = 0
self._elifs = {}

def process_tokens(self, tokens):
# Process tokens and look for 'if' or 'elif'
for _, token, _, _, _ in tokens:
if token == "elif":
self._elifs.append(True)
elif token == "if":
self._elifs.append(False)
"""Process tokens and look for 'if' or 'elif'"""
self._elifs = {
begin: token for _, token, begin, _, _ in tokens if token in {"elif", "if"}
}

def leave_module(self, _: nodes.Module) -> None:
self._init()

def visit_ifexp(self, node: nodes.IfExp) -> None:
if isinstance(node.parent, nodes.FormattedValue):
return
self._if_counter += 1

def visit_comprehension(self, node: nodes.Comprehension) -> None:
self._if_counter += len(node.ifs)

@check_messages("else-if-used")
def visit_if(self, node: nodes.If) -> None:
if isinstance(node.parent, nodes.If):
orelse = node.parent.orelse
# current if node must directly follow an "else"
if orelse and orelse == [node]:
if not self._elifs[self._if_counter]:
self.add_message("else-if-used", node=node)
self._if_counter += 1
"""Current if node must directly follow an 'else'"""
if (
isinstance(node.parent, nodes.If)
and node.parent.orelse == [node]
and (node.lineno, node.col_offset) in self._elifs
and self._elifs[(node.lineno, node.col_offset)] == "if"
):
self.add_message("else-if-used", node=node, confidence=HIGH)


def register(linter):
Expand Down
23 changes: 22 additions & 1 deletion tests/functional/ext/check_elif/check_elif.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
# pylint: disable=no-else-raise,unsupported-membership-test,using-constant-test

"""Checks use of "else if" triggers a refactor message"""
from typing import Union, Sequence, Any, Mapping


def my_function():
Expand All @@ -7,7 +10,7 @@ def my_function():
if myint > 5:
pass
else:
if myint <= 5: # [else-if-used]
if myint <= 5: # [else-if-used]
pass
else:
myint = 3
Expand All @@ -25,3 +28,21 @@ def my_function():
if myint:
pass
myint = 4


def _if_in_fstring_comprehension_with_elif(
params: Union[Sequence[Any], Mapping[str, Any]]
):
order = {}
if "z" not in "false":
raise TypeError(
f" {', '.join(sorted(i for i in order or () if i not in params))}"
)
elif "z" not in "true":
pass
else:
if "t" not in "false": # [else-if-used]
raise TypeError("d")
else:
if "y" in "life": # [else-if-used]
print("e")
6 changes: 4 additions & 2 deletions tests/functional/ext/check_elif/check_elif.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
else-if-used:10:8:my_function:"Consider using ""elif"" instead of ""else if""":HIGH
else-if-used:22:20:my_function:"Consider using ""elif"" instead of ""else if""":HIGH
else-if-used:13:8:my_function:"Consider using ""elif"" instead of ""else if""":HIGH
else-if-used:25:20:my_function:"Consider using ""elif"" instead of ""else if""":HIGH
else-if-used:44:8:_if_in_fstring_comprehension_with_elif:"Consider using ""elif"" instead of ""else if""":HIGH
else-if-used:47:12:_if_in_fstring_comprehension_with_elif:"Consider using ""elif"" instead of ""else if""":HIGH

0 comments on commit e8fa469

Please sign in to comment.