Skip to content

Commit

Permalink
Apply variable name regex to module-level names that are reassigned
Browse files Browse the repository at this point in the history
  • Loading branch information
jacobtylerwalls committed Feb 1, 2025
1 parent 8138620 commit 46fe6c6
Show file tree
Hide file tree
Showing 92 changed files with 552 additions and 514 deletions.
7 changes: 7 additions & 0 deletions doc/whatsnew/fragments/3585.false_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
`invalid-name` now distinguishes module-level names that are assigned only once
from those that are reassigned and now applies `--variable-rgx` to the latter.

Also, `invalid-name` is triggered for module-level names for additional types
(e.g. lists and sets).

Closes #3585
28 changes: 21 additions & 7 deletions pylint/checkers/base/name_checker/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,14 +461,28 @@ def visit_assignname( # pylint: disable=too-many-branches
elif isinstance(inferred_assign_type, nodes.ClassDef):
self._check_name("class", node.name, node)

# Don't emit if the name redefines an import in an ImportError except handler.
elif not _redefines_import(node) and isinstance(
inferred_assign_type, nodes.Const
# Don't emit if the name redefines an import in an ImportError except handler
# nor any other reassignment.
elif (
not (redefines_import := _redefines_import(node))
and not isinstance(
inferred_assign_type, (nodes.FunctionDef, nodes.Lambda)
)
and not utils.is_reassigned_before_current(node, node.name)
and not utils.is_reassigned_after_current(node, node.name)
and not utils.get_node_first_ancestor_of_type(
node, (nodes.For, nodes.While)
)
):
self._check_name("const", node.name, node)
else:
elif inferred_assign_type is not None and not isinstance(
inferred_assign_type, astroid.util.UninferableBase
):
self._check_name(
"variable", node.name, node, disallowed_check_only=True
"variable",
node.name,
node,
disallowed_check_only=redefines_import,
)

# Check names defined in AnnAssign nodes
Expand Down Expand Up @@ -608,11 +622,11 @@ def _assigns_typevar(node: nodes.NodeNG | None) -> bool:
def _assigns_typealias(node: nodes.NodeNG | None) -> bool:
"""Check if a node is assigning a TypeAlias."""
inferred = utils.safe_infer(node)
if isinstance(inferred, nodes.ClassDef):
if isinstance(inferred, (nodes.ClassDef, astroid.bases.UnionType)):
qname = inferred.qname()
if qname == "typing.TypeAlias":
return True
if qname == ".Union":
if qname in {".Union", "builtins.UnionType"}:
# Union is a special case because it can be used as a type alias
# or as a type annotation. We only want to check the former.
assert node is not None
Expand Down
12 changes: 12 additions & 0 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1865,6 +1865,18 @@ def is_sys_guard(node: nodes.If) -> bool:
return False


def is_reassigned_before_current(node: nodes.NodeNG, varname: str) -> bool:
"""Check if the given variable name is reassigned in the same scope before the
current node.
"""
return any(
a.name == varname and a.lineno < node.lineno
for a in node.scope().nodes_of_class(
(nodes.AssignName, nodes.ClassDef, nodes.FunctionDef)
)
)


def is_reassigned_after_current(node: nodes.NodeNG, varname: str) -> bool:
"""Check if the given variable name is reassigned in the same scope after the
current node.
Expand Down
12 changes: 6 additions & 6 deletions tests/functional/a/arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ def function_default_arg(one=1, two=2):
function_default_arg(1, one=5) # [redundant-keyword-arg]

# Remaining tests are for coverage of correct names in messages.
LAMBDA = lambda arg: 1
my_lambda = lambda arg: 1

LAMBDA() # [no-value-for-parameter]
my_lambda() # [no-value-for-parameter]

def method_tests():
"""Method invocations."""
Expand Down Expand Up @@ -260,7 +260,7 @@ def func(one, two, three):
return one + two + three


CALL = lambda *args: func(*args)
call = lambda *args: func(*args)


# Ensure `too-many-function-args` is not emitted when a function call is assigned
Expand All @@ -275,9 +275,9 @@ def _print_selection(self):
pick_apple = _pick_fruit("apple")
pick_pear = _pick_fruit("pear")

picker = FruitPicker()
picker.pick_apple()
picker.pick_pear()
PICKER = FruitPicker()
PICKER.pick_apple()
PICKER.pick_pear()


def name1(apple, /, **kwargs):
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/a/arguments.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ no-value-for-parameter:59:0:59:21::No value for argument 'first_argument' in fun
unexpected-keyword-arg:59:0:59:21::Unexpected keyword argument 'bob' in function call:UNDEFINED
unexpected-keyword-arg:60:0:60:40::Unexpected keyword argument 'coin' in function call:UNDEFINED
redundant-keyword-arg:62:0:62:30::Argument 'one' passed by position and keyword in function call:UNDEFINED
no-value-for-parameter:67:0:67:8::No value for argument 'arg' in lambda call:UNDEFINED
no-value-for-parameter:67:0:67:11::No value for argument 'arg' in lambda call:UNDEFINED
no-value-for-parameter:72:4:72:24:method_tests:No value for argument 'arg' in staticmethod call:UNDEFINED
no-value-for-parameter:73:4:73:29:method_tests:No value for argument 'arg' in staticmethod call:UNDEFINED
no-value-for-parameter:75:4:75:23:method_tests:No value for argument 'arg' in classmethod call:UNDEFINED
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# pylint: disable=useless-return, condition-evals-to-constant
# pylint: disable=useless-return, condition-evals-to-constant, invalid-name
"""check assignment to function call where the function doesn't return
'E1111': ('Assigning to function call which doesn\'t return',
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/c/condition_evals_to_constant.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def func(_):
while CONSTANT and False: # [condition-evals-to-constant]
break
1 if CONSTANT or True else 2 # [condition-evals-to-constant]
z = [x for x in range(10) if x or True] # [condition-evals-to-constant]
Z = [x for x in range(10) if x or True] # [condition-evals-to-constant]

# Simplifies recursively
assert True or CONSTANT or OTHER # [condition-evals-to-constant]
Expand Down
44 changes: 22 additions & 22 deletions tests/functional/c/consider/consider_iterating_dictionary.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ def keys(self):
(key for key in {}.keys()) # [consider-iterating-dictionary]
{key for key in {}.keys()} # [consider-iterating-dictionary]
{key: key for key in {}.keys()} # [consider-iterating-dictionary]
COMP1 = [key for key in {}.keys()] # [consider-iterating-dictionary]
COMP2 = (key for key in {}.keys()) # [consider-iterating-dictionary]
COMP3 = {key for key in {}.keys()} # [consider-iterating-dictionary]
comp1 = [key for key in {}.keys()] # [consider-iterating-dictionary]
comp2 = (key for key in {}.keys()) # [consider-iterating-dictionary]
comp3 = {key for key in {}.keys()} # [consider-iterating-dictionary]
COMP4 = {key: key for key in {}.keys()} # [consider-iterating-dictionary]
for key in {}.keys(): # [consider-iterating-dictionary]
pass

# Issue #1247
DICT = {'a': 1, 'b': 2}
COMP1 = [k * 2 for k in DICT.keys()] + [k * 3 for k in DICT.keys()] # [consider-iterating-dictionary,consider-iterating-dictionary]
COMP2, COMP3 = [k * 2 for k in DICT.keys()], [k * 3 for k in DICT.keys()] # [consider-iterating-dictionary,consider-iterating-dictionary]
comp1 = [k * 2 for k in DICT.keys()] + [k * 3 for k in DICT.keys()] # [consider-iterating-dictionary,consider-iterating-dictionary]
comp2, comp3 = [k * 2 for k in DICT.keys()], [k * 3 for k in DICT.keys()] # [consider-iterating-dictionary,consider-iterating-dictionary]
SOME_TUPLE = ([k * 2 for k in DICT.keys()], [k * 3 for k in DICT.keys()]) # [consider-iterating-dictionary,consider-iterating-dictionary]

# Checks for membership checks
Expand All @@ -62,21 +62,21 @@ def keys(self):
pass
if [1] == dict():
pass
VAR = 1 in {}.keys() # [consider-iterating-dictionary]
VAR = 1 in {}
VAR = 1 in dict()
VAR = [1, 2] == {}.keys() in {False}
var = 1 in {}.keys() # [consider-iterating-dictionary]
var = 1 in {}
var = 1 in dict()
var = [1, 2] == {}.keys() in {False}

# Additional membership checks
# See: https://github.com/pylint-dev/pylint/issues/5323
metadata = {}
if "a" not in list(metadata.keys()): # [consider-iterating-dictionary]
METADATA = {}
if "a" not in list(METADATA.keys()): # [consider-iterating-dictionary]
print(1)
if "a" not in metadata.keys(): # [consider-iterating-dictionary]
if "a" not in METADATA.keys(): # [consider-iterating-dictionary]
print(1)
if "a" in list(metadata.keys()): # [consider-iterating-dictionary]
if "a" in list(METADATA.keys()): # [consider-iterating-dictionary]
print(1)
if "a" in metadata.keys(): # [consider-iterating-dictionary]
if "a" in METADATA.keys(): # [consider-iterating-dictionary]
print(1)


Expand All @@ -93,24 +93,24 @@ def inner_function():
return inner_function()
return InnerClass().another_function()

a_dict = {"a": 1, "b": 2, "c": 3}
a_set = {"c", "d"}
A_DICT = {"a": 1, "b": 2, "c": 3}
A_SET = {"c", "d"}

# Test bitwise operations. These should not raise msg because removing `.keys()`
# either gives error or ends in a different result
print(a_dict.keys() | a_set)
print(A_DICT.keys() | A_SET)

if "a" in a_dict.keys() | a_set:
if "a" in A_DICT.keys() | A_SET:
pass

if "a" in a_dict.keys() & a_set:
if "a" in A_DICT.keys() & A_SET:
pass

if 1 in a_dict.keys() ^ [1, 2]:
if 1 in A_DICT.keys() ^ [1, 2]:
pass

if "a" in a_dict.keys() or a_set: # [consider-iterating-dictionary]
if "a" in A_DICT.keys() or A_SET: # [consider-iterating-dictionary]
pass

if "a" in a_dict.keys() and a_set: # [consider-iterating-dictionary]
if "a" in A_DICT.keys() and A_SET: # [consider-iterating-dictionary]
pass
64 changes: 32 additions & 32 deletions tests/functional/c/consider/consider_using_dict_items.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,39 +20,39 @@ def good():
print(k)


out_of_scope_dict = dict()
OUT_OF_SCOPE_DICT = dict()


def another_bad():
for k in out_of_scope_dict: # [consider-using-dict-items]
print(out_of_scope_dict[k])
for k in OUT_OF_SCOPE_DICT: # [consider-using-dict-items]
print(OUT_OF_SCOPE_DICT[k])


def another_good():
for k in out_of_scope_dict:
for k in OUT_OF_SCOPE_DICT:
k = 1
k = 2
k = 3
print(out_of_scope_dict[k])
print(OUT_OF_SCOPE_DICT[k])


b_dict = {}
for k2 in b_dict: # Should not emit warning, key access necessary
b_dict[k2] = 2
B_DICT = {}
for k2 in B_DICT: # Should not emit warning, key access necessary
B_DICT[k2] = 2

for k2 in b_dict: # Should not emit warning, key access necessary (AugAssign)
b_dict[k2] += 2
for k2 in B_DICT: # Should not emit warning, key access necessary (AugAssign)
B_DICT[k2] += 2

# Warning should be emitted in this case
for k6 in b_dict: # [consider-using-dict-items]
val = b_dict[k6]
b_dict[k6] = 2
for k6 in B_DICT: # [consider-using-dict-items]
val = B_DICT[k6]
B_DICT[k6] = 2

for k3 in b_dict: # [consider-using-dict-items]
val = b_dict[k3]
for k3 in B_DICT: # [consider-using-dict-items]
val = B_DICT[k3]

for k4 in b_dict.keys(): # [consider-iterating-dictionary,consider-using-dict-items]
val = b_dict[k4]
for k4 in B_DICT.keys(): # [consider-iterating-dictionary,consider-using-dict-items]
val = B_DICT[k4]


class Foo:
Expand All @@ -67,25 +67,25 @@ class Foo:

# Should NOT emit warning whey key used to access a different dict
for k5 in Foo.c_dict: # This is fine
val = b_dict[k5]
val = B_DICT[k5]

for k5 in Foo.c_dict: # This is fine
val = c_dict[k5]

# Should emit warning within a list/dict comprehension
val = {k9: b_dict[k9] for k9 in b_dict} # [consider-using-dict-items]
val = [(k7, b_dict[k7]) for k7 in b_dict] # [consider-using-dict-items]
val = {k9: B_DICT[k9] for k9 in B_DICT} # [consider-using-dict-items]
val = [(k7, B_DICT[k7]) for k7 in B_DICT] # [consider-using-dict-items]

# Should emit warning even when using dict attribute of a class within comprehension
val = [(k7, Foo.c_dict[k7]) for k7 in Foo.c_dict] # [consider-using-dict-items]
val = any(True for k8 in Foo.c_dict if Foo.c_dict[k8]) # [consider-using-dict-items]

# Should emit warning when dict access done in ``if`` portion of comprehension
val = any(True for k8 in b_dict if b_dict[k8]) # [consider-using-dict-items]
val = any(True for k8 in B_DICT if B_DICT[k8]) # [consider-using-dict-items]

# Should NOT emit warning whey key used to access a different dict
val = [(k7, b_dict[k7]) for k7 in Foo.c_dict]
val = any(True for k8 in Foo.c_dict if b_dict[k8])
val = [(k7, B_DICT[k7]) for k7 in Foo.c_dict]
val = any(True for k8 in Foo.c_dict if B_DICT[k8])

# Should NOT emit warning, essentially same check as above
val = [(k7, c_dict[k7]) for k7 in Foo.c_dict]
Expand All @@ -97,20 +97,20 @@ class Foo:
# Test false positive described in #4630
# (https://github.com/pylint-dev/pylint/issues/4630)

d = {"key": "value"}
D = {"key": "value"}

for k in d: # this is fine, with the reassignment of d[k], d[k] is necessary
d[k] += "123"
if "1" in d[k]: # index lookup necessary here, do not emit error
for k in D: # this is fine, with the reassignment of d[k], d[k] is necessary
D[k] += "123"
if "1" in D[k]: # index lookup necessary here, do not emit error
print("found 1")

for k in d: # if this gets rewritten to d.items(), we are back to the above problem
d[k] = d[k] + 1
if "1" in d[k]: # index lookup necessary here, do not emit error
for k in D: # if this gets rewritten to d.items(), we are back to the above problem
D[k] = D[k] + 1
if "1" in D[k]: # index lookup necessary here, do not emit error
print("found 1")

for k in d: # [consider-using-dict-items]
if "1" in d[k]: # index lookup necessary here, do not emit error
for k in D: # [consider-using-dict-items]
if "1" in D[k]: # index lookup necessary here, do not emit error
print("found 1")


Expand Down
2 changes: 1 addition & 1 deletion tests/functional/c/consider/consider_using_enumerate.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class MyClass:
def __init__(self):
self.my_list = []

my_obj = MyClass()
MY_OBJ = MyClass()
def my_function(instance: MyClass):
for i in range(len(instance.my_list)): # [consider-using-enumerate]
var = instance.my_list[i]
Expand Down
Loading

0 comments on commit 46fe6c6

Please sign in to comment.