diff --git a/README.rst b/README.rst index 159e290..a900ff8 100644 --- a/README.rst +++ b/README.rst @@ -297,13 +297,16 @@ and/or ignored codes. Configuration ------------- -The plugin currently has one setting: +The plugin currently has the following settings: ``extend-immutable-calls``: Specify a list of additional immutable calls. This could be useful, when using other libraries that provide more immutable calls, beside those already handled by ``flake8-bugbear``. Calls to these method will no longer raise a ``B008`` warning. +``classmethod-decorators``: Specify a list of decorators to additionally mark a method as a ``classmethod`` as used by B902. Default values are ``classmethod, validator, root_validator``, and when an ``@obj.name`` decorator is specified it will match against either ``name`` or ``obj.name``. +This functions similarly to how `pep8-naming ` handles it, but with different defaults, and they don't support specifying attributes such that a decorator will never match against a specified value ``obj.name`` even if decorated with ``@obj.name``. + For example:: [flake8] @@ -311,6 +314,7 @@ For example:: max-complexity = 12 ... extend-immutable-calls = pathlib.Path, Path + classmethod-decorators = myclassmethod, mylibrary.otherclassmethod Tests / Lints --------------- diff --git a/bugbear.py b/bugbear.py index 2366bbf..1cec5bd 100644 --- a/bugbear.py +++ b/bugbear.py @@ -10,6 +10,7 @@ from contextlib import suppress from functools import lru_cache, partial from keyword import iskeyword +from typing import Dict, List, Set, Union import attr import pycodestyle @@ -38,6 +39,8 @@ "assertWarnsRegex", } +B902_default_decorators = {"classmethod", "validator", "root_validator"} + Context = namedtuple("Context", ["node", "stack"]) @@ -62,10 +65,15 @@ def run(self): else: b008_extend_immutable_calls = set() + b902_classmethod_decorators: set[str] = B902_default_decorators + if self.options and hasattr(self.options, "classmethod_decorators"): + b902_classmethod_decorators = set(self.options.classmethod_decorators) + visitor = self.visitor( filename=self.filename, lines=self.lines, b008_extend_immutable_calls=b008_extend_immutable_calls, + b902_classmethod_decorators=b902_classmethod_decorators, ) visitor.visit(self.tree) for e in itertools.chain(visitor.errors, self.gen_line_based_checks()): @@ -143,6 +151,19 @@ def add_options(optmanager): default=[], help="Skip B008 test for additional immutable calls.", ) + # you cannot register the same option in two different plugins, so we + # only register --classmethod-decorators if pep8-naming is not installed + if "pep8ext_naming" not in sys.modules.keys(): + optmanager.add_option( + "--classmethod-decorators", + comma_separated_list=True, + parse_from_config=True, + default=B902_default_decorators, + help=( + "List of method decorators that should be treated as classmethods" + " by B902" + ), + ) @lru_cache # noqa: B019 def should_warn(self, code): @@ -308,6 +329,7 @@ class BugBearVisitor(ast.NodeVisitor): filename = attr.ib() lines = attr.ib() b008_extend_immutable_calls = attr.ib(default=attr.Factory(set)) + b902_classmethod_decorators = attr.ib(default=attr.Factory(set)) node_window = attr.ib(default=attr.Factory(list)) errors = attr.ib(default=attr.Factory(list)) futures = attr.ib(default=attr.Factory(set)) @@ -971,37 +993,51 @@ def check_for_b901(self, node): self.errors.append(B901(return_node.lineno, return_node.col_offset)) break - def check_for_b902(self, node): + # taken from pep8-naming + @classmethod + def find_decorator_name(cls, d): + if isinstance(d, ast.Name): + return d.id + elif isinstance(d, ast.Attribute): + return d.attr + elif isinstance(d, ast.Call): + return cls.find_decorator_name(d.func) + + def check_for_b902( # noqa: C901 (too complex) + self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef] + ) -> None: + def is_classmethod(decorators: Set[str]) -> bool: + return ( + any(name in decorators for name in self.b902_classmethod_decorators) + or node.name in B902.implicit_classmethods + ) + if len(self.contexts) < 2 or not isinstance( self.contexts[-2].node, ast.ClassDef ): return cls = self.contexts[-2].node - decorators = NameFinder() - decorators.visit(node.decorator_list) - if "staticmethod" in decorators.names: + decorators: set[str] = { + self.find_decorator_name(d) for d in node.decorator_list + } + + if "staticmethod" in decorators: # TODO: maybe warn if the first argument is surprisingly `self` or # `cls`? return bases = {b.id for b in cls.bases if isinstance(b, ast.Name)} if "type" in bases: - if ( - "classmethod" in decorators.names - or node.name in B902.implicit_classmethods - ): + if is_classmethod(decorators): expected_first_args = B902.metacls kind = "metaclass class" else: expected_first_args = B902.cls kind = "metaclass instance" else: - if ( - "classmethod" in decorators.names - or node.name in B902.implicit_classmethods - ): + if is_classmethod(decorators): expected_first_args = B902.cls kind = "class" else: @@ -1438,9 +1474,11 @@ class NameFinder(ast.NodeVisitor): key is name string, value is the node (useful for location purposes). """ - names = attr.ib(default=attr.Factory(dict)) + names: Dict[str, List[ast.Name]] = attr.ib(default=attr.Factory(dict)) - def visit_Name(self, node): # noqa: B906 # names don't contain other names + def visit_Name( # noqa: B906 # names don't contain other names + self, node: ast.Name + ): self.names.setdefault(node.id, []).append(node) def visit(self, node): diff --git a/tests/b902_extended.py b/tests/b902_extended.py new file mode 100644 index 0000000..b458f1f --- /dev/null +++ b/tests/b902_extended.py @@ -0,0 +1,104 @@ +# parameters: --classmethod-decorators=["mylibrary.classmethod", "validator"] +class Errors: + # correctly registered as classmethod + @validator + def foo_validator(self) -> None: ... + + @other.validator + def foo_other_validator(self) -> None: ... + + @foo.bar.validator + def foo_foo_bar_validator(self) -> None: ... + + @validator.blah + def foo_validator_blah(cls) -> None: ... + + # specifying attribute in options is not valid + @mylibrary.makeclassmethod + def foo2(cls) -> None: ... + + # specified attribute in options + @makeclassmethod + def foo6(cls) -> None: ... + + # classmethod is default, but if not specified it's ignored + @classmethod + def foo3(cls) -> None: ... + + # random unknown decorator + @aoeuaoeu + def foo5(cls) -> None: ... + + +class NoErrors: + @validator + def foo1(cls) -> None: ... + + @other.validator + def foo4(cls) -> None: ... + + @mylibrary.makeclassmethod + def foo2(self) -> None: ... + + @classmethod + def foo3(self) -> None: ... + + @aoeuaoeu + def foo5(self) -> None: ... + + @makeclassmethod + def foo6(self) -> None: ... + + +# Above tests, duplicated to check that the separate logic for metaclasses also works + + +class ErrorsMeta(type): + # correctly registered as classmethod + @validator + def foo_validator(cls) -> None: ... + + @other.validator + def foo_other_validator(cls) -> None: ... + + @foo.bar.validator + def foo_foo_bar_validator(cls) -> None: ... + + @validator.blah + def foo_validator_blah(metacls) -> None: ... + + # specifying attribute in options is not valid + @mylibrary.makeclassmethod + def foo2(metacls) -> None: ... + + # specified attribute in options + @makeclassmethod + def foo6(metacls) -> None: ... + + # classmethod is default, but if not specified it's ignored + @classmethod + def foo3(metacls) -> None: ... + + # random unknown decorator + @aoeuaoeu + def foo5(metacls) -> None: ... + + +class NoErrorsMeta(type): + @validator + def foo1(metacls) -> None: ... + + @other.validator + def foo4(metacls) -> None: ... + + @mylibrary.makeclassmethod + def foo2(cls) -> None: ... + + @classmethod + def foo3(cls) -> None: ... + + @aoeuaoeu + def foo5(cls) -> None: ... + + @makeclassmethod + def foo6(cls) -> None: ... diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index cc93e24..77d953c 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -667,6 +667,39 @@ def test_b902(self): ), ) + def test_b902_extended(self): + filename = Path(__file__).absolute().parent / "b902_extended.py" + + mock_options = Namespace( + classmethod_decorators=["mylibrary.makeclassmethod", "validator"], + select=["B902"], + ) + bbc = BugBearChecker(filename=str(filename), options=mock_options) + errors = list(bbc.run()) + + self.assertEqual( + errors, + self.errors( + B902(5, 22, vars=("'self'", "class", "cls")), + B902(8, 28, vars=("'self'", "class", "cls")), + B902(11, 30, vars=("'self'", "class", "cls")), + B902(14, 27, vars=("'cls'", "instance", "self")), + B902(18, 13, vars=("'cls'", "instance", "self")), + B902(22, 13, vars=("'cls'", "instance", "self")), + B902(26, 13, vars=("'cls'", "instance", "self")), + B902(30, 13, vars=("'cls'", "instance", "self")), + # metaclass + B902(59, 22, vars=("'cls'", "metaclass class", "metacls")), + B902(62, 28, vars=("'cls'", "metaclass class", "metacls")), + B902(65, 30, vars=("'cls'", "metaclass class", "metacls")), + B902(68, 27, vars=("'metacls'", "metaclass instance", "cls")), + B902(72, 13, vars=("'metacls'", "metaclass instance", "cls")), + B902(76, 13, vars=("'metacls'", "metaclass instance", "cls")), + B902(80, 13, vars=("'metacls'", "metaclass instance", "cls")), + B902(84, 13, vars=("'metacls'", "metaclass instance", "cls")), + ), + ) + def test_b902_py38(self): filename = Path(__file__).absolute().parent / "b902_py38.py" bbc = BugBearChecker(filename=str(filename)) diff --git a/tox.ini b/tox.ini index e27c4fc..183e397 100644 --- a/tox.ini +++ b/tox.ini @@ -1,14 +1,14 @@ # The test environment and commands [tox] # default environments to run without `-e` -envlist = py38, py39, py310, py311, py312 +envlist = py38, py39, py310, py311, py312, pep8_naming [gh-actions] python = 3.8: py38 3.9: py39 3.10: py310 - 3.11: py311 + 3.11: py311,pep8_naming 3.12: py312 [testenv] @@ -21,6 +21,16 @@ commands = coverage run tests/test_bugbear.py {posargs} coverage report -m +[testenv:pep8_naming] +deps = + coverage + hypothesis + hypothesmith + pep8-naming +commands = + coverage run tests/test_bugbear.py -k b902 {posargs} + coverage report -m + [testenv:py312] deps = # the other dependencies aren't yet installable on py312+