Skip to content

Commit 19cd0ee

Browse files
committed
[F812] Warn when list comprehension redefines a name
This lint rule is probably in Flake8 to prevent elements from leaking out of the comprehension scope, but the behavior was fixed in Python3. I worry such a strict lint rule will cause more problems than it solves, considering we use Python 3.10 at Meta. Perhaps we should consider disabling this rule by default. ghstack-source-id: 473edbf Pull Request resolved: #276
1 parent 1fbb2c3 commit 19cd0ee

File tree

1 file changed

+104
-0
lines changed

1 file changed

+104
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
#
3+
# This source code is licensed under the MIT license found in the
4+
# LICENSE file in the root directory of this source tree.
5+
6+
from typing import Optional, Set
7+
8+
import libcst as cst
9+
10+
from fixit import CstLintRule, InvalidTestCase as Invalid, ValidTestCase as Valid
11+
12+
13+
class ListCompMustUseUniqueNameRule(CstLintRule):
14+
"""
15+
A list comprehension shouldn't use the same name as another variable
16+
defined in the module.
17+
18+
Autofix: N/A
19+
"""
20+
21+
CUSTOM_MESSAGE = ("The name {name} is already defined in the module."
22+
"Although list comprehensions have their own "
23+
"scope, it is best practice to use a unique name.")
24+
25+
METADATA_DEPENDENCIES = (cst.metadata.ScopeProvider,)
26+
27+
def __init__(self) -> None:
28+
super().__init__()
29+
30+
def visit_ListComp(self, node: cst.ListComp) -> None:
31+
names: Set[str] = set()
32+
for_in: Optional[cst.CompFor] = node.for_in
33+
while for_in:
34+
assert isinstance(for_in.target, cst.Name)
35+
names.add(for_in.target.value)
36+
for_in = for_in.inner_for_in
37+
metadata = self.get_metadata(cst.metadata.ScopeProvider, node)
38+
assert isinstance(metadata, cst.metadata.Scope)
39+
for name in names:
40+
if metadata._contains_in_self_or_parent(name):
41+
self.report(node, self.CUSTOM_MESSAGE.format(name=name))
42+
43+
VALID = [
44+
Valid(
45+
"""
46+
n = 1
47+
squares = [i ** 2 for i in range(10)]
48+
"""
49+
),
50+
Valid(
51+
"""
52+
doubles = [i * 2 for i in range(10)]
53+
squares = [i ** 2 for i in range(10)]
54+
"""
55+
),
56+
Valid(
57+
"""
58+
tags = ["a", "b", "c", "d"]
59+
entries = [["a", "b"],["c", "d"]]
60+
results = [lst for tag in tags for lst in entries if tag in lst]
61+
"""
62+
),
63+
]
64+
65+
INVALID = [
66+
Invalid(
67+
"""
68+
def fn():
69+
return [i ** 2 for i in range(10)]
70+
71+
i = 1
72+
"""
73+
),
74+
Invalid(
75+
"""
76+
i = 1
77+
squares = [i ** 2 for i in range(10)]
78+
"""
79+
),
80+
Invalid(
81+
"""
82+
i = 1
83+
84+
def fn():
85+
return [i ** 2 for i in range(10)]
86+
"""
87+
),
88+
Invalid(
89+
"""
90+
tags = ["a", "b", "c", "d"]
91+
entries = [["a", "b"],["c", "d"]]
92+
tag = "a"
93+
results = [lst for tag in tags for lst in entries if tag in lst]
94+
"""
95+
),
96+
Invalid(
97+
"""
98+
tags = ["a", "b", "c", "d"]
99+
entries = [["a", "b"],["c", "d"]]
100+
lst = ["a", "b"]
101+
results = [lst for tag in tags for lst in entries if tag in lst]
102+
"""
103+
),
104+
]

0 commit comments

Comments
 (0)