You cannot select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
364 lines
15 KiB
Python
364 lines
15 KiB
Python
# Copyright (c) 2003-2013 LOGILAB S.A. (Paris, FRANCE).
|
|
# http://www.logilab.fr/ -- mailto:contact@logilab.fr
|
|
#
|
|
# This program is free software; you can redistribute it and/or modify it under
|
|
# the terms of the GNU General Public License as published by the Free Software
|
|
# Foundation; either version 2 of the License, or (at your option) any later
|
|
# version.
|
|
#
|
|
# This program is distributed in the hope that it will be useful, but WITHOUT
|
|
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
|
|
# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
|
|
#
|
|
# You should have received a copy of the GNU General Public License along with
|
|
# this program; if not, write to the Free Software Foundation, Inc.,
|
|
# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
|
|
"""check for signs of poor design"""
|
|
|
|
import re
|
|
from collections import defaultdict
|
|
|
|
from astroid import Function, If, InferenceError
|
|
|
|
from pylint.interfaces import IAstroidChecker
|
|
from pylint.checkers import BaseChecker
|
|
from pylint.checkers.utils import check_messages
|
|
|
|
# regexp for ignored argument name
|
|
IGNORED_ARGUMENT_NAMES = re.compile('_.*')
|
|
|
|
|
|
def class_is_abstract(klass):
|
|
"""return true if the given class node should be considered as an abstract
|
|
class
|
|
"""
|
|
for attr in klass.values():
|
|
if isinstance(attr, Function):
|
|
if attr.is_abstract(pass_is_abstract=False):
|
|
return True
|
|
return False
|
|
|
|
|
|
MSGS = {
|
|
'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.'),
|
|
|
|
'R0921': ('Abstract class not referenced',
|
|
'abstract-class-not-used',
|
|
'Used when an abstract class is not used as ancestor anywhere.'),
|
|
'R0922': ('Abstract class is only referenced %s times',
|
|
'abstract-class-little-used',
|
|
'Used when an abstract class is used less than X times as \
|
|
ancestor.'),
|
|
'R0923': ('Interface not implemented',
|
|
'interface-not-implemented',
|
|
'Used when an interface class is not implemented anywhere.'),
|
|
}
|
|
|
|
|
|
class MisdesignChecker(BaseChecker):
|
|
"""checks for sign of poor/misdesign:
|
|
* number of methods, attributes, local variables...
|
|
* size, complexity of functions, methods
|
|
"""
|
|
|
|
__implements__ = (IAstroidChecker,)
|
|
|
|
# configuration section name
|
|
name = 'design'
|
|
# messages
|
|
msgs = MSGS
|
|
priority = -2
|
|
# configuration options
|
|
options = (('max-args',
|
|
{'default' : 5, 'type' : 'int', 'metavar' : '<int>',
|
|
'help': 'Maximum number of arguments for function / method'}
|
|
),
|
|
('ignored-argument-names',
|
|
{'default' : IGNORED_ARGUMENT_NAMES,
|
|
'type' :'regexp', 'metavar' : '<regexp>',
|
|
'help' : 'Argument names that match this expression will be '
|
|
'ignored. Default to name with leading underscore'}
|
|
),
|
|
('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).'}
|
|
),
|
|
('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).'}
|
|
),
|
|
)
|
|
|
|
def __init__(self, linter=None):
|
|
BaseChecker.__init__(self, linter)
|
|
self.stats = None
|
|
self._returns = None
|
|
self._branches = None
|
|
self._used_abstracts = None
|
|
self._used_ifaces = None
|
|
self._abstracts = None
|
|
self._ifaces = None
|
|
self._stmts = 0
|
|
|
|
def open(self):
|
|
"""initialize visit variables"""
|
|
self.stats = self.linter.add_stats()
|
|
self._returns = []
|
|
self._branches = defaultdict(int)
|
|
self._used_abstracts = {}
|
|
self._used_ifaces = {}
|
|
self._abstracts = []
|
|
self._ifaces = []
|
|
|
|
# Check 'R0921', 'R0922', 'R0923'
|
|
def close(self):
|
|
"""check that abstract/interface classes are used"""
|
|
for abstract in self._abstracts:
|
|
if not abstract in self._used_abstracts:
|
|
self.add_message('abstract-class-not-used', node=abstract)
|
|
elif self._used_abstracts[abstract] < 2:
|
|
self.add_message('abstract-class-little-used', node=abstract,
|
|
args=self._used_abstracts[abstract])
|
|
for iface in self._ifaces:
|
|
if not iface in self._used_ifaces:
|
|
self.add_message('interface-not-implemented', node=iface)
|
|
|
|
@check_messages('too-many-ancestors', 'too-many-instance-attributes',
|
|
'too-few-public-methods', 'too-many-public-methods',
|
|
'abstract-class-not-used', 'abstract-class-little-used',
|
|
'interface-not-implemented')
|
|
def visit_class(self, node):
|
|
"""check size of inheritance hierarchy and number of instance attributes
|
|
"""
|
|
# Is the total inheritance hierarchy is 7 or less?
|
|
nb_parents = len(list(node.ancestors()))
|
|
if nb_parents > self.config.max_parents:
|
|
self.add_message('too-many-ancestors', node=node,
|
|
args=(nb_parents, self.config.max_parents))
|
|
# Does the class contain less than 20 attributes for
|
|
# non-GUI classes (40 for GUI)?
|
|
# FIXME detect gui classes
|
|
if len(node.instance_attrs) > self.config.max_attributes:
|
|
self.add_message('too-many-instance-attributes', node=node,
|
|
args=(len(node.instance_attrs),
|
|
self.config.max_attributes))
|
|
# update abstract / interface classes structures
|
|
if class_is_abstract(node):
|
|
self._abstracts.append(node)
|
|
elif node.type == 'interface' and node.name != 'Interface':
|
|
self._ifaces.append(node)
|
|
for parent in node.ancestors(False):
|
|
if parent.name == 'Interface':
|
|
continue
|
|
self._used_ifaces[parent] = 1
|
|
try:
|
|
for iface in node.interfaces():
|
|
self._used_ifaces[iface] = 1
|
|
except InferenceError:
|
|
# XXX log ?
|
|
pass
|
|
for parent in node.ancestors():
|
|
try:
|
|
self._used_abstracts[parent] += 1
|
|
except KeyError:
|
|
self._used_abstracts[parent] = 1
|
|
|
|
@check_messages('too-many-ancestors', 'too-many-instance-attributes',
|
|
'too-few-public-methods', 'too-many-public-methods',
|
|
'abstract-class-not-used', 'abstract-class-little-used',
|
|
'interface-not-implemented')
|
|
def leave_class(self, node):
|
|
"""check number of public methods"""
|
|
nb_public_methods = 0
|
|
for method in node.mymethods():
|
|
if not method.name.startswith('_'):
|
|
nb_public_methods += 1
|
|
# Does the class contain less than 20 public methods ?
|
|
if nb_public_methods > self.config.max_public_methods:
|
|
self.add_message('too-many-public-methods', node=node,
|
|
args=(nb_public_methods,
|
|
self.config.max_public_methods))
|
|
# stop here for exception, metaclass and interface classes
|
|
if node.type != 'class':
|
|
return
|
|
# Does the class contain more than 5 public methods ?
|
|
if nb_public_methods < self.config.min_public_methods:
|
|
self.add_message('too-few-public-methods', node=node,
|
|
args=(nb_public_methods,
|
|
self.config.min_public_methods))
|
|
|
|
@check_messages('too-many-return-statements', 'too-many-branches',
|
|
'too-many-arguments', 'too-many-locals',
|
|
'too-many-statements')
|
|
def visit_function(self, node):
|
|
"""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
|
|
if args is not None:
|
|
ignored_args_num = len(
|
|
[arg for arg in args
|
|
if self.config.ignored_argument_names.match(arg.name)])
|
|
argnum = len(args) - ignored_args_num
|
|
if argnum > self.config.max_args:
|
|
self.add_message('too-many-arguments', node=node,
|
|
args=(len(args), self.config.max_args))
|
|
else:
|
|
ignored_args_num = 0
|
|
# check number of local variables
|
|
locnum = len(node.locals) - ignored_args_num
|
|
if locnum > self.config.max_locals:
|
|
self.add_message('too-many-locals', node=node,
|
|
args=(locnum, self.config.max_locals))
|
|
# init statements counter
|
|
self._stmts = 1
|
|
|
|
@check_messages('too-many-return-statements', 'too-many-branches',
|
|
'too-many-arguments', 'too-many-locals',
|
|
'too-many-statements')
|
|
def leave_function(self, node):
|
|
"""most of the work is done here on close:
|
|
checks for max returns, branch, return in __init__
|
|
"""
|
|
returns = self._returns.pop()
|
|
if returns > self.config.max_returns:
|
|
self.add_message('too-many-return-statements', node=node,
|
|
args=(returns, self.config.max_returns))
|
|
branches = self._branches[node]
|
|
if branches > self.config.max_branches:
|
|
self.add_message('too-many-branches', node=node,
|
|
args=(branches, self.config.max_branches))
|
|
# check number of statements
|
|
if self._stmts > self.config.max_statements:
|
|
self.add_message('too-many-statements', node=node,
|
|
args=(self._stmts, self.config.max_statements))
|
|
|
|
def visit_return(self, _):
|
|
"""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):
|
|
"""default visit method -> increments the statements counter if
|
|
necessary
|
|
"""
|
|
if node.is_statement:
|
|
self._stmts += 1
|
|
|
|
def visit_tryexcept(self, node):
|
|
"""increments the branches counter"""
|
|
branches = len(node.handlers)
|
|
if node.orelse:
|
|
branches += 1
|
|
self._inc_branch(node, branches)
|
|
self._stmts += branches
|
|
|
|
def visit_tryfinally(self, node):
|
|
"""increments the branches counter"""
|
|
self._inc_branch(node, 2)
|
|
self._stmts += 2
|
|
|
|
def visit_if(self, node):
|
|
"""increments the branches counter"""
|
|
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], If)):
|
|
branches += 1
|
|
self._inc_branch(node, branches)
|
|
self._stmts += branches
|
|
|
|
def visit_while(self, node):
|
|
"""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, branchesnum=1):
|
|
"""increments the branches counter"""
|
|
self._branches[node.scope()] += branchesnum
|
|
|
|
# FIXME: make a nice report...
|
|
|
|
def register(linter):
|
|
"""required method to auto register this checker """
|
|
linter.register_checker(MisdesignChecker(linter))
|