From 20f4cc556b78a332883ee45e2e8ce3ff1c32a73c Mon Sep 17 00:00:00 2001 From: Tushar Sadhwani Date: Thu, 23 Dec 2021 16:51:54 +0530 Subject: [PATCH 1/6] Fix dict comprehension false positive --- .../refactoring/refactoring_checker.py | 25 ++++++++++++++++--- .../consider_using_dict_comprehension.py | 4 +++ .../consider_using_dict_comprehension.txt | 1 + 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index acaf7191cd..ce8b8eda87 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -934,9 +934,28 @@ def _check_consider_using_comprehension_constructor(self, node): and node.args and isinstance(node.args[0], nodes.ListComp) ): - if node.func.name == "dict" and not isinstance( - node.args[0].elt, nodes.Call - ): + if node.func.name == "dict": + element = node.args[0].elt + if isinstance(element, nodes.Call): + return + + # If we have an `IfExp` here where both the key AND value + # are different, then don't raise the issue. + if ( + isinstance(element, nodes.IfExp) + and isinstance(element.body, (nodes.Tuple, nodes.List)) + and len(element.body.elts) == 2 + and isinstance(element.orelse, (nodes.Tuple, nodes.List)) + and len(element.orelse.elts) == 2 + ): + key1, value1 = element.body.elts + key2, value2 = element.orelse.elts + if ( + key1.as_string() != key2.as_string() + and value1.as_string != value2.as_string() + ): + return + message_name = "consider-using-dict-comprehension" self.add_message(message_name, node=node) elif node.func.name == "set": diff --git a/tests/functional/c/consider/consider_using_dict_comprehension.py b/tests/functional/c/consider/consider_using_dict_comprehension.py index c9d740e18c..c26a0d5b12 100644 --- a/tests/functional/c/consider/consider_using_dict_comprehension.py +++ b/tests/functional/c/consider/consider_using_dict_comprehension.py @@ -8,5 +8,9 @@ dict([(number, number*2) for number in numbers]) # [consider-using-dict-comprehension] +stuff = {1: 10, 2: -20} +dict([(k, v) if v > 0 else (k, 0) for k, v in stuff.items()]) # [consider-using-dict-comprehension] +dict([(k, v) if v > 0 else (k * 2, 0) for k, v in stuff.items()]) + # Cannot emit as this cannot be written as a comprehension dict([value.split("=") for value in ["a=b", "c=d"]]) diff --git a/tests/functional/c/consider/consider_using_dict_comprehension.txt b/tests/functional/c/consider/consider_using_dict_comprehension.txt index a241d19078..23c805c792 100644 --- a/tests/functional/c/consider/consider_using_dict_comprehension.txt +++ b/tests/functional/c/consider/consider_using_dict_comprehension.txt @@ -1 +1,2 @@ consider-using-dict-comprehension:9:0:9:48::Consider using a dictionary comprehension:UNDEFINED +consider-using-dict-comprehension:12:0:12:61::Consider using a dictionary comprehension:UNDEFINED From f741b8ce7180e1e31065a3f4e3bba85ba1963676 Mon Sep 17 00:00:00 2001 From: Tushar Sadhwani Date: Thu, 23 Dec 2021 17:07:37 +0530 Subject: [PATCH 2/6] Update changelog --- ChangeLog | 5 +++++ doc/whatsnew/2.13.rst | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/ChangeLog b/ChangeLog index 77e77dfe56..4d1609af3a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,6 +11,11 @@ Release date: TBA .. Put new features here and also in 'doc/whatsnew/2.13.rst' +* Fixed false positive ``consider-using-dict-comprehension`` when creating a dict + using a list of tuples where key AND value vary depending on the same condition. + + Closes #5588 + * ``used-before-assignment`` now considers that assignments in a try block may not have occurred when the except or finally blocks are executed. diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst index 219aec5124..47b96b41dc 100644 --- a/doc/whatsnew/2.13.rst +++ b/doc/whatsnew/2.13.rst @@ -32,6 +32,11 @@ Extensions Other Changes ============= +* Fixed false positive ``consider-using-dict-comprehension`` when creating a dict + using a list of tuples where key AND value vary depending on the same condition. + + Closes #5588 + * By default, pylint does no longer take files starting with ``.#`` into account. Those are considered `emacs file locks`_. This behavior can be reverted by redefining the ``ignore-patterns`` option. From 539d8d6dd4535a1d048a98eb36fd3080ba3ec733 Mon Sep 17 00:00:00 2001 From: Tushar Sadhwani Date: Thu, 23 Dec 2021 17:09:26 +0530 Subject: [PATCH 3/6] Typo fix --- pylint/checkers/refactoring/refactoring_checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index ce8b8eda87..dc1d42e9be 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -952,7 +952,7 @@ def _check_consider_using_comprehension_constructor(self, node): key2, value2 = element.orelse.elts if ( key1.as_string() != key2.as_string() - and value1.as_string != value2.as_string() + and value1.as_string() != value2.as_string() ): return From 1cbc08af56b6fcbf553e0f39c24677281f9264db Mon Sep 17 00:00:00 2001 From: Tushar Sadhwani <86737547+tushar-deepsource@users.noreply.github.com> Date: Thu, 23 Dec 2021 20:39:28 +0530 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Pierre Sassoulas --- pylint/checkers/refactoring/refactoring_checker.py | 2 +- .../functional/c/consider/consider_using_dict_comprehension.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index dc1d42e9be..666314e1a6 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -940,7 +940,7 @@ def _check_consider_using_comprehension_constructor(self, node): return # If we have an `IfExp` here where both the key AND value - # are different, then don't raise the issue. + # are different, then don't raise the issue. See #5588 if ( isinstance(element, nodes.IfExp) and isinstance(element.body, (nodes.Tuple, nodes.List)) diff --git a/tests/functional/c/consider/consider_using_dict_comprehension.py b/tests/functional/c/consider/consider_using_dict_comprehension.py index c26a0d5b12..1c256ce594 100644 --- a/tests/functional/c/consider/consider_using_dict_comprehension.py +++ b/tests/functional/c/consider/consider_using_dict_comprehension.py @@ -10,6 +10,7 @@ stuff = {1: 10, 2: -20} dict([(k, v) if v > 0 else (k, 0) for k, v in stuff.items()]) # [consider-using-dict-comprehension] +dict([(k, v) if v > 0 else (k*2, v) for k, v in stuff.items()]) # [consider-using-dict-comprehension] dict([(k, v) if v > 0 else (k * 2, 0) for k, v in stuff.items()]) # Cannot emit as this cannot be written as a comprehension From ef559d4be465cc31941aeac039645f0b842ba2bc Mon Sep 17 00:00:00 2001 From: Tushar Sadhwani Date: Thu, 23 Dec 2021 20:44:01 +0530 Subject: [PATCH 5/6] Fix test file --- .../functional/c/consider/consider_using_dict_comprehension.py | 2 +- .../functional/c/consider/consider_using_dict_comprehension.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/functional/c/consider/consider_using_dict_comprehension.py b/tests/functional/c/consider/consider_using_dict_comprehension.py index 1c256ce594..ef9ee3f012 100644 --- a/tests/functional/c/consider/consider_using_dict_comprehension.py +++ b/tests/functional/c/consider/consider_using_dict_comprehension.py @@ -1,4 +1,4 @@ -# pylint: disable=missing-docstring, invalid-name, use-dict-literal +# pylint: disable=missing-docstring, invalid-name, use-dict-literal, line-too-long numbers = [1, 2, 3, 4, 5, 6] diff --git a/tests/functional/c/consider/consider_using_dict_comprehension.txt b/tests/functional/c/consider/consider_using_dict_comprehension.txt index 23c805c792..2bde7ffa23 100644 --- a/tests/functional/c/consider/consider_using_dict_comprehension.txt +++ b/tests/functional/c/consider/consider_using_dict_comprehension.txt @@ -1,2 +1,3 @@ consider-using-dict-comprehension:9:0:9:48::Consider using a dictionary comprehension:UNDEFINED consider-using-dict-comprehension:12:0:12:61::Consider using a dictionary comprehension:UNDEFINED +consider-using-dict-comprehension:13:0:13:63::Consider using a dictionary comprehension:UNDEFINED From 81f6eb5674bb628c84a4f19f5cf57aa1147e6102 Mon Sep 17 00:00:00 2001 From: Tushar Sadhwani Date: Mon, 27 Dec 2021 16:46:02 +0530 Subject: [PATCH 6/6] Add copyrite alias --- .copyrite_aliases | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.copyrite_aliases b/.copyrite_aliases index 3dea23b4f7..773b12d61c 100644 --- a/.copyrite_aliases +++ b/.copyrite_aliases @@ -108,5 +108,10 @@ ], "authoritative_mail": "bot@noreply.github.com", "name": "bot" + }, + { + "mails": ["tushar.sadhwani000@gmail.com", "tushar@deepsource.io"], + "authoritative_mail": "tushar.sadhwani000@gmail.com", + "name": "Tushar Sadhwani" } ]