Skip to content

Commit c668682

Browse files
jacobtylerwallsjacobtylerwalls
authored
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
1 parent 38b63ac commit c668682

File tree

92 files changed

+508
-427
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

92 files changed

+508
-427
lines changed

‎custom_dict.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ hmac
146146
html
147147
idgeneratormixin
148148
ifexpr
149+
igetattr
149150
importedname
150151
importfrom
151152
importnode

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

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,35 @@ Pylint recognizes a number of different name types internally. With a few
22
exceptions, the type of the name is governed by the location the assignment to a
33
name is found in, and not the type of object assigned.
44

5-
+--------------------+---------------------------------------------------------------------------------------------------+
6-
| Name Type | Description |
7-
+====================+===================================================================================================+
8-
| ``module`` | Module and package names, same as the file names. |
9-
+--------------------+---------------------------------------------------------------------------------------------------+
10-
| ``const`` | Module-level constants, any variable defined at module level that is not bound to a class object. |
11-
+--------------------+---------------------------------------------------------------------------------------------------+
12-
| ``class`` | Names in ``class`` statements, as well as names bound to class objects at module level. |
13-
+--------------------+---------------------------------------------------------------------------------------------------+
14-
| ``function`` | Functions, toplevel or nested in functions or methods. |
15-
+--------------------+---------------------------------------------------------------------------------------------------+
16-
| ``method`` | Methods, functions defined in class bodies. Includes static and class methods. |
17-
+--------------------+---------------------------------------------------------------------------------------------------+
18-
| ``attr`` | Attributes created on class instances inside methods. |
19-
+--------------------+---------------------------------------------------------------------------------------------------+
20-
| ``argument`` | Arguments to any function type, including lambdas. |
21-
+--------------------+---------------------------------------------------------------------------------------------------+
22-
| ``variable`` | Local variables in function scopes. |
23-
+--------------------+---------------------------------------------------------------------------------------------------+
24-
| ``class-attribute``| Attributes defined in class bodies. |
25-
+--------------------+---------------------------------------------------------------------------------------------------+
26-
| ``class-const`` | Enum constants and class variables annotated with ``Final`` |
27-
+--------------------+---------------------------------------------------------------------------------------------------+
28-
| ``inlinevar`` | Loop variables in list comprehensions and generator expressions. |
29-
+--------------------+---------------------------------------------------------------------------------------------------+
30-
| ``typevar`` | Type variable declared with ``TypeVar``. |
31-
+--------------------+---------------------------------------------------------------------------------------------------+
32-
| ``typealias`` | Type alias declared with ``TypeAlias`` or assignments of ``Union``. |
33-
+--------------------+---------------------------------------------------------------------------------------------------+
5+
+--------------------+-------------------------------------------------------------------------------------------------------------+
6+
| Name Type | Description |
7+
+====================+=============================================================================================================+
8+
| ``module`` | Module and package names, same as the file names. |
9+
+--------------------+-------------------------------------------------------------------------------------------------------------+
10+
| ``const`` | Module-level constants: any name defined at module level that is not bound to a class object nor reassigned.|
11+
+--------------------+-------------------------------------------------------------------------------------------------------------+
12+
| ``class`` | Names in ``class`` statements, as well as names bound to class objects at module level. |
13+
+--------------------+-------------------------------------------------------------------------------------------------------------+
14+
| ``function`` | Functions, toplevel or nested in functions or methods. |
15+
+--------------------+-------------------------------------------------------------------------------------------------------------+
16+
| ``method`` | Methods, functions defined in class bodies. Includes static and class methods. |
17+
+--------------------+-------------------------------------------------------------------------------------------------------------+
18+
| ``attr`` | Attributes created on class instances inside methods. |
19+
+--------------------+-------------------------------------------------------------------------------------------------------------+
20+
| ``argument`` | Arguments to any function type, including lambdas. |
21+
+--------------------+-------------------------------------------------------------------------------------------------------------+
22+
| ``variable`` | Local variables in function scopes or module-level names that are assigned multiple times. |
23+
+--------------------+-------------------------------------------------------------------------------------------------------------+
24+
| ``class-attribute``| Attributes defined in class bodies. |
25+
+--------------------+-------------------------------------------------------------------------------------------------------------+
26+
| ``class-const`` | Enum constants and class variables annotated with ``Final`` |
27+
+--------------------+-------------------------------------------------------------------------------------------------------------+
28+
| ``inlinevar`` | Loop variables in list comprehensions and generator expressions. |
29+
+--------------------+-------------------------------------------------------------------------------------------------------------+
30+
| ``typevar`` | Type variable declared with ``TypeVar``. |
31+
+--------------------+-------------------------------------------------------------------------------------------------------------+
32+
| ``typealias`` | Type alias declared with ``TypeAlias`` or assignments of ``Union``. |
33+
+--------------------+-------------------------------------------------------------------------------------------------------------+
3434

