diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index c0865d0c6d..e0eb7bd208 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -518,6 +518,8 @@ contributors: * Marco Gorelli: contributor - Documented Jupyter integration +* Rebecca Turner (9999years): contributor + * Yilei Yang: contributor * Marcin Kurczewski (rr-): contributor diff --git a/ChangeLog b/ChangeLog index 047ee37095..65b03de5f6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -8,6 +8,11 @@ What's New in Pylint 2.10.0? ============================ Release date: TBA +* Added ``ignored-parents`` option to the design checker to ignore specific + classes from the ``too-many-ancestors`` check (R0901). + + Partially closes #3057 + .. Put new features here and also in 'doc/whatsnew/2.10.rst' .. diff --git a/doc/whatsnew/2.10.rst b/doc/whatsnew/2.10.rst index e94530bf11..8980b0f91e 100644 --- a/doc/whatsnew/2.10.rst +++ b/doc/whatsnew/2.10.rst @@ -24,3 +24,5 @@ Other Changes * Performance of the Similarity checker has been improved. * Added ``time.clock`` to deprecated functions/methods for python 3.3 +* Added ``ignored-parents`` option to the design checker to ignore specific + classes from the ``too-many-ancestors`` check (R0901). diff --git a/pylint/checkers/design_analysis.py b/pylint/checkers/design_analysis.py index a029694102..26cd77cc52 100644 --- a/pylint/checkers/design_analysis.py +++ b/pylint/checkers/design_analysis.py @@ -25,6 +25,7 @@ import re from collections import defaultdict +from typing import FrozenSet, List, Set, cast import astroid from astroid import nodes @@ -237,6 +238,35 @@ def _count_methods_in_class(node): return all_methods +def _get_parents( + node: nodes.ClassDef, ignored_parents: FrozenSet[str] +) -> Set[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 = cast(List[nodes.ClassDef], list(node.ancestors(recurs=False))) + while to_explore: + parent = to_explore.pop() + if parent.qname() in ignored_parents: + continue + parents.add(parent) + to_explore.extend(parent.ancestors(recurs=False)) # type: ignore + return parents + + class MisdesignChecker(BaseChecker): """checks for sign of poor/misdesign: * number of methods, attributes, local variables... @@ -307,6 +337,15 @@ class MisdesignChecker(BaseChecker): "help": "Maximum number of parents for a class (see R0901).", }, ), + ( + "ignored-parents", + { + "default": (), + "type": "csv", + "metavar": "", + "help": "List of qualified class names to ignore when countint class parents (see R0901)", + }, + ), ( "max-attributes", { @@ -379,11 +418,10 @@ def _ignored_argument_names(self): ) def visit_classdef(self, node: nodes.ClassDef): """check size of inheritance hierarchy and number of instance attributes""" - nb_parents = sum( - 1 - for ancestor in node.ancestors() - if ancestor.qname() not in STDLIB_CLASSES_IGNORE_ANCESTOR + parents = _get_parents( + node, STDLIB_CLASSES_IGNORE_ANCESTOR.union(self.config.ignored_parents) ) + nb_parents = len(parents) if nb_parents > self.config.max_parents: self.add_message( "too-many-ancestors", diff --git a/pylintrc b/pylintrc index e488b1354b..5b2bc31dea 100644 --- a/pylintrc +++ b/pylintrc @@ -324,6 +324,9 @@ max-statements=100 # Maximum number of parents for a class (see R0901). max-parents=7 +# List of qualified class names to ignore when counting class parents (see R0901). +ignored-parents= + # Maximum number of attributes for a class (see R0902). max-attributes=11 diff --git a/tests/checkers/unittest_design.py b/tests/checkers/unittest_design.py new file mode 100644 index 0000000000..e3d4519903 --- /dev/null +++ b/tests/checkers/unittest_design.py @@ -0,0 +1,41 @@ +# Copyright (c) 2021 Rebecca Turner + +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE + + +import astroid + +from pylint.checkers import design_analysis +from pylint.testutils import CheckerTestCase, set_config + + +class TestDesignChecker(CheckerTestCase): + + CHECKER_CLASS = design_analysis.MisdesignChecker + + @set_config( + ignored_parents=(".Dddd",), + max_parents=1, + ) + def test_too_many_ancestors_ignored_parents_are_skipped(self): + """Make sure that classes listed in ``ignored-parents`` aren't counted + by the too-many-ancestors message. + """ + + node = astroid.extract_node( + """ + class Aaaa(object): + pass + class Bbbb(Aaaa): + pass + class Cccc(Bbbb): + pass + class Dddd(Cccc): + pass + class Eeee(Dddd): + pass + """ + ) + with self.assertNoMessages(): + self.checker.visit_classdef(node) diff --git a/tests/functional/t/too/too_many_ancestors_ignored_parents.py b/tests/functional/t/too/too_many_ancestors_ignored_parents.py new file mode 100644 index 0000000000..93598d941c --- /dev/null +++ b/tests/functional/t/too/too_many_ancestors_ignored_parents.py @@ -0,0 +1,40 @@ +# pylint: disable=missing-docstring, too-few-public-methods, invalid-name + +# Inheritance diagram: +# F +# / +# D E +# \/ +# B C +# \/ +# A +# +# Once `E` is pruned from the tree, we have: +# D +# \ +# B C +# \/ +# A +# +# By setting `max-parents=2`, we're able to check that tree-pruning works +# correctly; in the new diagram, `B` has only 1 parent, so it doesn't raise a +# message, and `A` has 3, so it does raise a message with the specific number +# of parents. + +class F: + """0 parents""" + +class E(F): + """1 parent""" + +class D: + """0 parents""" + +class B(D, E): + """3 parents""" + +class C: + """0 parents""" + +class A(B, C): # [too-many-ancestors] + """5 parents""" diff --git a/tests/functional/t/too/too_many_ancestors_ignored_parents.rc b/tests/functional/t/too/too_many_ancestors_ignored_parents.rc new file mode 100644 index 0000000000..1d06dad25d --- /dev/null +++ b/tests/functional/t/too/too_many_ancestors_ignored_parents.rc @@ -0,0 +1,3 @@ +[testoptions] +max-parents=2 +ignored-parents=functional.t.too.too_many_ancestors_ignored_parents.E diff --git a/tests/functional/t/too/too_many_ancestors_ignored_parents.txt b/tests/functional/t/too/too_many_ancestors_ignored_parents.txt new file mode 100644 index 0000000000..e1eea426a1 --- /dev/null +++ b/tests/functional/t/too/too_many_ancestors_ignored_parents.txt @@ -0,0 +1 @@ +too-many-ancestors:39:0:A:Too many ancestors (3/2)