From e7053b7846d5f1fc0391e8ba75e92fb3cc0ef0ed Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 00:26:38 +0200 Subject: [PATCH 01/16] Add an exception for `IndexError` inside `uninferable_final_decorators` method. --- ChangeLog | 2 ++ doc/whatsnew/2.14.rst | 2 ++ pylint/checkers/utils.py | 2 +- .../regression_6531_crash_index_error.py | 28 +++++++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/functional/r/regression/regression_6531_crash_index_error.py diff --git a/ChangeLog b/ChangeLog index aec24186b6..c5983fd3a3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,8 @@ Release date: TBA Closes #4301 +* Fix ``IndexError`` crash in ``uninferable_final_decorators`` method. + * By default the similarity checker will now ignore imports and ignore function signatures when computing duplication. If you want to keep the previous behaviour set ``ignore-imports`` and ``ignore-signatures`` to ``False``. diff --git a/doc/whatsnew/2.14.rst b/doc/whatsnew/2.14.rst index d59c4792ec..cfb4b57072 100644 --- a/doc/whatsnew/2.14.rst +++ b/doc/whatsnew/2.14.rst @@ -305,6 +305,8 @@ Other Changes * By default the similarity checker will now ignore imports and ignore function signatures when computing duplication. If you want to keep the previous behaviour set ``ignore-imports`` and ``ignore-signatures`` to ``False``. +* Fix ``IndexError`` crash in ``uninferable_final_decorators`` method. + * Pylint now expands the user path (i.e. ``~`` to ``home/yusef/``) and expands environment variables (i.e. ``home/$USER/$project`` to ``home/yusef/pylint`` for ``USER=yusef`` and ``project=pylint``) for pyreverse's ``output-directory``, ``import-graph``, ``ext-import-graph``, ``int-import-graph`` options, and the spell checker's ``spelling-private-dict-file`` diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 5a48ecf0a3..ecb7b68252 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -850,7 +850,7 @@ def uninferable_final_decorators( if isinstance(decorator, nodes.Attribute): try: import_node = decorator.expr.lookup(decorator.expr.name)[1][0] - except AttributeError: + except (AttributeError, IndexError): continue elif isinstance(decorator, nodes.Name): lookup_values = decorator.lookup(decorator.name) diff --git a/tests/functional/r/regression/regression_6531_crash_index_error.py b/tests/functional/r/regression/regression_6531_crash_index_error.py new file mode 100644 index 0000000000..9a2de9cdb4 --- /dev/null +++ b/tests/functional/r/regression/regression_6531_crash_index_error.py @@ -0,0 +1,28 @@ +# pylint: disable=missing-docstring, redefined-outer-name + +import pytest + + +class Wallet: + def __init__(self): + self.balance = 0 + + def add_cash(self, earned): + self.balance += earned + + def spend_cash(self, spent): + self.balance -= spent + +@pytest.fixture +def my_wallet(): + '''Returns a Wallet instance with a zero balance''' + return Wallet() + +@pytest.mark.parametrize("earned,spent,expected", [ + (30, 10, 20), + (20, 2, 18), +]) +def test_transactions(my_wallet, earned, spent, expected): + my_wallet.add_cash(earned) + my_wallet.spend_cash(spent) + assert my_wallet.balance == expected From 714c1d22b463cc86c071c6f7d89a0430e72ccc2b Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 7 May 2022 08:36:53 +0200 Subject: [PATCH 02/16] Apply suggestions from code review --- ChangeLog | 2 ++ doc/whatsnew/2.14.rst | 2 ++ 2 files changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index c5983fd3a3..8befb57c73 100644 --- a/ChangeLog +++ b/ChangeLog @@ -27,6 +27,8 @@ Release date: TBA * Fix ``IndexError`` crash in ``uninferable_final_decorators`` method. + Relates to #6531 + * By default the similarity checker will now ignore imports and ignore function signatures when computing duplication. If you want to keep the previous behaviour set ``ignore-imports`` and ``ignore-signatures`` to ``False``. diff --git a/doc/whatsnew/2.14.rst b/doc/whatsnew/2.14.rst index cfb4b57072..52a72b09c5 100644 --- a/doc/whatsnew/2.14.rst +++ b/doc/whatsnew/2.14.rst @@ -307,6 +307,8 @@ Other Changes * Fix ``IndexError`` crash in ``uninferable_final_decorators`` method. + Relates to #6531 + * Pylint now expands the user path (i.e. ``~`` to ``home/yusef/``) and expands environment variables (i.e. ``home/$USER/$project`` to ``home/yusef/pylint`` for ``USER=yusef`` and ``project=pylint``) for pyreverse's ``output-directory``, ``import-graph``, ``ext-import-graph``, ``int-import-graph`` options, and the spell checker's ``spelling-private-dict-file`` From 835707032d6706c99999a94349078e85fc8ec827 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 7 May 2022 08:43:18 +0200 Subject: [PATCH 03/16] Move changelog to 2.13.9 --- ChangeLog | 7 +++---- doc/whatsnew/2.13.rst | 4 ++++ doc/whatsnew/2.14.rst | 4 ---- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8befb57c73..56222daae5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,10 +25,6 @@ Release date: TBA Closes #4301 -* Fix ``IndexError`` crash in ``uninferable_final_decorators`` method. - - Relates to #6531 - * By default the similarity checker will now ignore imports and ignore function signatures when computing duplication. If you want to keep the previous behaviour set ``ignore-imports`` and ``ignore-signatures`` to ``False``. @@ -311,6 +307,9 @@ What's New in Pylint 2.13.9? ============================ Release date: TBA +* Fix ``IndexError`` crash in ``uninferable_final_decorators`` method. + + Relates to #6531 What's New in Pylint 2.13.8? diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst index 895d5d3694..b7a3eeff49 100644 --- a/doc/whatsnew/2.13.rst +++ b/doc/whatsnew/2.13.rst @@ -639,3 +639,7 @@ Other Changes ``open`` Closes #6414 + +* Fix ``IndexError`` crash in ``uninferable_final_decorators`` method. + + Relates to #6531 diff --git a/doc/whatsnew/2.14.rst b/doc/whatsnew/2.14.rst index 52a72b09c5..d59c4792ec 100644 --- a/doc/whatsnew/2.14.rst +++ b/doc/whatsnew/2.14.rst @@ -305,10 +305,6 @@ Other Changes * By default the similarity checker will now ignore imports and ignore function signatures when computing duplication. If you want to keep the previous behaviour set ``ignore-imports`` and ``ignore-signatures`` to ``False``. -* Fix ``IndexError`` crash in ``uninferable_final_decorators`` method. - - Relates to #6531 - * Pylint now expands the user path (i.e. ``~`` to ``home/yusef/``) and expands environment variables (i.e. ``home/$USER/$project`` to ``home/yusef/pylint`` for ``USER=yusef`` and ``project=pylint``) for pyreverse's ``output-directory``, ``import-graph``, ``ext-import-graph``, ``int-import-graph`` options, and the spell checker's ``spelling-private-dict-file`` From 38cbeebdca1242b8a539cc85239808d90b12a571 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 10:19:15 +0200 Subject: [PATCH 04/16] Simplify the logic - Handle `IndexError` & `AttributeError` situations. --- pylint/checkers/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index ecb7b68252..f7a2c0ed3b 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -848,10 +848,10 @@ def uninferable_final_decorators( decorators = [] for decorator in getattr(node, "nodes", []): if isinstance(decorator, nodes.Attribute): - try: - import_node = decorator.expr.lookup(decorator.expr.name)[1][0] - except (AttributeError, IndexError): + _, import_nodes = decorator.expr.lookup(decorator.expr.name) + if not import_nodes: continue + import_node = import_nodes[0] elif isinstance(decorator, nodes.Name): lookup_values = decorator.lookup(decorator.name) if lookup_values[1]: From 071251d790be972f09f6ea98193c954c8334184f Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 10:49:58 +0200 Subject: [PATCH 05/16] Re-add the try-except to catch an `AttributeError`. --- pylint/checkers/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index f7a2c0ed3b..8b12d6d8d2 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -848,7 +848,10 @@ def uninferable_final_decorators( decorators = [] for decorator in getattr(node, "nodes", []): if isinstance(decorator, nodes.Attribute): - _, import_nodes = decorator.expr.lookup(decorator.expr.name) + try: + _, import_nodes = decorator.expr.lookup(decorator.expr.name) + except AttributeError: + continue if not import_nodes: continue import_node = import_nodes[0] From 64a5b75de5185d5de66123865cdf309bda2643e8 Mon Sep 17 00:00:00 2001 From: Mark Byrne <31762852+mbyrnepr2@users.noreply.github.com> Date: Sat, 7 May 2022 11:49:51 +0200 Subject: [PATCH 06/16] Update tests/functional/r/regression/regression_6531_crash_index_error.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- .../r/regression/regression_6531_crash_index_error.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/functional/r/regression/regression_6531_crash_index_error.py b/tests/functional/r/regression/regression_6531_crash_index_error.py index 9a2de9cdb4..6cdc96617b 100644 --- a/tests/functional/r/regression/regression_6531_crash_index_error.py +++ b/tests/functional/r/regression/regression_6531_crash_index_error.py @@ -1,3 +1,5 @@ +"""Regression test for https://github.com/PyCQA/pylint/issues/6531.""" + # pylint: disable=missing-docstring, redefined-outer-name import pytest From 42bcfc1143870bec315763231bee176fab3c1c1f Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 11:55:32 +0200 Subject: [PATCH 07/16] Remove try-except. Replace it with a `hasattr` check. --- pylint/checkers/utils.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 8b12d6d8d2..a81be2952b 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -847,11 +847,10 @@ def uninferable_final_decorators( """ decorators = [] for decorator in getattr(node, "nodes", []): - if isinstance(decorator, nodes.Attribute): - try: - _, import_nodes = decorator.expr.lookup(decorator.expr.name) - except AttributeError: - continue + if isinstance(decorator, nodes.Attribute) and hasattr(decorator.expr, "lookup"): + _, import_nodes = decorator.expr.lookup(decorator.expr.name) + # The `final` decorator is expected to be found in the + # import_nodes. In case it is not, continue. if not import_nodes: continue import_node = import_nodes[0] From d6638796eff2ee5e7572db266b5e38c33145a750 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 16:41:11 +0200 Subject: [PATCH 08/16] Narrow the scope for error-handling --- pylint/checkers/utils.py | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index a81be2952b..d22c84c000 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -15,7 +15,7 @@ from collections.abc import Iterable from functools import lru_cache, partial from re import Match -from typing import TYPE_CHECKING, Callable, TypeVar +from typing import Tuple, TYPE_CHECKING, Callable, TypeVar import _string import astroid.objects @@ -847,30 +847,34 @@ def uninferable_final_decorators( """ decorators = [] for decorator in getattr(node, "nodes", []): - if isinstance(decorator, nodes.Attribute) and hasattr(decorator.expr, "lookup"): - _, import_nodes = decorator.expr.lookup(decorator.expr.name) - # The `final` decorator is expected to be found in the - # import_nodes. In case it is not, continue. - if not import_nodes: - continue - import_node = import_nodes[0] + import_nodes: Tuple[nodes.Import | nodes.ImportFrom] = None + + # Get the `Import` node. The decorator is of the form: @module.name + if isinstance(decorator, nodes.Attribute): + inferred = safe_infer(decorator.expr) + if isinstance(inferred, nodes.Module): + _, import_nodes = decorator.expr.lookup(decorator.expr.name) + + # Get the `ImportFrom` node. The decorator is of the form: @name elif isinstance(decorator, nodes.Name): - lookup_values = decorator.lookup(decorator.name) - if lookup_values[1]: - import_node = lookup_values[1][0] - else: - continue # pragma: no cover # Covered on Python < 3.8 - else: + _, import_nodes = decorator.lookup(decorator.name) + + # The `final` decorator is expected to be found in the + # import_nodes. Continue if we don't find any `Import` or `ImportFrom` + # nodes for this decorator. + if not import_nodes: continue + import_node = import_nodes[0] if not isinstance(import_node, (astroid.Import, astroid.ImportFrom)): continue import_names = dict(import_node.names) - # from typing import final + # Check if the import if of the form: `from typing import final` is_from_import = ("final" in import_names) and import_node.modname == "typing" - # import typing + + # Check if the import if of the form: `import typing` is_import = ("typing" in import_names) and getattr( decorator, "attrname", None ) == "final" From 62539364ee3000d8e0d143a17464e5691e2dc946 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 7 May 2022 14:42:19 +0000 Subject: [PATCH 09/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index d22c84c000..2284dacc3d 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -15,7 +15,7 @@ from collections.abc import Iterable from functools import lru_cache, partial from re import Match -from typing import Tuple, TYPE_CHECKING, Callable, TypeVar +from typing import TYPE_CHECKING, Callable, Tuple, TypeVar import _string import astroid.objects @@ -847,7 +847,7 @@ def uninferable_final_decorators( """ decorators = [] for decorator in getattr(node, "nodes", []): - import_nodes: Tuple[nodes.Import | nodes.ImportFrom] = None + import_nodes: tuple[nodes.Import | nodes.ImportFrom] = None # Get the `Import` node. The decorator is of the form: @module.name if isinstance(decorator, nodes.Attribute): From 7dc1beb8bc12fb08f92c84dca6b8ae8d02896a27 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 16:43:31 +0200 Subject: [PATCH 10/16] rm Tuple --- pylint/checkers/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 2284dacc3d..38291d43ba 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -15,7 +15,7 @@ from collections.abc import Iterable from functools import lru_cache, partial from re import Match -from typing import TYPE_CHECKING, Callable, Tuple, TypeVar +from typing import TYPE_CHECKING, Callable, TypeVar import _string import astroid.objects From 1e35c35f85ce752f46f4639992df8d34a6327063 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 16:47:01 +0200 Subject: [PATCH 11/16] typo --- pylint/checkers/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 38291d43ba..3c129ed8f9 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -871,10 +871,10 @@ def uninferable_final_decorators( import_names = dict(import_node.names) - # Check if the import if of the form: `from typing import final` + # Check if the import is of the form: `from typing import final` is_from_import = ("final" in import_names) and import_node.modname == "typing" - # Check if the import if of the form: `import typing` + # Check if the import is of the form: `import typing` is_import = ("typing" in import_names) and getattr( decorator, "attrname", None ) == "final" From 547ab5e6df9058708806e3b120b8e37d38f7015f Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 16:48:03 +0200 Subject: [PATCH 12/16] Fix typing --- pylint/checkers/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 3c129ed8f9..d062660138 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -15,7 +15,7 @@ from collections.abc import Iterable from functools import lru_cache, partial from re import Match -from typing import TYPE_CHECKING, Callable, TypeVar +from typing import TYPE_CHECKING, Callable, Tuple, TypeVar import _string import astroid.objects @@ -847,7 +847,7 @@ def uninferable_final_decorators( """ decorators = [] for decorator in getattr(node, "nodes", []): - import_nodes: tuple[nodes.Import | nodes.ImportFrom] = None + import_nodes: Tuple[Union[nodes.Import, nodes.ImportFrom]] = None # Get the `Import` node. The decorator is of the form: @module.name if isinstance(decorator, nodes.Attribute): From e2350df95447c1be51a0ed9484c35f1e9bd497a8 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 7 May 2022 14:49:03 +0000 Subject: [PATCH 13/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index d062660138..1530230da8 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -847,7 +847,7 @@ def uninferable_final_decorators( """ decorators = [] for decorator in getattr(node, "nodes", []): - import_nodes: Tuple[Union[nodes.Import, nodes.ImportFrom]] = None + import_nodes: tuple[Union[nodes.Import, nodes.ImportFrom]] = None # Get the `Import` node. The decorator is of the form: @module.name if isinstance(decorator, nodes.Attribute): From 8aedda241582f50af6775b4ed37a87a136d2247e Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 16:55:27 +0200 Subject: [PATCH 14/16] import `typing.Union` --- pylint/checkers/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 1530230da8..cd1e22fd2f 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -15,7 +15,7 @@ from collections.abc import Iterable from functools import lru_cache, partial from re import Match -from typing import TYPE_CHECKING, Callable, Tuple, TypeVar +from typing import TYPE_CHECKING, Callable, Tuple, TypeVar, Union import _string import astroid.objects From 83f696f5384be48a1d2f16bff4263d0067baba35 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 17:10:18 +0200 Subject: [PATCH 15/16] Fix the typing syntax. --- pylint/checkers/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index cd1e22fd2f..931b592208 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -15,7 +15,7 @@ from collections.abc import Iterable from functools import lru_cache, partial from re import Match -from typing import TYPE_CHECKING, Callable, Tuple, TypeVar, Union +from typing import TYPE_CHECKING, Callable, TypeVar import _string import astroid.objects @@ -847,7 +847,7 @@ def uninferable_final_decorators( """ decorators = [] for decorator in getattr(node, "nodes", []): - import_nodes: tuple[Union[nodes.Import, nodes.ImportFrom]] = None + import_nodes: tuple[nodes.Import | nodes.ImportFrom] | None = None # Get the `Import` node. The decorator is of the form: @module.name if isinstance(decorator, nodes.Attribute): From 51a3df854c21e0ae46fccb0fe800fbef5cc27341 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 7 May 2022 17:26:22 +0200 Subject: [PATCH 16/16] Only consider lookup when the module is named `typing` --- pylint/checkers/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 931b592208..75921a5b59 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -852,7 +852,7 @@ def uninferable_final_decorators( # Get the `Import` node. The decorator is of the form: @module.name if isinstance(decorator, nodes.Attribute): inferred = safe_infer(decorator.expr) - if isinstance(inferred, nodes.Module): + if isinstance(inferred, nodes.Module) and inferred.qname() == "typing": _, import_nodes = decorator.expr.lookup(decorator.expr.name) # Get the `ImportFrom` node. The decorator is of the form: @name