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

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