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

jacobtylerwalls · jacobtylerwalls · commit cbb41d4d2dbd · 2025-03-01T21:13:49.000-05:00
diff --git a/doc/user_guide/configuration/all-options.rst b/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', 'logger', 'ex', 'Run', '_')``
+**Default:**  ``('i', 'j', 'k', 'ex', 'Run', '_')``
 
 
 --good-names-rgxs
@@ -588,7 +588,7 @@ Standard Checkers
 
    # function-rgx =
 
-   good-names = ["i", "j", "k", "logger", "ex", "Run", "_"]
+   good-names = ["i", "j", "k", "ex", "Run", "_"]
 
    good-names-rgxs = []
 
diff --git a/doc/whatsnew/fragments/3585.breaking b/doc/whatsnew/fragments/3585.breaking
@@ -1,10 +1,9 @@
-`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.
+`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. `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).
+allow good names.
 
 Closes #3585
diff --git a/examples/pylintrc b/examples/pylintrc
@@ -193,7 +193,6 @@ function-naming-style=snake_case
 good-names=i,
            j,
            k,
-           logger,
            ex,
            Run,
            _
diff --git a/examples/pyproject.toml b/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", "logger", "ex", "Run", "_" ]
+good-names = [ "i", "j", "k", "ex", "Run", "_" ]
 
 # Good variable names regexes, separated by a comma. If names match any regex,
 # they will always be accepted
diff --git a/pylint/checkers/base/name_checker/checker.py b/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
@@ -199,7 +200,7 @@ class NameChecker(_BasicChecker):
         (
             "good-names",
             {
-                "default": ("i", "j", "k", "logger", "ex", "Run", "_"),
+                "default": ("i", "j", "k", "ex", "Run", "_"),
                 "type": "csv",
                 "metavar": "<names>",
                 "help": "Good variable names which should always be accepted,"
@@ -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."""
@@ -477,7 +478,10 @@ def visit_assignname(  # pylint: disable=too-many-branches
                         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 (
@@ -524,6 +528,17 @@ 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
+        if self._name_allowed_by_regex(name=name):
+            return True
+        regexp = self._name_regexps["variable"]
+        match = regexp.match(name)
+        return match 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:
diff --git a/pylintrc b/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,logger,ex,Run,_
+good-names=i,j,k,ex,Run,_
 
 # Good variable names regexes, separated by a comma. If names match any regex,
 # they will always be accepted
diff --git a/tests/functional/a/arguments.py b/tests/functional/a/arguments.py
@@ -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):
diff --git a/tests/functional/c/condition_evals_to_constant.py b/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]
diff --git a/tests/functional/c/consider/consider_using_enumerate.py b/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]
diff --git a/tests/functional/m/modified_iterating.py b/tests/functional/m/modified_iterating.py
@@ -4,29 +4,29 @@
 import copy
 from enum import Enum
 
-ITEM_LIST = [1, 2, 3]
-for item in ITEM_LIST:
-    ITEM_LIST.append(item)  # [modified-iterating-list]
+item_list = [1, 2, 3]
+for item in item_list:
+    item_list.append(item)  # [modified-iterating-list]
 
-for item in ITEM_LIST:
-    ITEM_LIST.remove(item)  # [modified-iterating-list]
+for item in item_list:
+    item_list.remove(item)  # [modified-iterating-list]
 
-for item in ITEM_LIST.copy():
-    ITEM_LIST.append(item)
-    ITEM_LIST.remove(item)
+for item in item_list.copy():
+    item_list.append(item)
+    item_list.remove(item)
 
-for item in copy(ITEM_LIST):
-    ITEM_LIST.append(item)
-    ITEM_LIST.remove(item)
+for item in copy(item_list):
+    item_list.append(item)
+    item_list.remove(item)
 
-for item in [k for k in ITEM_LIST]:
-    ITEM_LIST.append(item)
-    ITEM_LIST.remove(item)
+for item in [k for k in item_list]:
+    item_list.append(item)
+    item_list.remove(item)
 
 MY_DICT = {"1": 1, "2": 2, "3": 3}
 i = 1
 for item in MY_DICT:
-    ITEM_LIST[0] = i  # for coverage, see reference at /pull/5628#discussion_r792181642
+    item_list[0] = i  # for coverage, see reference at /pull/5628#discussion_r792181642
     MY_DICT[i] = 1  # [modified-iterating-dict]
     i += 1
 
@@ -58,11 +58,11 @@
 for item in item_set.copy():
     item_set.add(item + 10)
 
-for l in ITEM_LIST:
+for l in item_list:
     for s in item_set:
-        ITEM_LIST.append(1)  # [modified-iterating-list]
+        item_list.append(1)  # [modified-iterating-list]
         item_set.remove(4)  # [modified-iterating-set]
-    ITEM_LIST.remove(1)  # [modified-iterating-list]
+    item_list.remove(1)  # [modified-iterating-list]
 
 for item in [1, 2, 3]:
     del item  # [modified-iterating-list]
@@ -77,13 +77,13 @@
     del element  # [modified-iterating-set]
 
 # Check for nested for loops and changes to iterators
-for l in ITEM_LIST:
-    ITEM_LIST.append(1)  # [modified-iterating-list]
+for l in item_list:
+    item_list.append(1)  # [modified-iterating-list]
     for _ in []:
         for _ in []:
-            ITEM_LIST.remove(1)  # [modified-iterating-list]
+            item_list.remove(1)  # [modified-iterating-list]
             for _ in []:
-                ITEM_LIST.append(1)  # [modified-iterating-list]
+                item_list.append(1)  # [modified-iterating-list]
 
 
 def format_manifest_serializer_errors(errors):
diff --git a/tests/functional/m/modified_iterating.txt b/tests/functional/m/modified_iterating.txt
@@ -1,21 +1,21 @@
-modified-iterating-list:9:4:9:26::Iterated list 'ITEM_LIST' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
-modified-iterating-list:12:4:12:26::Iterated list 'ITEM_LIST' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
+modified-iterating-list:9:4:9:26::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
+modified-iterating-list:12:4:12:26::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
 modified-iterating-dict:30:4:30:18::Iterated dict 'MY_DICT' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
 modified-iterating-set:40:4:40:27::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
 modified-iterating-set:44:4:44:20::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
 modified-iterating-set:48:4:48:23::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
 modified-iterating-set:52:4:52:18::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
 modified-iterating-set:56:4:56:21::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
-modified-iterating-list:63:8:63:27::Iterated list 'ITEM_LIST' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
+modified-iterating-list:63:8:63:27::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
 modified-iterating-set:64:8:64:26::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
-modified-iterating-list:65:4:65:23::Iterated list 'ITEM_LIST' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
+modified-iterating-list:65:4:65:23::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
 modified-iterating-list:68:4:68:12::Iterated list 'list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
 modified-iterating-list:71:4:71:20::Iterated list 'list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
 modified-iterating-dict:74:4:74:9::Iterated dict 'MY_DICT' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
 modified-iterating-set:77:4:77:15::Iterated set 'item_set' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
-modified-iterating-list:81:4:81:23::Iterated list 'ITEM_LIST' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
-modified-iterating-list:84:12:84:31::Iterated list 'ITEM_LIST' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
-modified-iterating-list:86:16:86:35::Iterated list 'ITEM_LIST' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
+modified-iterating-list:81:4:81:23::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
+modified-iterating-list:84:12:84:31::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
+modified-iterating-list:86:16:86:35::Iterated list 'item_list' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
 modified-iterating-dict:112:8:112:28:update_existing_key:Iterated dict 'MY_DICT' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
 modified-iterating-list:124:12:124:19:MyClass.my_method:Iterated list 'attribute' is being modified inside for loop body, consider iterating through a copy of it instead.:INFERENCE
 modified-iterating-dict:142:8:142:15:my_call:Iterated dict '' is being modified inside for loop body, iterate through a copy of it instead.:INFERENCE
diff --git a/tests/functional/r/renamed_import_logging_not_lazy.py b/tests/functional/r/renamed_import_logging_not_lazy.py
@@ -10,11 +10,11 @@ class Logger:
 
 
 logger = renamed_logging.getLogger(__name__)
-FAKE_LOGGER = Logger()
+fake_logger = Logger()
 
 # Statements that should be flagged
 renamed_logging.warning('%s, %s' % (4, 5))  # [logging-not-lazy]
 logger.warning('%s' % 5)  # [logging-not-lazy]
 
 # Statements that should not be flagged:
-FAKE_LOGGER.warn('%s' % 5)
+fake_logger.warn('%s' % 5)
diff --git a/tests/functional/u/unnecessary/unnecessary_comprehension.py b/tests/functional/u/unnecessary/unnecessary_comprehension.py
@@ -15,15 +15,15 @@
 [2 * x for x in iterable]  # exclude useful comprehensions
 [(x, y, 1) for x, y in iterable]  # exclude useful comprehensions
 # Test case for issue #4499
-A_DICT = {}
-item = [(k, v) for k, v in A_DICT.items()]  # [unnecessary-comprehension]
+a_dict = {}
+b_dict = [(k, v) for k, v in a_dict.items()]  # [unnecessary-comprehension]
 
 # Set comprehensions
 {x for x in iterable}  # [unnecessary-comprehension]
 {y for x in iterable}  # expression != target_list
 {x for x,y,z in iterable}  # expression != target_list
 {(x,y,z) for x,y,z in iterable}  # [unnecessary-comprehension]
-item = {(x,y,z) for (x, y, z) in iterable}  # [unnecessary-comprehension]
+b_dict = {(x,y,z) for (x, y, z) in iterable}  # [unnecessary-comprehension]
 {(x,y,z) for x in iterable}  # expression != target_list
 {(x,y,(a,b,c)) for x in iterable}  # expression != target_list
 {x for x, *y in iterable}  # expression != target_list
@@ -41,11 +41,11 @@
 {2 * x: 3 + x for x in iterable}  # exclude useful comprehensions
 
 # Some additional tests on helptext -- when object is already a list/set/dict
-MY_LIST = []
-MY_DICT = {}
-MY_SET = set()
+my_list = []
+my_dict = {}
+my_set = set()
 
-[elem for elem in MY_LIST]  # [unnecessary-comprehension]
-ITEMS = {k: v for k, v in MY_DICT.items()} # [unnecessary-comprehension]
-{k: MY_DICT[k] for k in MY_DICT} # [consider-using-dict-items]
-{elem for elem in MY_SET}  # [unnecessary-comprehension]
+[elem for elem in my_list]  # [unnecessary-comprehension]
+ITEMS = {k: v for k, v in my_dict.items()} # [unnecessary-comprehension]
+{k: my_dict[k] for k in my_dict} # [consider-using-dict-items]
+{elem for elem in my_set}  # [unnecessary-comprehension]
diff --git a/tests/functional/u/unnecessary/unnecessary_comprehension.txt b/tests/functional/u/unnecessary/unnecessary_comprehension.txt
@@ -3,13 +3,13 @@ unnecessary-comprehension:7:0:7:21::Unnecessary use of a comprehension, use list
 unnecessary-comprehension:9:0:9:29::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
 unnecessary-comprehension:10:0:10:31::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
 unnecessary-comprehension:11:0:11:33::Unnecessary use of a comprehension, use list(iterable) instead.:UNDEFINED
-unnecessary-comprehension:19:7:19:42::Unnecessary use of a comprehension, use list(A_DICT.items()) instead.:UNDEFINED
+unnecessary-comprehension:19:9:19:44::Unnecessary use of a comprehension, use list(a_dict.items()) instead.:UNDEFINED
 unnecessary-comprehension:22:0:22:21::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
 unnecessary-comprehension:25:0:25:31::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
-unnecessary-comprehension:26:7:26:42::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
+unnecessary-comprehension:26:9:26:44::Unnecessary use of a comprehension, use set(iterable) instead.:UNDEFINED
 unnecessary-comprehension:34:0:34:27::Unnecessary use of a comprehension, use dict(iterable) instead.:UNDEFINED
 unnecessary-comprehension:36:0:36:29::Unnecessary use of a comprehension, use dict(iterable) instead.:UNDEFINED
-unnecessary-comprehension:48:0:48:26::Unnecessary use of a comprehension, use list(MY_LIST) instead.:UNDEFINED
-unnecessary-comprehension:49:8:49:42::Unnecessary use of a comprehension, use dict(MY_DICT) instead.:UNDEFINED
+unnecessary-comprehension:48:0:48:26::Unnecessary use of a comprehension, use list(my_list) instead.:UNDEFINED
+unnecessary-comprehension:49:8:49:42::Unnecessary use of a comprehension, use dict(my_dict) instead.:UNDEFINED
 consider-using-dict-items:50:0:None:None::Consider iterating with .items():UNDEFINED
-unnecessary-comprehension:51:0:51:25::Unnecessary use of a comprehension, use set(MY_SET) instead.:UNDEFINED
+unnecessary-comprehension:51:0:51:25::Unnecessary use of a comprehension, use set(my_set) instead.:UNDEFINED
diff --git a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py
diff --git a/tests/functional/u/used/used_before_assignment_comprehension_homonyms.py b/tests/functional/u/used/used_before_assignment_comprehension_homonyms.py
diff --git a/tests/functional/w/wrong_import_order.txt b/tests/functional/w/wrong_import_order.txt

Original file line number	Diff line number	Diff line change
`@@ -193,7 +193,6 @@ function-naming-style=snake_case`
`193`	`193`	`good-names=i,`
`194`	`194`	`j,`
`195`	`195`	`k,`
`196`		`- logger,`
`197`	`196`	`ex,`
`198`	`197`	`Run,`
`199`	`198`	`_`