| # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html |
| # For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE |
| # Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt |
| |
| """Check for signs of poor design.""" |
| |
| from __future__ import annotations |
| |
| import re |
| from collections import defaultdict |
| from collections.abc import Iterator |
| from typing import TYPE_CHECKING |
| |
| import astroid |
| from astroid import nodes |
| |
| from pylint.checkers import BaseChecker |
| from pylint.checkers.utils import is_enum, only_required_for_messages |
| from pylint.typing import MessageDefinitionTuple |
| |
| if TYPE_CHECKING: |
| from pylint.lint import PyLinter |
| |
| MSGS: dict[str, MessageDefinitionTuple] = ( |
| { # pylint: disable=consider-using-namedtuple-or-dataclass |
| "R0901": ( |
| "Too many ancestors (%s/%s)", |
| "too-many-ancestors", |
| "Used when class has too many parent classes, try to reduce " |
| "this to get a simpler (and so easier to use) class.", |
| ), |
| "R0902": ( |
| "Too many instance attributes (%s/%s)", |
| "too-many-instance-attributes", |
| "Used when class has too many instance attributes, try to reduce " |
| "this to get a simpler (and so easier to use) class.", |
| ), |
| "R0903": ( |
| "Too few public methods (%s/%s)", |
| "too-few-public-methods", |
| "Used when class has too few public methods, so be sure it's " |
| "really worth it.", |
| ), |
| "R0904": ( |
| "Too many public methods (%s/%s)", |
| "too-many-public-methods", |
| "Used when class has too many public methods, try to reduce " |
| "this to get a simpler (and so easier to use) class.", |
| ), |
| "R0911": ( |
| "Too many return statements (%s/%s)", |
| "too-many-return-statements", |
| "Used when a function or method has too many return statement, " |
| "making it hard to follow.", |
| ), |
| "R0912": ( |
| "Too many branches (%s/%s)", |
| "too-many-branches", |
| "Used when a function or method has too many branches, " |
| "making it hard to follow.", |
| ), |
| "R0913": ( |
| "Too many arguments (%s/%s)", |
| "too-many-arguments", |
| "Used when a function or method takes too many arguments.", |
| ), |
| "R0914": ( |
| "Too many local variables (%s/%s)", |
| "too-many-locals", |
| "Used when a function or method has too many local variables.", |
| ), |
| "R0915": ( |
| "Too many statements (%s/%s)", |
| "too-many-statements", |
| "Used when a function or method has too many statements. You " |
| "should then split it in smaller functions / methods.", |
| ), |
| "R0916": ( |
| "Too many boolean expressions in if statement (%s/%s)", |
| "too-many-boolean-expressions", |
| "Used when an if statement contains too many boolean expressions.", |
| ), |
| "R0917": ( |
| "Too many positional arguments in a function call.", |
| "too-many-positional", |
| "Will be implemented in https://github.com/pylint-dev/pylint/issues/9099," |
| "msgid/symbol pair reserved for compatibility with ruff, " |
| "see https://github.com/astral-sh/ruff/issues/8946.", |
| ), |
| } |
| ) |
| SPECIAL_OBJ = re.compile("^_{2}[a-z]+_{2}$") |
| DATACLASSES_DECORATORS = frozenset({"dataclass", "attrs"}) |
| DATACLASS_IMPORT = "dataclasses" |
| ATTRS_DECORATORS = frozenset({"define", "frozen"}) |
| ATTRS_IMPORT = "attrs" |
| TYPING_NAMEDTUPLE = "typing.NamedTuple" |
| TYPING_TYPEDDICT = "typing.TypedDict" |
| TYPING_EXTENSIONS_TYPEDDICT = "typing_extensions.TypedDict" |
| |
| # Set of stdlib classes to ignore when calculating number of ancestors |
| STDLIB_CLASSES_IGNORE_ANCESTOR = frozenset( |
| ( |
| "builtins.object", |
| "builtins.tuple", |
| "builtins.dict", |
| "builtins.list", |
| "builtins.set", |
| "bulitins.frozenset", |
| "collections.ChainMap", |
| "collections.Counter", |
| "collections.OrderedDict", |
| "collections.UserDict", |
| "collections.UserList", |
| "collections.UserString", |
| "collections.defaultdict", |
| "collections.deque", |
| "collections.namedtuple", |
| "_collections_abc.Awaitable", |
| "_collections_abc.Coroutine", |
| "_collections_abc.AsyncIterable", |
| "_collections_abc.AsyncIterator", |
| "_collections_abc.AsyncGenerator", |
| "_collections_abc.Hashable", |
| "_collections_abc.Iterable", |
| "_collections_abc.Iterator", |
| "_collections_abc.Generator", |
| "_collections_abc.Reversible", |
| "_collections_abc.Sized", |
| "_collections_abc.Container", |
| "_collections_abc.Collection", |
| "_collections_abc.Set", |
| "_collections_abc.MutableSet", |
| "_collections_abc.Mapping", |
| "_collections_abc.MutableMapping", |
| "_collections_abc.MappingView", |
| "_collections_abc.KeysView", |
| "_collections_abc.ItemsView", |
| "_collections_abc.ValuesView", |
| "_collections_abc.Sequence", |
| "_collections_abc.MutableSequence", |
| "_collections_abc.ByteString", |
| "typing.Tuple", |
| "typing.List", |
| "typing.Dict", |
| "typing.Set", |
| "typing.FrozenSet", |
| "typing.Deque", |
| "typing.DefaultDict", |
| "typing.OrderedDict", |
| "typing.Counter", |
| "typing.ChainMap", |
| "typing.Awaitable", |
| "typing.Coroutine", |
| "typing.AsyncIterable", |
| "typing.AsyncIterator", |
| "typing.AsyncGenerator", |
| "typing.Iterable", |
| "typing.Iterator", |
| "typing.Generator", |
| "typing.Reversible", |
| "typing.Container", |
| "typing.Collection", |
| "typing.AbstractSet", |
| "typing.MutableSet", |
| "typing.Mapping", |
| "typing.MutableMapping", |
| "typing.Sequence", |
| "typing.MutableSequence", |
| "typing.ByteString", |
| "typing.MappingView", |
| "typing.KeysView", |
| "typing.ItemsView", |
| "typing.ValuesView", |
| "typing.ContextManager", |
| "typing.AsyncContextManager", |
| "typing.Hashable", |
| "typing.Sized", |
| TYPING_NAMEDTUPLE, |
| TYPING_TYPEDDICT, |
| TYPING_EXTENSIONS_TYPEDDICT, |
| ) |
| ) |
| |
| |
| def _is_exempt_from_public_methods(node: astroid.ClassDef) -> bool: |
| """Check if a class is exempt from too-few-public-methods.""" |
| # If it's a typing.Namedtuple, typing.TypedDict or an Enum |
| for ancestor in node.ancestors(): |
| if is_enum(ancestor): |
| return True |
| if ancestor.qname() in ( |
| TYPING_NAMEDTUPLE, |
| TYPING_TYPEDDICT, |
| TYPING_EXTENSIONS_TYPEDDICT, |
| ): |
| return True |
| |
| # Or if it's a dataclass |
| if not node.decorators: |
| return False |
| |
| root_locals = set(node.root().locals) |
| for decorator in node.decorators.nodes: |
| if isinstance(decorator, astroid.Call): |
| decorator = decorator.func |
| if not isinstance(decorator, (astroid.Name, astroid.Attribute)): |
| continue |
| if isinstance(decorator, astroid.Name): |
| name = decorator.name |
| else: |
| name = decorator.attrname |
| if name in DATACLASSES_DECORATORS and ( |
| root_locals.intersection(DATACLASSES_DECORATORS) |
| or DATACLASS_IMPORT in root_locals |
| ): |
| return True |
| if name in ATTRS_DECORATORS and ( |
| root_locals.intersection(ATTRS_DECORATORS) or ATTRS_IMPORT in root_locals |
| ): |
| return True |
| return False |
| |
| |
| def _count_boolean_expressions(bool_op: nodes.BoolOp) -> int: |
| """Counts the number of boolean expressions in BoolOp `bool_op` (recursive). |
| |
| example: a and (b or c or (d and e)) ==> 5 boolean expressions |
| """ |
| nb_bool_expr = 0 |
| for bool_expr in bool_op.get_children(): |
| if isinstance(bool_expr, astroid.BoolOp): |
| nb_bool_expr += _count_boolean_expressions(bool_expr) |
| else: |
| nb_bool_expr += 1 |
| return nb_bool_expr |
| |
| |
| def _count_methods_in_class(node: nodes.ClassDef) -> int: |
| all_methods = sum(1 for method in node.methods() if not method.name.startswith("_")) |
| # Special methods count towards the number of public methods, |
| # but don't count towards there being too many methods. |
| for method in node.mymethods(): |
| if SPECIAL_OBJ.search(method.name) and method.name != "__init__": |
| all_methods += 1 |
| return all_methods |
| |
| |
| def _get_parents_iter( |
| node: nodes.ClassDef, ignored_parents: frozenset[str] |
| ) -> Iterator[nodes.ClassDef]: |
| r"""Get parents of ``node``, excluding ancestors of ``ignored_parents``. |
| |
| If we have the following inheritance diagram: |
| |
| F |
| / |
| D E |
| \/ |
| B C |
| \/ |
| A # class A(B, C): ... |
| |
| And ``ignored_parents`` is ``{"E"}``, then this function will return |
| ``{A, B, C, D}`` -- both ``E`` and its ancestors are excluded. |
| """ |
| parents: set[nodes.ClassDef] = set() |
| to_explore = list(node.ancestors(recurs=False)) |
| while to_explore: |
| parent = to_explore.pop() |
| if parent.qname() in ignored_parents: |
| continue |
| if parent not in parents: |
| # This guard might appear to be performing the same function as |
| # adding the resolved parents to a set to eliminate duplicates |
| # (legitimate due to diamond inheritance patterns), but its |
| # additional purpose is to prevent cycles (not normally possible, |
| # but potential due to inference) and thus guarantee termination |
| # of the while-loop |
| yield parent |
| parents.add(parent) |
| to_explore.extend(parent.ancestors(recurs=False)) |
| |
| |
| def _get_parents( |
| node: nodes.ClassDef, ignored_parents: frozenset[str] |
| ) -> set[nodes.ClassDef]: |
| return set(_get_parents_iter(node, ignored_parents)) |
| |
| |
| class MisdesignChecker(BaseChecker): |
| """Checker of potential misdesigns. |
| |
| Checks for sign of poor/misdesign: |
| * number of methods, attributes, local variables... |
| * size, complexity of functions, methods |
| """ |
| |
| # configuration section name |
| name = "design" |
| # messages |
| msgs = MSGS |
| # configuration options |
| options = ( |
| ( |
| "max-args", |
| { |
| "default": 5, |
| "type": "int", |
| "metavar": "<int>", |
| "help": "Maximum number of arguments for function / method.", |
| }, |
| ), |
| ( |
| "max-locals", |
| { |
| "default": 15, |
| "type": "int", |
| "metavar": "<int>", |
| "help": "Maximum number of locals for function / method body.", |
| }, |
| ), |
| ( |
| "max-returns", |
| { |
| "default": 6, |
| "type": "int", |
| "metavar": "<int>", |
| "help": "Maximum number of return / yield for function / " |
| "method body.", |
| }, |
| ), |
| ( |
| "max-branches", |
| { |
| "default": 12, |
| "type": "int", |
| "metavar": "<int>", |
| "help": "Maximum number of branch for function / method body.", |
| }, |
| ), |
| ( |
| "max-statements", |
| { |
| "default": 50, |
| "type": "int", |
| "metavar": "<int>", |
| "help": "Maximum number of statements in function / method body.", |
| }, |
| ), |
| ( |
| "max-parents", |
| { |
| "default": 7, |
| "type": "int", |
| "metavar": "<num>", |
| "help": "Maximum number of parents for a class (see R0901).", |
| }, |
| ), |
| ( |
| "ignored-parents", |
| { |
| "default": (), |
| "type": "csv", |
| "metavar": "<comma separated list of class names>", |
| "help": "List of qualified class names to ignore when counting class parents (see R0901)", |
| }, |
| ), |
| ( |
| "max-attributes", |
| { |
| "default": 7, |
| "type": "int", |
| "metavar": "<num>", |
| "help": "Maximum number of attributes for a class \ |
| (see R0902).", |
| }, |
| ), |
| ( |
| "min-public-methods", |
| { |
| "default": 2, |
| "type": "int", |
| "metavar": "<num>", |
| "help": "Minimum number of public methods for a class \ |
| (see R0903).", |
| }, |
| ), |
| ( |
| "max-public-methods", |
| { |
| "default": 20, |
| "type": "int", |
| "metavar": "<num>", |
| "help": "Maximum number of public methods for a class \ |
| (see R0904).", |
| }, |
| ), |
| ( |
| "max-bool-expr", |
| { |
| "default": 5, |
| "type": "int", |
| "metavar": "<num>", |
| "help": "Maximum number of boolean expressions in an if " |
| "statement (see R0916).", |
| }, |
| ), |
| ( |
| "exclude-too-few-public-methods", |
| { |
| "default": [], |
| "type": "regexp_csv", |
| "metavar": "<pattern>[,<pattern>...]", |
| "help": "List of regular expressions of class ancestor names " |
| "to ignore when counting public methods (see R0903)", |
| }, |
| ), |
| ) |
| |
| def __init__(self, linter: PyLinter) -> None: |
| super().__init__(linter) |
| self._returns: list[int] |
| self._branches: defaultdict[nodes.LocalsDictNodeNG, int] |
| self._stmts: list[int] |
| |
| def open(self) -> None: |
| """Initialize visit variables.""" |
| self.linter.stats.reset_node_count() |
| self._returns = [] |
| self._branches = defaultdict(int) |
| self._stmts = [] |
| self._exclude_too_few_public_methods = ( |
| self.linter.config.exclude_too_few_public_methods |
| ) |
| |
| def _inc_all_stmts(self, amount: int) -> None: |
| for i, _ in enumerate(self._stmts): |
| self._stmts[i] += amount |
| |
| @only_required_for_messages( |
| "too-many-ancestors", |
| "too-many-instance-attributes", |
| "too-few-public-methods", |
| "too-many-public-methods", |
| ) |
| def visit_classdef(self, node: nodes.ClassDef) -> None: |
| """Check size of inheritance hierarchy and number of instance attributes.""" |
| parents = _get_parents( |
| node, |
| STDLIB_CLASSES_IGNORE_ANCESTOR.union(self.linter.config.ignored_parents), |
| ) |
| nb_parents = len(parents) |
| if nb_parents > self.linter.config.max_parents: |
| self.add_message( |
| "too-many-ancestors", |
| node=node, |
| args=(nb_parents, self.linter.config.max_parents), |
| ) |
| |
| # Something at inference time is modifying instance_attrs to add |
| # properties from parent classes. Given how much we cache inference |
| # results, mutating instance_attrs can become a real mess. Filter |
| # them out here until the root cause is solved. |
| # https://github.com/pylint-dev/astroid/issues/2273 |
| root = node.root() |
| filtered_attrs = [ |
| k for (k, v) in node.instance_attrs.items() if v[0].root() is root |
| ] |
| if len(filtered_attrs) > self.linter.config.max_attributes: |
| self.add_message( |
| "too-many-instance-attributes", |
| node=node, |
| args=(len(filtered_attrs), self.linter.config.max_attributes), |
| ) |
| |
| @only_required_for_messages("too-few-public-methods", "too-many-public-methods") |
| def leave_classdef(self, node: nodes.ClassDef) -> None: |
| """Check number of public methods.""" |
| my_methods = sum( |
| 1 for method in node.mymethods() if not method.name.startswith("_") |
| ) |
| |
| # Does the class contain less than n public methods ? |
| # This checks only the methods defined in the current class, |
| # since the user might not have control over the classes |
| # from the ancestors. It avoids some false positives |
| # for classes such as unittest.TestCase, which provides |
| # a lot of assert methods. It doesn't make sense to warn |
| # when the user subclasses TestCase to add his own tests. |
| if my_methods > self.linter.config.max_public_methods: |
| self.add_message( |
| "too-many-public-methods", |
| node=node, |
| args=(my_methods, self.linter.config.max_public_methods), |
| ) |
| |
| # Stop here if the class is excluded via configuration. |
| if node.type == "class" and self._exclude_too_few_public_methods: |
| for ancestor in node.ancestors(): |
| if any( |
| pattern.match(ancestor.qname()) |
| for pattern in self._exclude_too_few_public_methods |
| ): |
| return |
| |
| # Stop here for exception, metaclass, interface classes and other |
| # classes for which we don't need to count the methods. |
| if node.type != "class" or _is_exempt_from_public_methods(node): |
| return |
| |
| # Does the class contain more than n public methods ? |
| # This checks all the methods defined by ancestors and |
| # by the current class. |
| all_methods = _count_methods_in_class(node) |
| if all_methods < self.linter.config.min_public_methods: |
| self.add_message( |
| "too-few-public-methods", |
| node=node, |
| args=(all_methods, self.linter.config.min_public_methods), |
| ) |
| |
| @only_required_for_messages( |
| "too-many-return-statements", |
| "too-many-branches", |
| "too-many-arguments", |
| "too-many-locals", |
| "too-many-statements", |
| "keyword-arg-before-vararg", |
| ) |
| def visit_functiondef(self, node: nodes.FunctionDef) -> None: |
| """Check function name, docstring, arguments, redefinition, |
| variable names, max locals. |
| """ |
| # init branch and returns counters |
| self._returns.append(0) |
| # check number of arguments |
| args = node.args.args + node.args.posonlyargs + node.args.kwonlyargs |
| ignored_argument_names = self.linter.config.ignored_argument_names |
| if args is not None: |
| ignored_args_num = 0 |
| if ignored_argument_names: |
| ignored_args_num = sum( |
| 1 for arg in args if ignored_argument_names.match(arg.name) |
| ) |
| |
| argnum = len(args) - ignored_args_num |
| if argnum > self.linter.config.max_args: |
| self.add_message( |
| "too-many-arguments", |
| node=node, |
| args=(len(args), self.linter.config.max_args), |
| ) |
| else: |
| ignored_args_num = 0 |
| # check number of local variables |
| locnum = len(node.locals) - ignored_args_num |
| |
| # decrement number of local variables if '_' is one of them |
| if "_" in node.locals: |
| locnum -= 1 |
| |
| if locnum > self.linter.config.max_locals: |
| self.add_message( |
| "too-many-locals", |
| node=node, |
| args=(locnum, self.linter.config.max_locals), |
| ) |
| # init new statements counter |
| self._stmts.append(1) |
| |
| visit_asyncfunctiondef = visit_functiondef |
| |
| @only_required_for_messages( |
| "too-many-return-statements", |
| "too-many-branches", |
| "too-many-arguments", |
| "too-many-locals", |
| "too-many-statements", |
| ) |
| def leave_functiondef(self, node: nodes.FunctionDef) -> None: |
| """Most of the work is done here on close: |
| checks for max returns, branch, return in __init__. |
| """ |
| returns = self._returns.pop() |
| if returns > self.linter.config.max_returns: |
| self.add_message( |
| "too-many-return-statements", |
| node=node, |
| args=(returns, self.linter.config.max_returns), |
| ) |
| branches = self._branches[node] |
| if branches > self.linter.config.max_branches: |
| self.add_message( |
| "too-many-branches", |
| node=node, |
| args=(branches, self.linter.config.max_branches), |
| ) |
| # check number of statements |
| stmts = self._stmts.pop() |
| if stmts > self.linter.config.max_statements: |
| self.add_message( |
| "too-many-statements", |
| node=node, |
| args=(stmts, self.linter.config.max_statements), |
| ) |
| |
| leave_asyncfunctiondef = leave_functiondef |
| |
| def visit_return(self, _: nodes.Return) -> None: |
| """Count number of returns.""" |
| if not self._returns: |
| return # return outside function, reported by the base checker |
| self._returns[-1] += 1 |
| |
| def visit_default(self, node: nodes.NodeNG) -> None: |
| """Default visit method -> increments the statements counter if |
| necessary. |
| """ |
| if node.is_statement: |
| self._inc_all_stmts(1) |
| |
| def visit_try(self, node: nodes.Try) -> None: |
| """Increments the branches counter.""" |
| branches = len(node.handlers) |
| if node.orelse: |
| branches += 1 |
| if node.finalbody: |
| branches += 1 |
| self._inc_branch(node, branches) |
| self._inc_all_stmts(branches) |
| |
| @only_required_for_messages("too-many-boolean-expressions", "too-many-branches") |
| def visit_if(self, node: nodes.If) -> None: |
| """Increments the branches counter and checks boolean expressions.""" |
| self._check_boolean_expressions(node) |
| branches = 1 |
| # don't double count If nodes coming from some 'elif' |
| if node.orelse and ( |
| len(node.orelse) > 1 or not isinstance(node.orelse[0], astroid.If) |
| ): |
| branches += 1 |
| self._inc_branch(node, branches) |
| self._inc_all_stmts(branches) |
| |
| def _check_boolean_expressions(self, node: nodes.If) -> None: |
| """Go through "if" node `node` and count its boolean expressions |
| if the 'if' node test is a BoolOp node. |
| """ |
| condition = node.test |
| if not isinstance(condition, astroid.BoolOp): |
| return |
| nb_bool_expr = _count_boolean_expressions(condition) |
| if nb_bool_expr > self.linter.config.max_bool_expr: |
| self.add_message( |
| "too-many-boolean-expressions", |
| node=condition, |
| args=(nb_bool_expr, self.linter.config.max_bool_expr), |
| ) |
| |
| def visit_while(self, node: nodes.While) -> None: |
| """Increments the branches counter.""" |
| branches = 1 |
| if node.orelse: |
| branches += 1 |
| self._inc_branch(node, branches) |
| |
| visit_for = visit_while |
| |
| def _inc_branch(self, node: nodes.NodeNG, branchesnum: int = 1) -> None: |
| """Increments the branches counter.""" |
| self._branches[node.scope()] += branchesnum |
| |
| |
| def register(linter: PyLinter) -> None: |
| linter.register_checker(MisdesignChecker(linter)) |