3535
Default behavior
3636
~~~~~~~~~~~~~~~~
@@ -50,7 +50,7 @@ Following predefined naming styles are available:
5050
* ``UPPER_CASE``
5151
* ``any`` - fake style which does not enforce any limitations
5252

53-
Following options are exposed:
53+
The following options are exposed:
5454

5555
.. option:: --module-naming-style=<style>
5656

‎doc/whatsnew/2/2.5/summary.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Other Changes
6161
* Add new ``good-names-rgx`` and ``bad-names-rgx`` to enable permitting or disallowing of names via regular expressions
6262

6363
To enable better handling of permitted/disallowed names, we added two new config options: good-names-rgxs: a comma-
64-
separated list of regexes, that if a name matches will be exempt of naming-checking. bad-names-rgxs: a comma-
64+
separated list of regexes, that if a name matches will be exempt from naming-checking. bad-names-rgxs: a comma-
6565
separated list of regexes, that if a name matches will be always marked as a disallowed name.
6666

6767
* Mutable ``collections.*`` are now flagged as dangerous defaults.

‎doc/whatsnew/fragments/3585.breaking

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
`invalid-name` now distinguishes module-level constants that are assigned only once
2+
from those that are reassigned and now applies `--variable-rgx` to the latter. Values
3+
other than literals (lists, sets, objects) can pass against either the constant or
4+
variable regexes (e.g. "LOGGER" or "logger" but not "LoGgEr").
5+
6+
Remember that `--good-names` or `--good-names-rgxs` can be provided to explicitly
7+
allow good names.
8+
9+
Closes #3585

‎pylint/checkers/base/name_checker/checker.py

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import astroid
2020
from astroid import nodes
21+
from astroid.typing import InferenceResult
2122

