Skip to content

Commit f894678

Browse files
fix: avoid false positive when module-level names match class-level names (#10723)
Add scope comparision to avoid module-level constants to be incorrectly classified as variables when a class-level attribute with the same name exists Closes #10719 (cherry picked from commit e55e830)
1 parent 84b6552 commit f894678

File tree

6 files changed

+153
-13
lines changed

6 files changed

+153
-13
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fixed false positive for ``invalid-name`` where module-level constants were incorrectly classified as variables when a class-level attribute with the same name exists.
2+
3+
Closes #10719

pylint/checkers/utils.py

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1843,28 +1843,50 @@ def is_sys_guard(node: nodes.If) -> bool:
18431843
return False
18441844

18451845

1846+
def _is_node_in_same_scope(
1847+
candidate: nodes.NodeNG, node_scope: nodes.LocalsDictNodeNG
1848+
) -> bool:
1849+
if isinstance(candidate, (nodes.ClassDef, nodes.FunctionDef)):
1850+
return candidate.parent is not None and candidate.parent.scope() is node_scope
1851+
return candidate.scope() is node_scope
1852+
1853+
1854+
def _is_reassigned_relative_to_current(
1855+
node: nodes.NodeNG, varname: str, before: bool
1856+
) -> bool:
1857+
"""Check if the given variable name is reassigned in the same scope relative to
1858+
the current node.
1859+
"""
1860+
node_scope = node.scope()
1861+
node_lineno = node.lineno
1862+
if node_lineno is None:
1863+
return False
1864+
for a in node_scope.nodes_of_class(
1865+
(nodes.AssignName, nodes.ClassDef, nodes.FunctionDef)
1866+
):
1867+
if a.name == varname and a.lineno is not None:
1868+
if before:
1869+
if a.lineno < node_lineno:
1870+
if _is_node_in_same_scope(a, node_scope):
1871+
return True
1872+
elif a.lineno > node_lineno:
1873+
if _is_node_in_same_scope(a, node_scope):
1874+
return True
1875+
return False
1876+
1877+
18461878
def is_reassigned_before_current(node: nodes.NodeNG, varname: str) -> bool:
18471879
"""Check if the given variable name is reassigned in the same scope before the
18481880
current node.
18491881
"""
1850-
return any(
1851-
a.name == varname and a.lineno < node.lineno
1852-
for a in node.scope().nodes_of_class(
1853-
(nodes.AssignName, nodes.ClassDef, nodes.FunctionDef)
1854-
)
1855-
)
1882+
return _is_reassigned_relative_to_current(node, varname, before=True)
18561883

18571884

18581885
def is_reassigned_after_current(node: nodes.NodeNG, varname: str) -> bool:
18591886
"""Check if the given variable name is reassigned in the same scope after the
18601887
current node.
18611888
"""
1862-
return any(
1863-
a.name == varname and a.lineno > node.lineno
1864-
for a in node.scope().nodes_of_class(
1865-
(nodes.AssignName, nodes.ClassDef, nodes.FunctionDef)
1866-
)
1867-
)
1889+
return _is_reassigned_relative_to_current(node, varname, before=False)
18681890

18691891

18701892
def is_deleted_after_current(node: nodes.NodeNG, varname: str) -> bool:

tests/checkers/unittest_utils.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,3 +520,105 @@ def test_is_typing_member() -> None:
520520
)
521521
assert not utils.is_typing_member(code[0], ("Literal",))
522522
assert not utils.is_typing_member(code[1], ("Literal",))
523+
524+
525+
def test_is_reassigned_after_current_requires_isinstance_check() -> None:
526+
tree = astroid.parse(
527+
"""
528+
CONSTANT = 1
529+
530+
def global_function_assign():
531+
global CONSTANT
532+
def CONSTANT():
533+
pass
534+
CONSTANT()
535+
"""
536+
)
537+
func = tree.body[1]
538+
global_stmt = func.body[0]
539+
nested_func = func.body[1]
540+
541+
assert isinstance(global_stmt, nodes.Global)
542+
assert isinstance(nested_func, nodes.FunctionDef)
543+
544+
node_scope = global_stmt.scope()
545+
546+
assert nested_func.scope() == nested_func
547+
assert nested_func.scope() != node_scope
548+
549+
assert nested_func.parent.scope() == node_scope
550+
551+
assert utils.is_reassigned_after_current(global_stmt, "CONSTANT") is True
552+
553+
554+
def test_is_reassigned_before_current() -> None:
555+
tree = astroid.parse(
556+
"""
557+
x = 1
558+
x = 2
559+
x = 3
560+
"""
561+
)
562+
first_assign = tree.body[0]
563+
second_assign = tree.body[1]
564+
third_assign = tree.body[2]
565+
566+
assert isinstance(first_assign, nodes.Assign)
567+
assert isinstance(second_assign, nodes.Assign)
568+
assert isinstance(third_assign, nodes.Assign)
569+
570+
third_assign_name = third_assign.targets[0]
571+
first_assign_name = first_assign.targets[0]
572+
573+
assert isinstance(third_assign_name, nodes.AssignName)
574+
assert isinstance(first_assign_name, nodes.AssignName)
575+
576+
assert utils.is_reassigned_before_current(third_assign_name, "x") is True
577+
assert utils.is_reassigned_before_current(first_assign_name, "x") is False
578+
579+
580+
def test_is_reassigned_after_current_with_assignname() -> None:
581+
tree = astroid.parse(
582+
"""
583+
x = 1
584+
x = 2
585+
x = 3
586+
"""
587+
)
588+
first_assign = tree.body[0]
589+
second_assign = tree.body[1]
590+
third_assign = tree.body[2]
591+
592+
assert isinstance(first_assign, nodes.Assign)
593+
assert isinstance(second_assign, nodes.Assign)
594+
assert isinstance(third_assign, nodes.Assign)
595+
596+
first_assign_name = first_assign.targets[0]
597+
third_assign_name = third_assign.targets[0]
598+
599+
assert isinstance(first_assign_name, nodes.AssignName)
600+
assert isinstance(third_assign_name, nodes.AssignName)
601+
602+
assert utils.is_reassigned_after_current(first_assign_name, "x") is True
603+
assert utils.is_reassigned_after_current(third_assign_name, "x") is False
604+
605+
606+
def test_is_reassigned_with_node_no_lineno() -> None:
607+
tree = astroid.parse(
608+
"""
609+
x = 1
610+
x = 2
611+
"""
612+
)
613+
first_assign = tree.body[0]
614+
first_assign_name = first_assign.targets[0]
615+
616+
assert isinstance(first_assign_name, nodes.AssignName)
617+
original_lineno = first_assign_name.lineno
618+
first_assign_name.lineno = None
619+
620+
try:
621+
assert utils.is_reassigned_after_current(first_assign_name, "x") is False
622+
assert utils.is_reassigned_before_current(first_assign_name, "x") is False
623+
finally:
624+
first_assign_name.lineno = original_lineno

tests/functional/i/invalid/invalid_name/invalid_name_module_level.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,15 @@ def A(): # [invalid-name]
5555

5656
from typing import Annotated
5757
IntWithAnnotation = Annotated[int, "anything"]
58+
59+
60+
# Regression test for #10719: module-level constants should not be incorrectly
61+
# classified as variables when a class-level attribute with the same name exists.
62+
class Theme:
63+
INPUT = ">>> "
64+
65+
66+
INPUT = Theme()
67+
input = Theme() # pylint: disable=redefined-builtin
68+
OUTPUT = Theme()
69+
output = Theme()

tests/functional/n/no/no_dummy_redefined.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
"""Make sure warnings about redefinitions do not trigger for dummy variables."""
2+
# pylint: disable=invalid-name
23

34

45
_, INTERESTING = 'a=b'.split('=')
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
redefined-outer-name:11:4:11:9:clobbering:Redefining name 'value' from outer scope (line 6):UNDEFINED
1+
redefined-outer-name:12:4:12:9:clobbering:Redefining name 'value' from outer scope (line 7):UNDEFINED

0 commit comments

Comments
 (0)