Index: third_party/pylint/checkers/design_analysis.py |
=================================================================== |
--- third_party/pylint/checkers/design_analysis.py (revision 292986) |
+++ third_party/pylint/checkers/design_analysis.py (working copy) |
@@ -1,4 +1,4 @@ |
-# Copyright (c) 2003-2006 LOGILAB S.A. (Paris, FRANCE). |
+# 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 |
@@ -12,18 +12,14 @@ |
# |
# 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., |
-# 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. |
-"""check for signs of poor design |
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. |
+"""check for signs of poor design""" |
+from astroid import Function, If, InferenceError |
- see http://intranet.logilab.fr/jpl/view?rql=Any%20X%20where%20X%20eid%201243 |
- FIXME: missing 13, 15, 16 |
-""" |
- |
-from logilab.astng import Function, If, InferenceError |
- |
-from pylint.interfaces import IASTNGChecker |
+from pylint.interfaces import IAstroidChecker |
from pylint.checkers import BaseChecker |
+from pylint.checkers.utils import check_messages |
import re |
@@ -30,6 +26,7 @@ |
# 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 |
@@ -43,50 +40,62 @@ |
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 more simple (and so easier to use) class.'), |
+ 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 more simple (and so easier to use) class.'), |
+ 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 more simple (and so easier to use) class.'), |
- |
+ 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 |
+ """checks for sign of poor/misdesign: |
+ * number of methods, attributes, local variables... |
+ * size, complexity of functions, methods |
""" |
- |
- __implements__ = (IASTNGChecker,) |
+ __implements__ = (IAstroidChecker,) |
+ |
# configuration section name |
name = 'design' |
# messages |
@@ -96,37 +105,37 @@ |
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-branchs', |
+ ), |
+ ('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', |
@@ -133,7 +142,7 @@ |
'metavar' : '<num>', |
'help' : 'Maximum number of attributes for a class \ |
(see R0902).'} |
- ), |
+ ), |
('min-public-methods', |
{'default' : 2, |
'type' : 'int', |
@@ -140,7 +149,7 @@ |
'metavar' : '<num>', |
'help' : 'Minimum number of public methods for a class \ |
(see R0903).'} |
- ), |
+ ), |
('max-public-methods', |
{'default' : 20, |
'type' : 'int', |
@@ -147,42 +156,47 @@ |
'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._branchs = 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._branchs = [] |
+ self._branches = [] |
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('R0921', node=abstract) |
+ self.add_message('abstract-class-not-used', node=abstract) |
elif self._used_abstracts[abstract] < 2: |
- self.add_message('R0922', node=abstract, |
+ 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('R0923', node=iface) |
- |
+ 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 |
""" |
@@ -190,13 +204,13 @@ |
# 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('R0901', node=node, |
+ 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('R0902', node=node, |
+ self.add_message('too-many-instance-attributes', node=node, |
args=(len(node.instance_attrs), |
self.config.max_attributes)) |
# update abstract / interface classes structures |
@@ -212,7 +226,7 @@ |
for iface in node.interfaces(): |
self._used_ifaces[iface] = 1 |
except InferenceError: |
- # XXX log ? |
+ # XXX log ? |
pass |
for parent in node.ancestors(): |
try: |
@@ -219,16 +233,23 @@ |
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 |
+ special_methods = set() |
for method in node.methods(): |
if not method.name.startswith('_'): |
nb_public_methods += 1 |
+ if method.name.startswith("__"): |
+ special_methods.add(method.name) |
# Does the class contain less than 20 public methods ? |
if nb_public_methods > self.config.max_public_methods: |
- self.add_message('R0904', node=node, |
+ 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 |
@@ -240,7 +261,8 @@ |
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 |
@@ -248,7 +270,7 @@ |
self._inc_branch() |
# init branch and returns counters |
self._returns.append(0) |
- self._branchs.append(0) |
+ self._branches.append(0) |
# check number of arguments |
args = node.args.args |
if args is not None: |
@@ -257,7 +279,7 @@ |
if self.config.ignored_argument_names.match(arg.name)]) |
argnum = len(args) - ignored_args_num |
if argnum > self.config.max_args: |
- self.add_message('R0913', node=node, |
+ self.add_message('too-many-arguments', node=node, |
args=(len(args), self.config.max_args)) |
else: |
ignored_args_num = 0 |
@@ -264,11 +286,12 @@ |
# check number of local variables |
locnum = len(node.locals) - ignored_args_num |
if locnum > self.config.max_locals: |
- self.add_message('R0914', node=node, |
+ 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__ |
@@ -275,15 +298,15 @@ |
""" |
returns = self._returns.pop() |
if returns > self.config.max_returns: |
- self.add_message('R0911', node=node, |
+ self.add_message('too-many-return-statements', node=node, |
args=(returns, self.config.max_returns)) |
- branchs = self._branchs.pop() |
- if branchs > self.config.max_branchs: |
- self.add_message('R0912', node=node, |
- args=(branchs, self.config.max_branchs)) |
+ branches = self._branches.pop() |
+ 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('R0915', node=node, |
+ self.add_message('too-many-statements', node=node, |
args=(self._stmts, self.config.max_statements)) |
def visit_return(self, _): |
@@ -291,7 +314,7 @@ |
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 |
@@ -300,42 +323,42 @@ |
self._stmts += 1 |
def visit_tryexcept(self, node): |
- """increments the branchs counter""" |
- branchs = len(node.handlers) |
+ """increments the branches counter""" |
+ branches = len(node.handlers) |
if node.orelse: |
- branchs += 1 |
- self._inc_branch(branchs) |
- self._stmts += branchs |
- |
+ branches += 1 |
+ self._inc_branch(branches) |
+ self._stmts += branches |
+ |
def visit_tryfinally(self, _): |
- """increments the branchs counter""" |
+ """increments the branches counter""" |
self._inc_branch(2) |
self._stmts += 2 |
- |
+ |
def visit_if(self, node): |
- """increments the branchs counter""" |
- branchs = 1 |
+ """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 |
+ if node.orelse and (len(node.orelse) > 1 or |
not isinstance(node.orelse[0], If)): |
- branchs += 1 |
- self._inc_branch(branchs) |
- self._stmts += branchs |
- |
+ branches += 1 |
+ self._inc_branch(branches) |
+ self._stmts += branches |
+ |
def visit_while(self, node): |
- """increments the branchs counter""" |
- branchs = 1 |
+ """increments the branches counter""" |
+ branches = 1 |
if node.orelse: |
- branchs += 1 |
- self._inc_branch(branchs) |
- |
+ branches += 1 |
+ self._inc_branch(branches) |
+ |
visit_for = visit_while |
- def _inc_branch(self, branchsnum=1): |
- """increments the branchs counter""" |
- branchs = self._branchs |
- for i in xrange(len(branchs)): |
- branchs[i] += branchsnum |
+ def _inc_branch(self, branchesnum=1): |
+ """increments the branches counter""" |
+ branches = self._branches |
+ for i in xrange(len(branches)): |
+ branches[i] += branchesnum |
# FIXME: make a nice report... |