2223
from pylint import constants, interfaces
2324
from pylint.checkers import utils
@@ -399,7 +400,7 @@ def visit_functiondef(self, node: nodes.FunctionDef) -> None:
399400
"typevar-double-variance",
400401
"typevar-name-mismatch",
401402
)
402-
def visit_assignname( # pylint: disable=too-many-branches
403+
def visit_assignname( # pylint: disable=too-many-branches, too-many-statements
403404
self, node: nodes.AssignName
404405
) -> None:
405406
"""Check module level assigned names."""
@@ -461,14 +462,40 @@ def visit_assignname( # pylint: disable=too-many-branches
461462
elif isinstance(inferred_assign_type, nodes.ClassDef):
462463
self._check_name("class", node.name, node)
463464

464-
# Don't emit if the name redefines an import in an ImportError except handler.
465-
elif not _redefines_import(node) and isinstance(
466-
inferred_assign_type, nodes.Const
465+
elif inferred_assign_type in (None, astroid.util.Uninferable):
466+
return
467+
468+
# Don't emit if the name redefines an import in an ImportError except handler
469+
# nor any other reassignment.
470+
elif (
471+
not (redefines_import := _redefines_import(node))
472+
and not isinstance(
473+
inferred_assign_type, (nodes.FunctionDef, nodes.Lambda)
474+
)
475+
and not utils.is_reassigned_before_current(node, node.name)
476+
and not utils.is_reassigned_after_current(node, node.name)
477+
and not utils.get_node_first_ancestor_of_type(
478+
node, (nodes.For, nodes.While)
479+
)
467480
):
468-
self._check_name("const", node.name, node)
481+
if not self._meets_exception_for_non_consts(
482+
inferred_assign_type, node.name
483+
):
484+
self._check_name("const", node.name, node)
469485
else:
486+
node_type = "variable"
487+
if (
488+
(iattrs := tuple(node.frame().igetattr(node.name)))
489+
and astroid.util.Uninferable not in iattrs
490+
and len(iattrs) == 2
491+
and astroid.are_exclusive(*iattrs)
492+
):
493+
node_type = "const"
470494
self._check_name(
471-
"variable", node.name, node, disallowed_check_only=True
495+
node_type,
496+
node.name,
497+
node,
498+
disallowed_check_only=redefines_import,
472499
)
473500

474501
# Check names defined in AnnAssign nodes
@@ -501,6 +528,14 @@ def visit_assignname( # pylint: disable=too-many-branches
501528
else:
502529
self._check_name("class_attribute", node.name, node)
503530

531+
def _meets_exception_for_non_consts(
532+
self, inferred_assign_type: InferenceResult | None, name: str
533+
) -> bool:
534+
if isinstance(inferred_assign_type, nodes.Const):
535+
return False
536+
regexp = self._name_regexps["variable"]
537+
return regexp.match(name) is not None
538+
504539
def _recursive_check_names(self, args: list[nodes.AssignName]) -> None:
505540
"""Check names in a possibly recursive list <arg>."""
506541
for arg in args:
@@ -608,11 +643,11 @@ def _assigns_typevar(node: nodes.NodeNG | None) -> bool:
608643
def _assigns_typealias(node: nodes.NodeNG | None) -> bool:
609644
"""Check if a node is assigning a TypeAlias."""
610645
inferred = utils.safe_infer(node)
611-
if isinstance(inferred, nodes.ClassDef):
646+
if isinstance(inferred, (nodes.ClassDef, astroid.bases.UnionType)):
612647
qname = inferred.qname()
613648
if qname == "typing.TypeAlias":
614649
return True
615-
if qname == ".Union":
650+
if qname in {".Union", "builtins.UnionType"}:
616651
# Union is a special case because it can be used as a type alias
617652
# or as a type annotation. We only want to check the former.
618653
assert node is not None

‎pylint/checkers/utils.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1865,6 +1865,18 @@ def is_sys_guard(node: nodes.If) -> bool:
18651865
return False
18661866

18671867

1868+
def is_reassigned_before_current(node: nodes.NodeNG, varname: str) -> bool:
1869+
"""Check if the given variable name is reassigned in the same scope before the
1870+
current node.
1871+
"""
1872+
return any(
1873+
a.name == varname and a.lineno < node.lineno
1874+
for a in node.scope().nodes_of_class(
1875+
(nodes.AssignName, nodes.ClassDef, nodes.FunctionDef)
1876+
)
1877+
)
1878+
1879+
18681880
def is_reassigned_after_current(node: nodes.NodeNG, varname: str) -> bool:
18691881
"""Check if the given variable name is reassigned in the same scope after the
18701882
current node.
@@ -2135,7 +2147,13 @@ def is_augmented_assign(node: nodes.Assign) -> tuple[bool, str]:
21352147
binop.op in COMMUTATIVE_OPERATORS
21362148
and _is_target_name_in_binop_side(target, binop.right)
21372149
):
2138-
inferred_left = safe_infer(binop.left)
2150+
if isinstance(binop.left, nodes.Const):
2151+
# This bizarrely became necessary after an unrelated call to igetattr().
2152+
# Seems like a code smell uncovered in #10212.
2153+
# tuple(node.frame().igetattr(node.name))
2154+
inferred_left = binop.left
2155+
else:
2156+
inferred_left = safe_infer(binop.left)
21392157
if isinstance(inferred_left, nodes.Const) and isinstance(
21402158
inferred_left.value, int
21412159
):

‎tests/functional/a/arguments.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ def function_default_arg(one=1, two=2):
6262
function_default_arg(1, one=5) # [redundant-keyword-arg]
6363

6464
# Remaining tests are for coverage of correct names in messages.
65-
LAMBDA = lambda arg: 1
65+
my_lambda = lambda arg: 1
6666

67-
LAMBDA() # [no-value-for-parameter]
67+
my_lambda() # [no-value-for-parameter]
6868

6969
def method_tests():
7070
"""Method invocations."""
@@ -260,7 +260,7 @@ def func(one, two, three):
260260
return one + two + three
261261

262262

263-
CALL = lambda *args: func(*args)
263+
call = lambda *args: func(*args)
264264

265265

266266
# Ensure `too-many-function-args` is not emitted when a function call is assigned

‎tests/functional/a/arguments.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ no-value-for-parameter:59:0:59:21::No value for argument 'first_argument' in fun
99
unexpected-keyword-arg:59:0:59:21::Unexpected keyword argument 'bob' in function call:UNDEFINED
1010
unexpected-keyword-arg:60:0:60:40::Unexpected keyword argument 'coin' in function call:UNDEFINED
1111
redundant-keyword-arg:62:0:62:30::Argument 'one' passed by position and keyword in function call:UNDEFINED
12-
no-value-for-parameter:67:0:67:8::No value for argument 'arg' in lambda call:UNDEFINED
12+
no-value-for-parameter:67:0:67:11::No value for argument 'arg' in lambda call:UNDEFINED
1313
no-value-for-parameter:72:4:72:24:method_tests:No value for argument 'arg' in staticmethod call:UNDEFINED
1414
no-value-for-parameter:73:4:73:29:method_tests:No value for argument 'arg' in staticmethod call:UNDEFINED
1515
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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# pylint: disable=useless-return, condition-evals-to-constant
1+
# pylint: disable=useless-return, condition-evals-to-constant, invalid-name
22
"""check assignment to function call where the function doesn't return
33
44
'E1111': ('Assigning to function call which doesn\'t return',

‎tests/functional/c/consider/consider_iterating_dictionary.py

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,17 @@ def keys(self):
2626
(key for key in {}.keys()) # [consider-iterating-dictionary]
2727
{key for key in {}.keys()} # [consider-iterating-dictionary]
2828
{key: key for key in {}.keys()} # [consider-iterating-dictionary]
29-
COMP1 = [key for key in {}.keys()] # [consider-iterating-dictionary]
30-
COMP2 = (key for key in {}.keys()) # [consider-iterating-dictionary]
31-
COMP3 = {key for key in {}.keys()} # [consider-iterating-dictionary]
29+
comp1 = [key for key in {}.keys()] # [consider-iterating-dictionary]
30+
comp2 = (key for key in {}.keys()) # [consider-iterating-dictionary]
31+
comp3 = {key for key in {}.keys()} # [consider-iterating-dictionary]
3232
COMP4 = {key: key for key in {}.keys()} # [consider-iterating-dictionary]
3333
for key in {}.keys(): # [consider-iterating-dictionary]
3434
pass
3535

3636
# Issue #1247
3737
DICT = {'a': 1, 'b': 2}
38-
COMP1 = [k * 2 for k in DICT.keys()] + [k * 3 for k in DICT.keys()] # [consider-iterating-dictionary,consider-iterating-dictionary]
39-
COMP2, COMP3 = [k * 2 for k in DICT.keys()], [k * 3 for k in DICT.keys()] # [consider-iterating-dictionary,consider-iterating-dictionary]
38+
comp1 = [k * 2 for k in DICT.keys()] + [k * 3 for k in DICT.keys()] # [consider-iterating-dictionary,consider-iterating-dictionary]
39+
comp2, comp3 = [k * 2 for k in DICT.keys()], [k * 3 for k in DICT.keys()] # [consider-iterating-dictionary,consider-iterating-dictionary]
4040
SOME_TUPLE = ([k * 2 for k in DICT.keys()], [k * 3 for k in DICT.keys()]) # [consider-iterating-dictionary,consider-iterating-dictionary]
4141

4242
# Checks for membership checks
@@ -62,21 +62,21 @@ def keys(self):
6262
pass
6363
if [1] == dict():
6464
pass
65-
VAR = 1 in {}.keys() # [consider-iterating-dictionary]
66-
VAR = 1 in {}
67-
VAR = 1 in dict()
68-
VAR = [1, 2] == {}.keys() in {False}
65+
var = 1 in {}.keys() # [consider-iterating-dictionary]
66+
var = 1 in {}
67+
var = 1 in dict()
68+
var = [1, 2] == {}.keys() in {False}
6969

7070
# Additional membership checks
7171
# See: https://github.com/pylint-dev/pylint/issues/5323
72-
metadata = {}
73-
if "a" not in list(metadata.keys()): # [consider-iterating-dictionary]
72+
METADATA = {}
73+
if "a" not in list(METADATA.keys()): # [consider-iterating-dictionary]
7474
print(1)
75-
if "a" not in metadata.keys(): # [consider-iterating-dictionary]
75+
if "a" not in METADATA.keys(): # [consider-iterating-dictionary]
7676
print(1)
77-
if "a" in list(metadata.keys()): # [consider-iterating-dictionary]
77+
if "a" in list(METADATA.keys()): # [consider-iterating-dictionary]
7878
print(1)
79-
if "a" in metadata.keys(): # [consider-iterating-dictionary]
79+
if "a" in METADATA.keys(): # [consider-iterating-dictionary]
8080
print(1)
8181

8282

@@ -93,24 +93,24 @@ def inner_function():
9393
return inner_function()
9494
return InnerClass().another_function()
9595

96-
a_dict = {"a": 1, "b": 2, "c": 3}
97-
a_set = {"c", "d"}
96+
A_DICT = {"a": 1, "b": 2, "c": 3}
97+
A_SET = {"c", "d"}
9898

9999
# Test bitwise operations. These should not raise msg because removing `.keys()`
100100
# either gives error or ends in a different result
101-
print(a_dict.keys() | a_set)
101+
print(A_DICT.keys() | A_SET)
102102

103-
if "a" in a_dict.keys() | a_set:
103+
if "a" in A_DICT.keys() | A_SET:
104104
pass
105105

106-
if "a" in a_dict.keys() & a_set:
106+
if "a" in A_DICT.keys() & A_SET:
107107
pass
108108

109-
if 1 in a_dict.keys() ^ [1, 2]:
109+
if 1 in A_DICT.keys() ^ [1, 2]:
110110
pass
111111

112-
if "a" in a_dict.keys() or a_set: # [consider-iterating-dictionary]
112+
if "a" in A_DICT.keys() or A_SET: # [consider-iterating-dictionary]
113113
pass
114114

115-
if "a" in a_dict.keys() and a_set: # [consider-iterating-dictionary]
115+
if "a" in A_DICT.keys() and A_SET: # [consider-iterating-dictionary]
116116
pass

0 commit comments

Comments
 (0)