Refactor flake8 plugin checks and improve test cases

Author: vrslev

src/community_of_python_flake8_plugin/checks/disabled/module_import_many_names.py

@@ -1,4 +1,9 @@
-"""Check is disabled because it doesn't work when flake8 invoked directly, not as python module (.venv/bin/flake8 doesn't work, python -m flake8 works)."""
+"""Check is disabled because it doesn't work when flake8 invoked directly, not as python module.
+
+(.venv/bin/flake8 doesn't work, python -m flake8 works).
+
+
+"""
 
 from __future__ import annotations
 import ast
@@ -32,12 +37,12 @@ def check_module_path_exists(module_name: str) -> bool:
             return importlib_util.find_spec(module_name) is not None
 
         parent, _, _child = module_name.rpartition(".")
-        parent_spec = importlib_util.find_spec(parent)  # does not execute parent
+        parent_spec: typing.Final = importlib_util.find_spec(parent)  # does not execute parent
         if parent_spec is None:
             return False
 
         # Parent must be a package (have places to search for submodules)
-        locations = parent_spec.submodule_search_locations
+        locations: typing.Final = parent_spec.submodule_search_locations
         if locations is None:
             return False
 

src/community_of_python_flake8_plugin/checks/function_verb.py

@@ -26,8 +26,7 @@ def check_is_property(function_node: ast.AST) -> bool:
     return any(check_is_property_decorator(decorator) for decorator in function_node.decorator_list)  # noqa: COP011
 
 
-def check_is_property_decorator(decorator: ast.expr) -> bool:
-    # Handle direct name references like @property or @cached_property
+def check_is_property_decorator(decorator: ast.expr) -> bool:  # noqa: PLR0911
     if isinstance(decorator, ast.Name):
         return decorator.id in {"property", "cached_property"}
 
@@ -42,9 +41,13 @@ def check_is_property_decorator(decorator: ast.expr) -> bool:
         if isinstance(decorator.func, ast.Name):
             return decorator.func.id in {"property", "cached_property"}
         if isinstance(decorator.func, ast.Attribute):
+            if (
+                decorator.func.attr in {"property", "setter", "cached_property"}
+                and isinstance(decorator.func.value, ast.Name)
+                and decorator.func.value.id == "functools"
+            ):
+                return decorator.func.attr == "cached_property"
             if decorator.func.attr in {"property", "setter", "cached_property"}:
-                if isinstance(decorator.func.value, ast.Name) and decorator.func.value.id == "functools":
-                    return decorator.func.attr == "cached_property"
                 return decorator.func.attr in {"property", "setter"}
 
     return False

src/community_of_python_flake8_plugin/checks/name_length.py

