Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/10790.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix a false positive for ``invalid-name`` where a dataclass field typed with ``Final``
was evaluated against the ``class_const`` regex instead of the ``class_attribute`` regex.

Closes #10790
11 changes: 8 additions & 3 deletions pylint/checkers/base/name_checker/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,14 @@ def visit_assignname( # pylint: disable=too-many-branches,too-many-statements
elif isinstance(frame, nodes.ClassDef) and not any(
frame.local_attr_ancestors(node.name)
):
if utils.is_enum_member(node) or utils.is_assign_name_annotated_with(
node, "Final"
):
if utils.is_assign_name_annotated_with_class_var_typing_name(node, "Final"):
self._check_name("class_const", node.name, node)
elif utils.is_assign_name_annotated_with(node, "Final"):
if frame.is_dataclass:
self._check_name("class_attribute", node.name, node)
else:
self._check_name("class_const", node.name, node)
elif utils.is_enum_member(node):
self._check_name("class_const", node.name, node)
else:
self._check_name("class_attribute", node.name, node)
Expand Down
16 changes: 16 additions & 0 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1762,6 +1762,22 @@ def is_assign_name_annotated_with(node: nodes.AssignName, typing_name: str) -> b
return False


def is_assign_name_annotated_with_class_var_typing_name(
node: nodes.AssignName, typing_name: str
) -> bool:
if not is_assign_name_annotated_with(node, "ClassVar"):
return False
annotation = node.parent.annotation
if isinstance(annotation, nodes.Subscript):
annotation = annotation.slice
if isinstance(annotation, nodes.Subscript):
annotation = annotation.value
match annotation:
case nodes.Name(name=n) | nodes.Attribute(attrname=n) if n == typing_name:
return True
return False


def get_iterating_dictionary_name(node: nodes.For | nodes.Comprehension) -> str | None:
"""Get the name of the dictionary which keys are being iterated over on
a ``nodes.For`` or ``nodes.Comprehension`` node.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# pylint: disable=missing-class-docstring, missing-function-docstring

from dataclasses import dataclass
from typing import Final
from typing import ClassVar, Final

module_snake_case_constant: Final[int] = 42 # [invalid-name]
MODULE_UPPER_CASE_CONSTANT: Final[int] = 42
Expand All @@ -17,8 +17,11 @@ def function() -> None:

@dataclass
class Class:
class_snake_case_constant: Final[int] = 42 # [invalid-name]
CLASS_UPPER_CASE_CONSTANT: Final[int] = 42
class_snake_case_constant: ClassVar[Final[int]] = 42 # [invalid-name]
CLASS_UPPER_CASE_CONSTANT: ClassVar[Final[int]] = 42

field_annotated_with_final: Final[int] = 42
FIELD_ANNOTATED_WITH_FINAL: Final[int] = 42 # this could emit invalid-name eventually.
Comment on lines +23 to +24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading PEP591 I felt that Final was supposed to indicate a constant. Note that most of the Final annotated variable are in UPPERCASE here : https://peps.python.org/pep-0591/#id2 (with a notable exception of the mutable list and a 2 int vector example). Also here in the "updated documentation" linked in PEP 591 : https://docs.python.org/3/library/typing.html#typing.Final. Could you explain your reasoning for forbidding uppercase ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I can remove the editorial comment. I don't think there's any plan to forbid upper cased final variables.

I replied to the reporter, if it helps:

I'm a little surprised we enforce snake_case for function variables annotated with Final, since it seems like they should be UPPER_CASE, which would conflict with the example here, but I take it we should just be liberal and allow either.


def method(self) -> None:
method_snake_case_constant: Final[int] = 42
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
invalid-name:8:0:8:26::"Constant name ""module_snake_case_constant"" doesn't conform to UPPER_CASE naming style":HIGH
invalid-name:14:4:14:32:function:"Variable name ""FUNCTION_UPPER_CASE_CONSTANT"" doesn't conform to snake_case naming style":HIGH
invalid-name:20:4:20:29:Class:"Class constant name ""class_snake_case_constant"" doesn't conform to UPPER_CASE naming style":HIGH
invalid-name:25:8:25:34:Class.method:"Variable name ""METHOD_UPPER_CASE_CONSTANT"" doesn't conform to snake_case naming style":HIGH
invalid-name:28:8:28:34:Class.method:"Variable name ""METHOD_UPPER_CASE_CONSTANT"" doesn't conform to snake_case naming style":HIGH