| 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...
 | 
|  
 | 
| 
 |