@@ -92,13 +92,14 @@ def validate_name_length(self, identifier: str, ast_node: ast.stmt, parent_class
 
         if len(identifier) < MIN_NAME_LENGTH:
             # Determine if this is an attribute (inside a class) or variable (at module level)
-            is_attribute = parent_class is not None
             self.violations.append(
                 Violation(
                     line_number=ast_node.lineno,
                     column_number=ast_node.col_offset,
                     violation_code=(
-                        ViolationCodes.ATTRIBUTE_NAME_LENGTH if is_attribute else ViolationCodes.VARIABLE_NAME_LENGTH
+                        ViolationCodes.ATTRIBUTE_NAME_LENGTH
+                        if parent_class is not None
+                        else ViolationCodes.VARIABLE_NAME_LENGTH
                     ),
                 )
             )

src/community_of_python_flake8_plugin/checks/temp_var.py

@@ -7,24 +7,21 @@
 from community_of_python_flake8_plugin.violations import Violation
 
 
-def collect_variable_usage_and_stores(function_node: ast.AST) -> tuple[dict[str, list[ast.Name]], set[str]]:
-    variable_usage: typing.Final[defaultdict[str, list[ast.Name]]] = defaultdict(list)
-    assigned_variable_names: typing.Final[set[str]] = set()
+def is_tuple_unpacking(assign_node: ast.Assign) -> bool:
+    return bool(assign_node.targets and isinstance(assign_node.targets[0], ast.Tuple))
+
 
-    def extract_names(expression: ast.expr) -> typing.Iterable[str]:
-        if isinstance(expression, ast.Name):
-            yield expression.id
-        elif isinstance(expression, ast.Tuple):
-            for elt in expression.elts:
-                yield from extract_names(elt)
+def extract_names(expression: ast.expr) -> typing.Iterable[str]:
+    if isinstance(expression, ast.Name):
+        yield expression.id
+    elif isinstance(expression, ast.Tuple):
+        for elt in expression.elts:
+            yield from extract_names(elt)
 
-    def is_tuple_unpacking(assign_node: ast.Assign) -> bool:
-        """Check if this is a tuple unpacking assignment."""
-        if not assign_node.targets:
-            return False
 
-        target = assign_node.targets[0]
-        return isinstance(target, ast.Tuple)
+def collect_variable_usage_and_stores(function_node: ast.AST) -> tuple[dict[str, list[ast.Name]], set[str]]:
+    variable_usage: typing.Final[defaultdict[str, list[ast.Name]]] = defaultdict(list)
+    assigned_variable_names: typing.Final[set[str]] = set()
 
     @typing.final
     class UsageCollector(ast.NodeVisitor):
@@ -34,9 +31,12 @@ def visit_Name(self, name_node: ast.Name) -> None:
 
         def visit_Assign(self, assign_node: ast.Assign) -> None:
             # Skip collecting variables from tuple unpacking assignments
-            if not is_tuple_unpacking(assign_node):
-                for target in assign_node.targets:
-                    assigned_variable_names.update(extract_names(target))
+            if is_tuple_unpacking(assign_node):
+                self.generic_visit(assign_node)
+                return
+
+            for target in assign_node.targets:
+                assigned_variable_names.update(extract_names(target))
             self.generic_visit(assign_node)
 
         def visit_AugAssign(self, aug_assign_node: ast.AugAssign) -> None:

tests/test_plugin.py

@@ -184,7 +184,8 @@ def test_type_annotation_validations(input_source: str, expected_output: list[st
         ),
         # No violation: ModelFactory methods should be exempt from COP009
         (
-            "from polyfactory.factories.pydantic_factory import ModelFactory\nclass MyFactory(ModelFactory):\n    def calculator(self): pass",
+            "from polyfactory.factories.pydantic_factory import ModelFactory\n"
+            "class MyFactory(ModelFactory):\n    def calculator(self): pass",
             ["COP012"],
         ),
         # No violation: ModelFactory generic methods should be exempt from COP009
@@ -205,21 +206,20 @@ def test_type_annotation_validations(input_source: str, expected_output: list[st
             "class MyFactory(ModelFactory[some_module.SomeClass]):\n    @classmethod\n    def create(cls): pass",
             ["COP012"],
         ),
-        # No violation: cached_property imported directly should exempt function from COP009 (but triggers COP002 for import style)
+        # No violation: cached_property imported directly should exempt function from COP009
+        # (but triggers COP002 for import style)
         (
-            "from functools import cached_property\nclass ExampleClass:\n    @cached_property\n    def calculator(self): pass",
+            "from functools import cached_property\n"
+            "class ExampleClass:\n    @cached_property\n    def calculator(self): pass",
             ["COP002", "COP012"],
         ),
-        # No violation: cached_property() imported directly should exempt function from COP009 (but triggers COP002)
+        # No violation: cached_property() imported directly should exempt function from COP009
+        # (but triggers COP002)
         (
-            "from functools import cached_property\nclass ExampleClass:\n    @cached_property()\n    def calculator(self): pass",
+            "from functools import cached_property\n"
+            "class ExampleClass:\n    @cached_property()\n    def calculator(self): pass",
             ["COP002", "COP012"],
         ),
-        # No violation: pytest fixture with name parameter should be exempt from COP009
-        (
-            "import pytest\n@pytest.fixture(name='events')\ndef fixture_events() -> list[dict]:\n    return []",
-            [],
-        ),
         # COP009: faker.Faker annotation doesn't exempt function naming rules
         (
             "import faker\ndef some_func(arg: faker.Faker): pass",