Skip to content

Commit

Permalink
[F812] Warn when list comprehension redefines a name
Browse files Browse the repository at this point in the history
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: 0f7f3b64d39e518b4b64bd1ca55aa3bd6b934eb2
Pull Request resolved: #276
  • Loading branch information
ansley committed Mar 8, 2023
1 parent 805770b commit 40ed43b
Showing 1 changed file with 104 additions and 0 deletions.
104 changes: 104 additions & 0 deletions src/fixit/rules/pyflakes/list_comp_must_use_unique_name.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

from typing import Optional, Set

import libcst as cst

from fixit import CstLintRule, InvalidTestCase as Invalid, ValidTestCase as Valid


class ListCompMustUseUniqueNameRule(CstLintRule):
"""
A list comprehension shouldn't use the same name as another variable
defined in the module.
Autofix: N/A
"""

CUSTOM_MESSAGE = ("The name {name} is already defined in the module."
"Although list comprehensions have their own "
"scope, it is best practice to use a unique name.")

METADATA_DEPENDENCIES = (cst.metadata.ScopeProvider,)

def __init__(self) -> None:
super().__init__()

def visit_ListComp(self, node: cst.ListComp) -> None:
names: Set[str] = set()
for_in: Optional[cst.CompFor] = node.for_in
while for_in:
assert isinstance(for_in.target, cst.Name)
names.add(for_in.target.value)
for_in = for_in.inner_for_in
metadata = self.get_metadata(cst.metadata.ScopeProvider, node)
assert isinstance(metadata, cst.metadata.Scope)
for name in names:
if metadata._contains_in_self_or_parent(name):
self.report(node, self.CUSTOM_MESSAGE.format(name=name))

VALID = [
Valid(
"""
n = 1
squares = [i ** 2 for i in range(10)]
"""
),
Valid(
"""
doubles = [i * 2 for i in range(10)]
squares = [i ** 2 for i in range(10)]
"""
),
Valid(
"""
tags = ["a", "b", "c", "d"]
entries = [["a", "b"],["c", "d"]]
results = [lst for tag in tags for lst in entries if tag in lst]
"""
),
]

INVALID = [
Invalid(
"""
def fn():
return [i ** 2 for i in range(10)]
i = 1
"""
),
Invalid(
"""
i = 1
squares = [i ** 2 for i in range(10)]
"""
),
Invalid(
"""
i = 1
def fn():
return [i ** 2 for i in range(10)]
"""
),
Invalid(
"""
tags = ["a", "b", "c", "d"]
entries = [["a", "b"],["c", "d"]]
tag = "a"
results = [lst for tag in tags for lst in entries if tag in lst]
"""
),
Invalid(
"""
tags = ["a", "b", "c", "d"]
entries = [["a", "b"],["c", "d"]]
lst = ["a", "b"]
results = [lst for tag in tags for lst in entries if tag in lst]
"""
),
]

0 comments on commit 40ed43b

Please sign in to comment.