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

* Treat exclusive assignments as "constants"

* Allow passing module-level non-constants against either constant or variable regexes

Author: jacobtylerwalls

custom_dict.txt

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

doc/data/messages/i/invalid-name/details.rst

@@ -2,35 +2,35 @@ Pylint recognizes a number of different name types internally. With a few
 exceptions, the type of the name is governed by the location the assignment to a
 name is found in, and not the type of object assigned.
 
-+--------------------+---------------------------------------------------------------------------------------------------+
-| Name Type          | Description                                                                                       |
-+====================+===================================================================================================+
-| ``module``         | Module and package names, same as the file names.                                                 |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``const``          | Module-level constants, any variable defined at module level that is not bound to a class object. |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``class``          | Names in ``class`` statements, as well as names bound to class objects at module level.           |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``function``       | Functions, toplevel or nested in functions or methods.                                            |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``method``         | Methods, functions defined in class bodies. Includes static and class methods.                    |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``attr``           | Attributes created on class instances inside methods.                                             |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``argument``       | Arguments to any function type, including lambdas.                                                |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``variable``       | Local variables in function scopes.                                                               |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``class-attribute``| Attributes defined in class bodies.                                                               |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``class-const``    | Enum constants and class variables annotated with ``Final``                                       |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``inlinevar``      | Loop variables in list comprehensions and generator expressions.                                  |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``typevar``        | Type variable declared with ``TypeVar``.                                                          |
-+--------------------+---------------------------------------------------------------------------------------------------+
-| ``typealias``      | Type alias declared with ``TypeAlias`` or assignments of ``Union``.                               |
-+--------------------+---------------------------------------------------------------------------------------------------+
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| Name Type          | Description                                                                                                 |
++====================+=============================================================================================================+
+| ``module``         | Module and package names, same as the file names.                                                           |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``const``          | Module-level constants: any name defined at module level that is not bound to a class object nor reassigned.|
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``class``          | Names in ``class`` statements, as well as names bound to class objects at module level.                     |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``function``       | Functions, toplevel or nested in functions or methods.                                                      |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``method``         | Methods, functions defined in class bodies. Includes static and class methods.                              |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``attr``           | Attributes created on class instances inside methods.                                                       |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``argument``       | Arguments to any function type, including lambdas.                                                          |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``variable``       | Local variables in function scopes or module-level names that are assigned multiple times.                  |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``class-attribute``| Attributes defined in class bodies.                                                                         |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``class-const``    | Enum constants and class variables annotated with ``Final``                                                 |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``inlinevar``      | Loop variables in list comprehensions and generator expressions.                                            |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``typevar``        | Type variable declared with ``TypeVar``.                                                                    |
++--------------------+-------------------------------------------------------------------------------------------------------------+
+| ``typealias``      | Type alias declared with ``TypeAlias`` or assignments of ``Union``.                                         |
++--------------------+-------------------------------------------------------------------------------------------------------------+
 
 Default behavior
 ~~~~~~~~~~~~~~~~
@@ -50,7 +50,7 @@ Following predefined naming styles are available:
 * ``UPPER_CASE``
 * ``any`` - fake style which does not enforce any limitations
 
-Following options are exposed:
+The following options are exposed:
 
 .. option:: --module-naming-style=<style>
 

doc/whatsnew/2/2.5/summary.rst

@@ -61,7 +61,7 @@ Other Changes
 * Add new ``good-names-rgx`` and ``bad-names-rgx`` to enable permitting or disallowing of names via regular expressions
 
   To enable better handling of permitted/disallowed names, we added two new config options: good-names-rgxs: a comma-
-  separated list of regexes, that if a name matches will be exempt of naming-checking. bad-names-rgxs: a comma-
+  separated list of regexes, that if a name matches will be exempt from naming-checking. bad-names-rgxs: a comma-
   separated list of regexes, that if a name matches will be always marked as a disallowed name.
 
 * Mutable ``collections.*`` are now flagged as dangerous defaults.

doc/whatsnew/fragments/3585.breaking

@@ -0,0 +1,9 @@
+`invalid-name` now distinguishes module-level constants that are assigned only once
+from those that are reassigned and now applies `--variable-rgx` to the latter. Values
+other than literals (lists, sets, objects) can pass against either the constant or
+variable regexes (e.g. "LOGGER" or "logger" but not "LoGgEr").
+
+Remember that `--good-names` or `--good-names-rgxs` can be provided to explicitly
+allow good names.
+
+Closes #3585

pylint/checkers/base/name_checker/checker.py

@@ -18,6 +18,7 @@
 
 import astroid
 from astroid import nodes
+from astroid.typing import InferenceResult
 
 from pylint import constants, interfaces
 from pylint.checkers import utils
@@ -399,7 +400,7 @@ def visit_functiondef(self, node: nodes.FunctionDef) -> None:
         "typevar-double-variance",
         "typevar-name-mismatch",
     )
-    def visit_assignname(  # pylint: disable=too-many-branches
+    def visit_assignname(  # pylint: disable=too-many-branches, too-many-statements
         self, node: nodes.AssignName
     ) -> None:
         """Check module level assigned names."""
@@ -461,14 +462,40 @@ 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
+                elif inferred_assign_type in (None, astroid.util.Uninferable):
+                    return
+
+                # 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)
+                    if not self._meets_exception_for_non_consts(
+                        inferred_assign_type, node.name
+                    ):
+                        self._check_name("const", node.name, node)
                 else:
+                    node_type = "variable"
+                    if (
+                        (iattrs := tuple(node.frame().igetattr(node.name)))
+                        and 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
@@ -501,6 +528,14 @@ def visit_assignname(  # pylint: disable=too-many-branches
             else:
                 self._check_name("class_attribute", node.name, node)
 
+    def _meets_exception_for_non_consts(
+        self, inferred_assign_type: InferenceResult | None, name: str
+    ) -> bool:
+        if isinstance(inferred_assign_type, nodes.Const):
+            return False
+        regexp = self._name_regexps["variable"]
+        return regexp.match(name) is not None
+
     def _recursive_check_names(self, args: list[nodes.AssignName]) -> None:
         """Check names in a possibly recursive list <arg>."""
         for arg in args:
@@ -608,11 +643,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

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