Skip to content

Commit

Permalink
Add ignored-parents option to design checker (#4758)
Browse files Browse the repository at this point in the history
* Add ignored-parents option to design checker

This allows users to specify classes to ignore while counting parent
classes.

Partially closes #3057

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Jul 28, 2021
1 parent 67f4056 commit 24d03e9
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CONTRIBUTORS.txt
Expand Up @@ -518,6 +518,8 @@ contributors:
* Marco Gorelli: contributor
- Documented Jupyter integration

* Rebecca Turner (9999years): contributor

* Yilei Yang: contributor

* Marcin Kurczewski (rr-): contributor
Expand Down
5 changes: 5 additions & 0 deletions ChangeLog
Expand Up @@ -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'
..
Expand Down
2 changes: 2 additions & 0 deletions doc/whatsnew/2.10.rst
Expand Up @@ -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).
46 changes: 42 additions & 4 deletions pylint/checkers/design_analysis.py
Expand Up @@ -25,6 +25,7 @@

import re
from collections import defaultdict
from typing import FrozenSet, List, Set, cast

import astroid
from astroid import nodes
Expand Down Expand Up @@ -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...
Expand Down Expand Up @@ -307,6 +337,15 @@ class MisdesignChecker(BaseChecker):
"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 countint class parents (see R0901)",
},
),
(
"max-attributes",
{
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions pylintrc
Expand Up @@ -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

Expand Down
41 changes: 41 additions & 0 deletions tests/checkers/unittest_design.py
@@ -0,0 +1,41 @@
# Copyright (c) 2021 Rebecca Turner <rturner@starry.com>

# 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)
40 changes: 40 additions & 0 deletions 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"""
3 changes: 3 additions & 0 deletions 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
@@ -0,0 +1 @@
too-many-ancestors:39:0:A:Too many ancestors (3/2)

0 comments on commit 24d03e9

Please sign in to comment.