Skip to content

Commit

Permalink
Fix incorrect suggestion for unnecessary-comprehension (#9172)
Browse files Browse the repository at this point in the history
  • Loading branch information
faisalalisayyed authored Nov 17, 2023
1 parent 49358d4 commit 6f83d5a
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 20 deletions.
12 changes: 12 additions & 0 deletions doc/whatsnew/fragments/9172.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Fixed incorrect suggestion for shallow copy in unnecessary-comprehension

Example of the suggestion:
#pylint: disable=missing-module-docstring
a = [1, 2, 3]
b = [x for x in a]
b[0] = 0
print(a) # [1, 2, 3]

After changing b = [x for x in a] to b = a based on the suggestion, the script now prints [0, 2, 3]. The correct suggestion should be use list(a) to preserve the original behavior.

Closes #9172
16 changes: 8 additions & 8 deletions pylint/checkers/refactoring/refactoring_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1801,15 +1801,15 @@ def _check_unnecessary_comprehension(self, node: nodes.Comprehension) -> None:
if isinstance(node.parent, nodes.DictComp) and isinstance(
inferred, astroid.objects.DictItems
):
args = (f"{node.iter.func.expr.as_string()}",)
elif (
isinstance(node.parent, nodes.ListComp)
and isinstance(inferred, nodes.List)
) or (
isinstance(node.parent, nodes.SetComp)
and isinstance(inferred, nodes.Set)
args = (f"dict({node.iter.func.expr.as_string()})",)
elif isinstance(node.parent, nodes.ListComp) and isinstance(
inferred, nodes.List
):
args = (f"{node.iter.as_string()}",)
args = (f"list({node.iter.as_string()})",)
elif isinstance(node.parent, nodes.SetComp) and isinstance(
inferred, nodes.Set
):
args = (f"set({node.iter.as_string()})",)
if args:
self.add_message(
"unnecessary-comprehension", node=node.parent, args=args
Expand Down
2 changes: 2 additions & 0 deletions tests/functional/u/unnecessary/unnecessary_comprehension.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
# List comprehensions
[x for x in iterable] # [unnecessary-comprehension]
[y for x in iterable] # expression != target_list
[x for x in iterable] # [unnecessary-comprehension] use list(iterable)
[x for x,y,z in iterable] # expression != target_list
[(x, y) for x, y in iterable] # [unnecessary-comprehension]
[(x,y,z) for x,y,z in iterable] # [unnecessary-comprehension]
[(x,y,z) for (x,y,z) in iterable] # [unnecessary-comprehension]
[x for x, *y in iterable] # expression != target_list
Expand Down
26 changes: 14 additions & 12 deletions tests/functional/u/unnecessary/unnecessary_comprehension.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
unnecessary-comprehension:5:0:5:21::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
unnecessary-comprehension:8:0:8:31::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
unnecessary-comprehension:9:0:9:33::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
unnecessary-comprehension:17:7:17:42::Unnecessary use of a comprehension, use list(a_dict.items()) instead.:UNDEFINED
unnecessary-comprehension:20:0:20:21::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
unnecessary-comprehension:23:0:23:31::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
unnecessary-comprehension:24:7:24:42::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
unnecessary-comprehension:32:0:32:27::Unnecessary use of a comprehension, use dict(iterable) instead.:UNDEFINED
unnecessary-comprehension:34:0:34:29::Unnecessary use of a comprehension, use dict(iterable) instead.:UNDEFINED
unnecessary-comprehension:46:0:46:26::Unnecessary use of a comprehension, use my_list instead.:UNDEFINED
unnecessary-comprehension:47:8:47:42::Unnecessary use of a comprehension, use my_dict instead.:UNDEFINED
consider-using-dict-items:48:0:None:None::Consider iterating with .items():UNDEFINED
unnecessary-comprehension:49:0:49:25::Unnecessary use of a comprehension, use my_set instead.:UNDEFINED
unnecessary-comprehension:7:0:7:21::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
unnecessary-comprehension:9:0:9:29::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
unnecessary-comprehension:10:0:10:31::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
unnecessary-comprehension:11:0:11:33::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
unnecessary-comprehension:19:7:19:42::Unnecessary use of a comprehension, use list(a_dict.items()) instead.:UNDEFINED
unnecessary-comprehension:22:0:22:21::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
unnecessary-comprehension:25:0:25:31::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
unnecessary-comprehension:26:7:26:42::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
unnecessary-comprehension:34:0:34:27::Unnecessary use of a comprehension, use dict(iterable) instead.:UNDEFINED
unnecessary-comprehension:36:0:36:29::Unnecessary use of a comprehension, use dict(iterable) instead.:UNDEFINED
unnecessary-comprehension:48:0:48:26::Unnecessary use of a comprehension, use list(my_list) instead.:UNDEFINED
unnecessary-comprehension:49:8:49:42::Unnecessary use of a comprehension, use dict(my_dict) instead.:UNDEFINED
consider-using-dict-items:50:0:None:None::Consider iterating with .items():UNDEFINED
unnecessary-comprehension:51:0:51:25::Unnecessary use of a comprehension, use set(my_set) instead.:UNDEFINED

0 comments on commit 6f83d5a

Please sign in to comment.