Apply variable name regex to module-level names that are reassigned #3585

custom_dict.txt

@@ -146,6 +146,7 @@ hmac
 html
 idgeneratormixin
 ifexpr
+igetattr
 importedname
 importfrom
 importnode

doc/whatsnew/fragments/3585.false_negative

@@ -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

pylint/checkers/base/name_checker/checker.py

@@ -461,14 +461,34 @@ 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 not in (None, astroid.util.Uninferable):
+                    node_type = "variable"
+                    iattrs = tuple(node.frame().igetattr(node.name))
+                    if (
+                        astroid.util.Uninferable not in iattrs
+                        and len(iattrs) == 2
+                        and astroid.are_exclusive(*iattrs)
+                    ):
+                        node_type = "const"
                     self._check_name(
-                        "variable", node.name, node, disallowed_check_only=True
+                        node_type,
+                        node.name,
+                        node,
+                        disallowed_check_only=redefines_import,
                     )
 
             # Check names defined in AnnAssign nodes
@@ -608,11 +628,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

pylint/checkers/utils.py

@@ -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.
@@ -2135,7 +2147,13 @@ def is_augmented_assign(node: nodes.Assign) -> tuple[bool, str]:
         binop.op in COMMUTATIVE_OPERATORS
         and _is_target_name_in_binop_side(target, binop.right)
     ):
-        inferred_left = safe_infer(binop.left)
+        if isinstance(binop.left, nodes.Const):
+            # This bizarrely became necessary after an unrelated call to igetattr().
+            # Seems like a code smell uncovered in #10212.
+            # tuple(node.frame().igetattr(node.name))
+            inferred_left = binop.left
+        else:
+            inferred_left = safe_infer(binop.left)
         if isinstance(inferred_left, nodes.Const) and isinstance(
             inferred_left.value, int
         ):

tests/functional/a/arguments.py

@@ -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."""
@@ -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
@@ -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):

tests/functional/a/arguments.txt

@@ -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

tests/functional/a/assignment/assignment_from_no_return_2.py

@@ -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',

tests/functional/c/condition_evals_to_constant.py

@@ -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]

tests/functional/c/consider/consider_iterating_dictionary.py

@@ -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
@@ -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)
 
 
@@ -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

tests/functional/c/consider/consider_using_dict_items.py

@@ -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:
@@ -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]
@@ -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")
 
 

tests/functional/c/consider/consider_using_enumerate.py

@@ -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]