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/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/user_guide/configuration/all-options.rst

@@ -428,7 +428,7 @@ Standard Checkers
 """"""""""""
 *Good variable names which should always be accepted, separated by a comma.*
 
-**Default:**  ``('i', 'j', 'k', 'ex', 'Run', '_')``
+**Default:**  ``('i', 'j', 'k', 'logger', 'ex', 'Run', '_')``
 
 
 --good-names-rgxs
@@ -588,7 +588,7 @@ Standard Checkers
 
    # function-rgx =
 
-   good-names = ["i", "j", "k", "ex", "Run", "_"]
+   good-names = ["i", "j", "k", "logger", "ex", "Run", "_"]
 
    good-names-rgxs = []
 

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,10 @@
+`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.
+
+Remember that `--good-names` or `--good-names-rgxs` can be provided to explicitly
+allow good names. `logger` has been added to the default `--good-names`.
+
+Also, `invalid-name` is triggered for module-level names for additional types
+(e.g. lists and sets).
+
+Closes #3585

examples/pylintrc

@@ -193,6 +193,7 @@ function-naming-style=snake_case
 good-names=i,
            j,
            k,
+           logger,
            ex,
            Run,
            _

examples/pyproject.toml

@@ -167,7 +167,7 @@ function-naming-style = "snake_case"
 # function-rgx =
 
 # Good variable names which should always be accepted, separated by a comma.
-good-names = [ "i", "j", "k", "ex", "Run", "_" ]
+good-names = [ "i", "j", "k", "logger", "ex", "Run", "_" ]
 
 # Good variable names regexes, separated by a comma. If names match any regex,
 # they will always be accepted

pylint/checkers/base/name_checker/checker.py

@@ -199,7 +199,7 @@ class NameChecker(_BasicChecker):
         (
             "good-names",
             {
-                "default": ("i", "j", "k", "ex", "Run", "_"),
+                "default": ("i", "j", "k", "logger", "ex", "Run", "_"),
                 "type": "csv",
                 "metavar": "<names>",
                 "help": "Good variable names which should always be accepted,"
@@ -461,14 +461,37 @@ 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)
                 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
@@ -608,11 +631,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
         ):

pylintrc

@@ -231,7 +231,7 @@ expected-line-ending-format=
 [BASIC]
 
 # Good variable names which should always be accepted, separated by a comma
-good-names=i,j,k,ex,Run,_
+good-names=i,j,k,logger,ex,Run,_
 
 # Good variable names regexes, separated by a comma. If names match any regex,
 # they will always be accepted

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]