From c654b91dfd027f88e452e5c75483c47706171b53 Mon Sep 17 00:00:00 2001 From: Lev Vereshchagin Date: Wed, 11 Feb 2026 14:20:04 +0300 Subject: [PATCH 1/5] Enhance final class check and for-loop validation logic --- .../checks/final_class.py | 21 ++++++++++- .../checks/for_loop_one_prefix.py | 34 ++++++++++++++--- tests/test_plugin.py | 37 +++++++++++++++++++ 3 files changed, 85 insertions(+), 7 deletions(-) diff --git a/src/community_of_python_flake8_plugin/checks/final_class.py b/src/community_of_python_flake8_plugin/checks/final_class.py index 1da75b2..513dc58 100644 --- a/src/community_of_python_flake8_plugin/checks/final_class.py +++ b/src/community_of_python_flake8_plugin/checks/final_class.py @@ -40,9 +40,24 @@ def is_model_factory_class(class_node: ast.ClassDef) -> bool: return check_inherits_from_bases(class_node, {"ModelFactory", "SQLAlchemyFactory"}) +def has_inherited_classes(syntax_tree: ast.AST, class_node: ast.ClassDef) -> bool: + """Check if there are classes that inherit from this class.""" + for node in ast.walk(syntax_tree): + if isinstance(node, ast.ClassDef) and node != class_node: + for base in node.bases: + # Check for direct class reference: class Child(Parent): + if isinstance(base, ast.Name) and base.id == class_node.name: + return True + # Check for attributed class reference: class Child(module.Parent): + if isinstance(base, ast.Attribute) and base.attr == class_node.name: + return True + return False + + @typing.final class FinalClassCheck(ast.NodeVisitor): - def __init__(self, syntax_tree: ast.AST) -> None: # noqa: ARG002 + def __init__(self, syntax_tree: ast.AST) -> None: + self.syntax_tree = syntax_tree self.violations: list[Violation] = [] def visit_ClassDef(self, ast_node: ast.ClassDef) -> None: @@ -54,6 +69,10 @@ def _check_final_decorator(self, ast_node: ast.ClassDef) -> None: if is_protocol_class(ast_node) or ast_node.name.startswith("Test") or is_model_factory_class(ast_node): return + # If there are classes inheriting from this class, don't require the decorator + if has_inherited_classes(self.syntax_tree, ast_node): + return + if not contains_final_decorator(ast_node): self.violations.append( Violation( diff --git a/src/community_of_python_flake8_plugin/checks/for_loop_one_prefix.py b/src/community_of_python_flake8_plugin/checks/for_loop_one_prefix.py index 4f46dce..1416f95 100644 --- a/src/community_of_python_flake8_plugin/checks/for_loop_one_prefix.py +++ b/src/community_of_python_flake8_plugin/checks/for_loop_one_prefix.py @@ -22,14 +22,14 @@ def visit_ListComp(self, ast_node: ast.ListComp) -> None: # Validate targets in generators (the 'v' in 'for v in lst') for one_comprehension in ast_node.generators: if not self._is_partial_unpacking(ast_node.elt, one_comprehension.target): - self._validate_comprehension_target(one_comprehension.target) + self._validate_comprehension_target(one_comprehension.target, one_comprehension.iter) self.generic_visit(ast_node) def visit_SetComp(self, ast_node: ast.SetComp) -> None: # Validate targets in generators (the 'v' in 'for v in lst') for one_comprehension in ast_node.generators: if not self._is_partial_unpacking(ast_node.elt, one_comprehension.target): - self._validate_comprehension_target(one_comprehension.target) + self._validate_comprehension_target(one_comprehension.target, one_comprehension.iter) self.generic_visit(ast_node) def visit_DictComp(self, ast_node: ast.DictComp) -> None: @@ -38,7 +38,7 @@ def visit_DictComp(self, ast_node: ast.DictComp) -> None: # key and value are both used for one_comprehension in ast_node.generators: if not self._is_partial_unpacking_expr_count(2, one_comprehension.target): - self._validate_comprehension_target(one_comprehension.target) + self._validate_comprehension_target(one_comprehension.target, one_comprehension.iter) self.generic_visit(ast_node) def visit_For(self, ast_node: ast.For) -> None: @@ -46,14 +46,14 @@ def visit_For(self, ast_node: ast.For) -> None: # Apply same unpacking logic as comprehensions # For-loops don't have an expression that references vars if not self._is_partial_unpacking_expr_count(1, ast_node.target): - self._validate_comprehension_target(ast_node.target) + self._validate_comprehension_target(ast_node.target, ast_node.iter) self.generic_visit(ast_node) def visit_GeneratorExp(self, ast_node: ast.GeneratorExp) -> None: # Validate targets in generators (the 'v' in 'for v in lst') for one_comprehension in ast_node.generators: if not self._is_partial_unpacking(ast_node.elt, one_comprehension.target): - self._validate_comprehension_target(one_comprehension.target) + self._validate_comprehension_target(one_comprehension.target, one_comprehension.iter) self.generic_visit(ast_node) def _is_partial_unpacking(self, expression: ast.expr, target_node: ast.expr) -> bool: @@ -65,6 +65,24 @@ def _is_partial_unpacking_expr_count(self, expression_count: int, target_node: a target_count: typing.Final = self._count_unpacked_vars(target_node) return target_count > expression_count and target_count > 1 + def _is_literal_range(self, iter_node: ast.expr) -> bool: + """Check if the iteration is over a literal range() call.""" + # Check for direct range() call + if isinstance(iter_node, ast.Call): + if isinstance(iter_node.func, ast.Name) and iter_node.func.id == "range": + # Check if all arguments are literals (no variables) + for arg in iter_node.args: + if not isinstance(arg, (ast.Constant, ast.UnaryOp)): + # If any argument is not a literal, this is not a literal range + # Note: UnaryOp is included to handle negative numbers like -1 + return False + # For UnaryOp (like -1), check if operand is a literal + if isinstance(arg, ast.UnaryOp): + if not isinstance(arg.operand, ast.Constant): + return False + return True + return False + def _count_referenced_vars(self, expression: ast.expr) -> int: """Count how many variables are referenced in the expression.""" if isinstance(expression, ast.Name): @@ -83,8 +101,12 @@ def _count_unpacked_vars(self, target_node: ast.expr) -> int: return len([one_element for one_element in target_node.elts if isinstance(one_element, ast.Name)]) return 0 - def _validate_comprehension_target(self, target_node: ast.expr) -> None: + def _validate_comprehension_target(self, target_node: ast.expr, iter_node: ast.expr | None = None) -> None: """Validate that comprehension target follows the one_ prefix rule.""" + # Skip validation if iterating over literal range() + if iter_node is not None and self._is_literal_range(iter_node): + return + # Skip ignored targets (underscore, unpacking) if _is_ignored_target(target_node): return diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 71bb976..4ed47e3 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -422,6 +422,21 @@ def test_variable_usage_validations(input_source: str, expected_output: list[str "import typing\nclass MyProtocol(typing.Protocol, object):\n def fetch_value(self) -> int: ...\n", [], ), + # No violation: Classes with inherited classes don't require @typing.final + ( + "class ParentClass:\n pass\n\nclass ChildClass(ParentClass):\n pass", + ["COP012"], # Only ParentClass should require final decorator + ), + # No violation: Classes with multiple levels of inheritance + ( + "class GrandParentClass:\n pass\n\nclass ParentClass(GrandParentClass):\n pass\n\nclass ChildClass(ParentClass):\n pass", + ["COP012"], # Only GrandParentClass should require final decorator + ), + # No violation: Classes with inherited classes using module notation + ( + "class ParentClass:\n pass\n\nclass ChildClass(module.ParentClass):\n pass", + ["COP012"], # Only ParentClass should require final decorator + ), ], ) def test_class_validations(input_source: str, expected_output: list[str]) -> None: @@ -605,6 +620,28 @@ def test_dataclass_validations(input_source: str, expected_output: list[str]) -> ("for x, y in pairs: pass", []), # No violation: Regular for-loop with one_ prefix ("for one_x in some_list: pass", []), + # No violation: Regular for-loop over literal range() without one_ prefix + ("for i in range(10): pass", []), + # No violation: Regular for-loop over literal range() with start and stop + ("for i in range(5, 10): pass", []), + # No violation: Regular for-loop over literal range() with start, stop, and step + ("for i in range(0, 10, 2): pass", []), + # No violation: Regular for-loop over literal range() with negative values + ("for i in range(-5, 5): pass", []), + # COP015: Regular for-loop over non-literal range() should still require one_ prefix + ("for i in range(some_variable): pass", ["COP015"]), + # COP015: Regular for-loop over non-literal range() with multiple variables should still require one_ prefix + ("for i in range(start, stop): pass", ["COP015"]), + # No violation: List comprehension over literal range() without one_ prefix + ("result = [i for i in range(10)]", []), + # COP015: List comprehension over non-literal range() should still require one_ prefix + ("result = [i for i in range(variable)]", ["COP015"]), + # No violation: Set comprehension over literal range() without one_ prefix + ("result = {i for i in range(10)}", []), + # No violation: Dict comprehension over literal range() without one_ prefix + ("result = {i: i for i in range(10)}", []), + # No violation: Generator expression over literal range() without one_ prefix + ("result = (i for i in range(10))", []), ], ) def test_module_vs_class_level_assignments(input_source: str, expected_output: list[str]) -> None: From 0febc429fe0ef52d96d90474fa364b4e109dd6fb Mon Sep 17 00:00:00 2001 From: Lev Vereshchagin Date: Wed, 11 Feb 2026 14:44:42 +0300 Subject: [PATCH 2/5] Update test cases for variable usage and dataclass validations --- tests/test_plugin.py | 57 +++++++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 4ed47e3..489b0c8 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -422,20 +422,43 @@ def test_variable_usage_validations(input_source: str, expected_output: list[str "import typing\nclass MyProtocol(typing.Protocol, object):\n def fetch_value(self) -> int: ...\n", [], ), - # No violation: Classes with inherited classes don't require @typing.final + # No violation: Child classes don't require @typing.final since they inherit from other classes ( "class ParentClass:\n pass\n\nclass ChildClass(ParentClass):\n pass", - ["COP012"], # Only ParentClass should require final decorator + ["COP012"], # Only ParentClass should require final decorator, ChildClass inherits so it's exempt ), - # No violation: Classes with multiple levels of inheritance + # No violation: Multiple levels of inheritance ( "class GrandParentClass:\n pass\n\nclass ParentClass(GrandParentClass):\n pass\n\nclass ChildClass(ParentClass):\n pass", - ["COP012"], # Only GrandParentClass should require final decorator + ["COP012"], # Only GrandParentClass requires final decorator, ParentClass and ChildClass inherit so they're exempt ), - # No violation: Classes with inherited classes using module notation + # No violation: Child classes with module notation don't require @typing.final ( "class ParentClass:\n pass\n\nclass ChildClass(module.ParentClass):\n pass", - ["COP012"], # Only ParentClass should require final decorator + ["COP012"], # Only ParentClass should require final decorator, ChildClass inherits so it's exempt + ), + # No violation: Parent class with local subclass doesn't require final decorator + ( + "import typing\n\n" + "@typing.final\nclass ParentClass:\n pass\n\n" + "class ChildClass(ParentClass):\n pass", + [], # No violations - ParentClass is properly marked final + ), + # No violation: Complex inheritance hierarchy with proper final decorators + ( + "import typing\n\n" + "@typing.final\nclass BaseClass:\n pass\n\n" + "class MiddleClass(BaseClass):\n pass\n\n" + "class DerivedClass(MiddleClass):\n pass", + [], # No violations - BaseClass is properly marked final + ), + # No violation: Multiple inheritance with local subclasses + ( + "import typing\n\n" + "@typing.final\nclass FirstParent:\n pass\n\n" + "@typing.final\nclass SecondParent:\n pass\n\n" + "class ChildClass(FirstParent, SecondParent):\n pass", + [], # No violations - parent classes are properly marked final ), ], ) @@ -621,27 +644,27 @@ def test_dataclass_validations(input_source: str, expected_output: list[str]) -> # No violation: Regular for-loop with one_ prefix ("for one_x in some_list: pass", []), # No violation: Regular for-loop over literal range() without one_ prefix - ("for i in range(10): pass", []), + ("for cur_number in range(10): pass", []), # No violation: Regular for-loop over literal range() with start and stop - ("for i in range(5, 10): pass", []), + ("for cur_number in range(5, 10): pass", []), # No violation: Regular for-loop over literal range() with start, stop, and step - ("for i in range(0, 10, 2): pass", []), + ("for cur_number in range(0, 10, 2): pass", []), # No violation: Regular for-loop over literal range() with negative values - ("for i in range(-5, 5): pass", []), + ("for cur_number in range(-5, 5): pass", []), # COP015: Regular for-loop over non-literal range() should still require one_ prefix - ("for i in range(some_variable): pass", ["COP015"]), + ("for cur_number in range(some_variable): pass", ["COP015"]), # COP015: Regular for-loop over non-literal range() with multiple variables should still require one_ prefix - ("for i in range(start, stop): pass", ["COP015"]), + ("for cur_number in range(start, stop): pass", ["COP015"]), # No violation: List comprehension over literal range() without one_ prefix - ("result = [i for i in range(10)]", []), + ("my_result = [cur_number for cur_number in range(10)]", []), # COP015: List comprehension over non-literal range() should still require one_ prefix - ("result = [i for i in range(variable)]", ["COP015"]), + ("my_result = [cur_number for cur_number in range(variable)]", ["COP015"]), # No violation: Set comprehension over literal range() without one_ prefix - ("result = {i for i in range(10)}", []), + ("my_result = {cur_number for cur_number in range(10)}", []), # No violation: Dict comprehension over literal range() without one_ prefix - ("result = {i: i for i in range(10)}", []), + ("my_result = {cur_number: cur_number for cur_number in range(10)}", []), # No violation: Generator expression over literal range() without one_ prefix - ("result = (i for i in range(10))", []), + ("my_result = (cur_number for cur_number in range(10))", []), ], ) def test_module_vs_class_level_assignments(input_source: str, expected_output: list[str]) -> None: From 3c77d5b1ff504c1a8b59db7802d0f0f7a9e839f6 Mon Sep 17 00:00:00 2001 From: Lev Vereshchagin Date: Wed, 11 Feb 2026 14:44:48 +0300 Subject: [PATCH 3/5] Rename function to clarify local subclass checking --- .../checks/final_class.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/community_of_python_flake8_plugin/checks/final_class.py b/src/community_of_python_flake8_plugin/checks/final_class.py index 513dc58..f034899 100644 --- a/src/community_of_python_flake8_plugin/checks/final_class.py +++ b/src/community_of_python_flake8_plugin/checks/final_class.py @@ -40,8 +40,8 @@ def is_model_factory_class(class_node: ast.ClassDef) -> bool: return check_inherits_from_bases(class_node, {"ModelFactory", "SQLAlchemyFactory"}) -def has_inherited_classes(syntax_tree: ast.AST, class_node: ast.ClassDef) -> bool: - """Check if there are classes that inherit from this class.""" +def has_local_subclasses(syntax_tree: ast.AST, class_node: ast.ClassDef) -> bool: + """Check if there are classes in the same file that inherit from this class.""" for node in ast.walk(syntax_tree): if isinstance(node, ast.ClassDef) and node != class_node: for base in node.bases: @@ -69,8 +69,8 @@ def _check_final_decorator(self, ast_node: ast.ClassDef) -> None: if is_protocol_class(ast_node) or ast_node.name.startswith("Test") or is_model_factory_class(ast_node): return - # If there are classes inheriting from this class, don't require the decorator - if has_inherited_classes(self.syntax_tree, ast_node): + # If there are classes in this file that inherit from this class, don't require the decorator + if has_local_subclasses(self.syntax_tree, ast_node): return if not contains_final_decorator(ast_node): From e3b8b33d65abefde654bb1d3cd02f19991605699 Mon Sep 17 00:00:00 2001 From: Lev Vereshchagin Date: Wed, 11 Feb 2026 14:50:29 +0300 Subject: [PATCH 4/5] Update test cases to properly validate final decorator usage in inheritance hierarchies --- tests/test_plugin.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 489b0c8..ffeee26 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -437,28 +437,28 @@ def test_variable_usage_validations(input_source: str, expected_output: list[str "class ParentClass:\n pass\n\nclass ChildClass(module.ParentClass):\n pass", ["COP012"], # Only ParentClass should require final decorator, ChildClass inherits so it's exempt ), - # No violation: Parent class with local subclass doesn't require final decorator + # No violation: Child class properly inherits, parent doesn't need final decorator ( "import typing\n\n" - "@typing.final\nclass ParentClass:\n pass\n\n" - "class ChildClass(ParentClass):\n pass", - [], # No violations - ParentClass is properly marked final + "class ParentClass:\n pass\n\n" + "@typing.final\nclass ChildClass(ParentClass):\n pass", + [], # No violations - ChildClass is properly marked final ), # No violation: Complex inheritance hierarchy with proper final decorators ( "import typing\n\n" - "@typing.final\nclass BaseClass:\n pass\n\n" + "class BaseClass:\n pass\n\n" "class MiddleClass(BaseClass):\n pass\n\n" - "class DerivedClass(MiddleClass):\n pass", - [], # No violations - BaseClass is properly marked final + "@typing.final\nclass DerivedClass(MiddleClass):\n pass", + [], # No violations - derived classes are properly marked final ), - # No violation: Multiple inheritance with local subclasses + # No violation: Multiple inheritance with proper final decorators ( "import typing\n\n" - "@typing.final\nclass FirstParent:\n pass\n\n" - "@typing.final\nclass SecondParent:\n pass\n\n" - "class ChildClass(FirstParent, SecondParent):\n pass", - [], # No violations - parent classes are properly marked final + "class FirstParent:\n pass\n\n" + "class SecondParent:\n pass\n\n" + "@typing.final\nclass ChildClass(FirstParent, SecondParent):\n pass", + [], # No violations - ChildClass is properly marked final ), ], ) From 2d53afcd9cb380f6334ff212e9fbabb91caf326f Mon Sep 17 00:00:00 2001 From: Lev Vereshchagin Date: Wed, 11 Feb 2026 14:53:35 +0300 Subject: [PATCH 5/5] Refactor variable names and simplify range check logic --- .../checks/final_class.py | 10 +++---- .../checks/for_loop_one_prefix.py | 26 +++++++++---------- tests/test_plugin.py | 10 +++---- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/community_of_python_flake8_plugin/checks/final_class.py b/src/community_of_python_flake8_plugin/checks/final_class.py index f034899..5842e9b 100644 --- a/src/community_of_python_flake8_plugin/checks/final_class.py +++ b/src/community_of_python_flake8_plugin/checks/final_class.py @@ -42,14 +42,14 @@ def is_model_factory_class(class_node: ast.ClassDef) -> bool: def has_local_subclasses(syntax_tree: ast.AST, class_node: ast.ClassDef) -> bool: """Check if there are classes in the same file that inherit from this class.""" - for node in ast.walk(syntax_tree): - if isinstance(node, ast.ClassDef) and node != class_node: - for base in node.bases: + for one_node in ast.walk(syntax_tree): + if isinstance(one_node, ast.ClassDef) and one_node != class_node: + for one_base in one_node.bases: # Check for direct class reference: class Child(Parent): - if isinstance(base, ast.Name) and base.id == class_node.name: + if isinstance(one_base, ast.Name) and one_base.id == class_node.name: return True # Check for attributed class reference: class Child(module.Parent): - if isinstance(base, ast.Attribute) and base.attr == class_node.name: + if isinstance(one_base, ast.Attribute) and one_base.attr == class_node.name: return True return False diff --git a/src/community_of_python_flake8_plugin/checks/for_loop_one_prefix.py b/src/community_of_python_flake8_plugin/checks/for_loop_one_prefix.py index 1416f95..91ce668 100644 --- a/src/community_of_python_flake8_plugin/checks/for_loop_one_prefix.py +++ b/src/community_of_python_flake8_plugin/checks/for_loop_one_prefix.py @@ -68,19 +68,17 @@ def _is_partial_unpacking_expr_count(self, expression_count: int, target_node: a def _is_literal_range(self, iter_node: ast.expr) -> bool: """Check if the iteration is over a literal range() call.""" # Check for direct range() call - if isinstance(iter_node, ast.Call): - if isinstance(iter_node.func, ast.Name) and iter_node.func.id == "range": - # Check if all arguments are literals (no variables) - for arg in iter_node.args: - if not isinstance(arg, (ast.Constant, ast.UnaryOp)): - # If any argument is not a literal, this is not a literal range - # Note: UnaryOp is included to handle negative numbers like -1 - return False - # For UnaryOp (like -1), check if operand is a literal - if isinstance(arg, ast.UnaryOp): - if not isinstance(arg.operand, ast.Constant): - return False - return True + if isinstance(iter_node, ast.Call) and isinstance(iter_node.func, ast.Name) and iter_node.func.id == "range": + # Check if all arguments are literals (no variables) + for one_arg in iter_node.args: + if not isinstance(one_arg, (ast.Constant, ast.UnaryOp)): + # If any argument is not a literal, this is not a literal range + # Note: UnaryOp is included to handle negative numbers like -1 + return False + # For UnaryOp (like -1), check if operand is a literal + if isinstance(one_arg, ast.UnaryOp) and not isinstance(one_arg.operand, ast.Constant): + return False + return True return False def _count_referenced_vars(self, expression: ast.expr) -> int: @@ -106,7 +104,7 @@ def _validate_comprehension_target(self, target_node: ast.expr, iter_node: ast.e # Skip validation if iterating over literal range() if iter_node is not None and self._is_literal_range(iter_node): return - + # Skip ignored targets (underscore, unpacking) if _is_ignored_target(target_node): return diff --git a/tests/test_plugin.py b/tests/test_plugin.py index ffeee26..ca06ef1 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -429,8 +429,10 @@ def test_variable_usage_validations(input_source: str, expected_output: list[str ), # No violation: Multiple levels of inheritance ( - "class GrandParentClass:\n pass\n\nclass ParentClass(GrandParentClass):\n pass\n\nclass ChildClass(ParentClass):\n pass", - ["COP012"], # Only GrandParentClass requires final decorator, ParentClass and ChildClass inherit so they're exempt + "class GrandParentClass:\n pass\n\nclass ParentClass(GrandParentClass):\n pass\n\n" + "class ChildClass(ParentClass):\n pass", + ["COP012"], # Only GrandParentClass requires final decorator, ParentClass and + # ChildClass inherit so they're exempt ), # No violation: Child classes with module notation don't require @typing.final ( @@ -439,9 +441,7 @@ def test_variable_usage_validations(input_source: str, expected_output: list[str ), # No violation: Child class properly inherits, parent doesn't need final decorator ( - "import typing\n\n" - "class ParentClass:\n pass\n\n" - "@typing.final\nclass ChildClass(ParentClass):\n pass", + "import typing\n\nclass ParentClass:\n pass\n\n@typing.final\nclass ChildClass(ParentClass):\n pass", [], # No violations - ChildClass is properly marked final ), # No violation: Complex inheritance hierarchy with proper final decorators