Skip to content

Commit

Permalink
Issue 3292: fix WPS605 false-positive on @staticmethod usage (#3294)
Browse files Browse the repository at this point in the history
Co-authored-by: sobolevn <[email protected]>
  • Loading branch information
kekekekule and sobolevn authored Jan 30, 2025
1 parent 8a8de3b commit ec56951
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ This version introduces `wps` CLI tool.
- Fixes `WPS115` false-positive on `Enum` attributes, #3238
- Removes duplicated `WPS312`, #3239
- Fixes `WPS432`, now it shows literal num, #1402
- Fixes `WPS605` false-positive on `@staticmethod`, #3292

## 1.0.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from wemake_python_styleguide.violations.oop import (
MethodWithoutArgumentsViolation,
StaticMethodViolation,
)
from wemake_python_styleguide.visitors.ast.classes import WrongMethodVisitor

Expand All @@ -10,6 +11,12 @@ class Example:
def some_name({0}): ...
"""

staticmethod_inside_class = """
class Example:
@staticmethod
def some_name({0}): ...
"""

regular_function = 'def some_name({0}): ...'


Expand Down Expand Up @@ -45,10 +52,27 @@ def test_function_without_arguments(
assert_errors(visitor, [])


def test_staticmethod_without_arguments(
assert_errors,
parse_ast_tree,
mode,
default_options,
):
"""Testing that no arguments for method raises a violation."""
tree = parse_ast_tree(mode(staticmethod_inside_class.format('')))

visitor = WrongMethodVisitor(default_options, tree=tree)
visitor.run()

# will error on @staticmethod presence, but not on arguments absence
assert_errors(visitor, [], ignored_types=StaticMethodViolation)


@pytest.mark.parametrize(
'code',
[
method_inside_class,
staticmethod_inside_class,
regular_function,
],
)
Expand Down Expand Up @@ -78,4 +102,4 @@ def test_with_arguments(
visitor = WrongMethodVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [])
assert_errors(visitor, [], ignored_types=StaticMethodViolation)
11 changes: 11 additions & 0 deletions wemake_python_styleguide/logic/tree/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
'typing_extensions.overload',
))

_STATICMETHOD_NAMES: Final = frozenset(('staticmethod',))


def given_function_called(
node: ast.Call,
Expand Down Expand Up @@ -103,3 +105,12 @@ def is_overload(node: ast.AST) -> bool:
if source.node_to_string(decorator) in _OVERLOAD_EXCEPTIONS:
return True
return False


def is_staticmethod(node: AnyFunctionDef) -> bool:
"""Check that function decorated with @staticmethod."""
for decorator in node.decorator_list:
decorator_name = getattr(decorator, 'id', None)
if decorator_name in _STATICMETHOD_NAMES:
return True
return False
4 changes: 3 additions & 1 deletion wemake_python_styleguide/violations/oop.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ class MethodWithoutArgumentsViolation(ASTViolation):
Reasoning:
Methods without arguments are allowed to be defined,
but almost impossible to use.
but almost impossible to use, if they are not @staticmethods.
Furthermore, they don't have an access to ``self``,
so cannot access the inner state of the object.
It might be an intentional design or just a typo.
Expand All @@ -271,6 +271,8 @@ def method(): ...
.. versionadded:: 0.7.0
.. versionchanged:: 0.11.0
.. versionchanged:: 1.1.0
Allows usage of ``@staticmethod`` with no arguments.
"""

Expand Down
12 changes: 3 additions & 9 deletions wemake_python_styleguide/visitors/ast/classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,12 @@ def _check_getters_setters_methods(self, node: ast.ClassDef) -> None:
class WrongMethodVisitor(base.BaseNodeVisitor):
"""Visits functions, but treats them as methods."""

_staticmethod_names: ClassVar[frozenset[str]] = frozenset(('staticmethod',))
_special_async_iter: ClassVar[frozenset[str]] = frozenset(('__aiter__',))

def visit_any_function(self, node: types.AnyFunctionDef) -> None:
"""Checking class methods: async and regular."""
node_context = nodes.get_context(node)
if isinstance(node_context, ast.ClassDef):
self._check_decorators(node)
self._check_bound_methods(node)
self._check_yield_magic_methods(node)
self._check_async_magic_methods(node)
Expand All @@ -163,14 +161,10 @@ def visit_any_function(self, node: types.AnyFunctionDef) -> None:
)
self.generic_visit(node)

def _check_decorators(self, node: types.AnyFunctionDef) -> None:
for decorator in node.decorator_list:
decorator_name = getattr(decorator, 'id', None)
if decorator_name in self._staticmethod_names:
self.add_violation(oop.StaticMethodViolation(node))

def _check_bound_methods(self, node: types.AnyFunctionDef) -> None:
if not functions.get_all_arguments(node):
if functions.is_staticmethod(node):
self.add_violation(oop.StaticMethodViolation(node))
elif not functions.get_all_arguments(node):
self.add_violation(
oop.MethodWithoutArgumentsViolation(node, text=node.name),
)
Expand Down

0 comments on commit ec56951

Please sign in to comment.