| OLD | NEW | 
|---|
| 1 # Copyright (c) 2003-2013 LOGILAB S.A. (Paris, FRANCE). | 1 # Copyright (c) 2003-2013 LOGILAB S.A. (Paris, FRANCE). | 
| 2 # http://www.logilab.fr/ -- mailto:contact@logilab.fr | 2 # http://www.logilab.fr/ -- mailto:contact@logilab.fr | 
| 3 # | 3 # | 
| 4 # This program is free software; you can redistribute it and/or modify it under | 4 # This program is free software; you can redistribute it and/or modify it under | 
| 5 # the terms of the GNU General Public License as published by the Free Software | 5 # the terms of the GNU General Public License as published by the Free Software | 
| 6 # Foundation; either version 2 of the License, or (at your option) any later | 6 # Foundation; either version 2 of the License, or (at your option) any later | 
| 7 # version. | 7 # version. | 
| 8 # | 8 # | 
| 9 # This program is distributed in the hope that it will be useful, but WITHOUT | 9 # This program is distributed in the hope that it will be useful, but WITHOUT | 
| 10 # ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS | 10 # ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS | 
| 11 # FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. | 11 # FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. | 
| 12 # | 12 # | 
| 13 # You should have received a copy of the GNU General Public License along with | 13 # You should have received a copy of the GNU General Public License along with | 
| 14 # this program; if not, write to the Free Software Foundation, Inc., | 14 # this program; if not, write to the Free Software Foundation, Inc., | 
| 15 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | 15 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | 
| 16 """check for signs of poor design""" | 16 """check for signs of poor design""" | 
| 17 | 17 | 
|  | 18 import re | 
|  | 19 from collections import defaultdict | 
|  | 20 | 
| 18 from astroid import Function, If, InferenceError | 21 from astroid import Function, If, InferenceError | 
| 19 | 22 | 
| 20 from pylint.interfaces import IAstroidChecker | 23 from pylint.interfaces import IAstroidChecker | 
| 21 from pylint.checkers import BaseChecker | 24 from pylint.checkers import BaseChecker | 
| 22 from pylint.checkers.utils import check_messages | 25 from pylint.checkers.utils import check_messages | 
| 23 | 26 | 
| 24 import re |  | 
| 25 |  | 
| 26 # regexp for ignored argument name | 27 # regexp for ignored argument name | 
| 27 IGNORED_ARGUMENT_NAMES = re.compile('_.*') | 28 IGNORED_ARGUMENT_NAMES = re.compile('_.*') | 
| 28 | 29 | 
| 29 | 30 | 
| 30 def class_is_abstract(klass): | 31 def class_is_abstract(klass): | 
| 31     """return true if the given class node should be considered as an abstract | 32     """return true if the given class node should be considered as an abstract | 
| 32     class | 33     class | 
| 33     """ | 34     """ | 
| 34     for attr in klass.values(): | 35     for attr in klass.values(): | 
| 35         if isinstance(attr, Function): | 36         if isinstance(attr, Function): | 
| (...skipping 131 matching lines...) Expand 10 before | Expand all | Expand 10 after  Loading... | 
| 167         self._used_abstracts = None | 168         self._used_abstracts = None | 
| 168         self._used_ifaces = None | 169         self._used_ifaces = None | 
| 169         self._abstracts = None | 170         self._abstracts = None | 
| 170         self._ifaces = None | 171         self._ifaces = None | 
| 171         self._stmts = 0 | 172         self._stmts = 0 | 
| 172 | 173 | 
| 173     def open(self): | 174     def open(self): | 
| 174         """initialize visit variables""" | 175         """initialize visit variables""" | 
| 175         self.stats = self.linter.add_stats() | 176         self.stats = self.linter.add_stats() | 
| 176         self._returns = [] | 177         self._returns = [] | 
| 177         self._branches = [] | 178         self._branches = defaultdict(int) | 
| 178         self._used_abstracts = {} | 179         self._used_abstracts = {} | 
| 179         self._used_ifaces = {} | 180         self._used_ifaces = {} | 
| 180         self._abstracts = [] | 181         self._abstracts = [] | 
| 181         self._ifaces = [] | 182         self._ifaces = [] | 
| 182 | 183 | 
| 183     # Check 'R0921', 'R0922', 'R0923' | 184     # Check 'R0921', 'R0922', 'R0923' | 
| 184     def close(self): | 185     def close(self): | 
| 185         """check that abstract/interface classes are used""" | 186         """check that abstract/interface classes are used""" | 
| 186         for abstract in self._abstracts: | 187         for abstract in self._abstracts: | 
| 187             if not abstract in self._used_abstracts: | 188             if not abstract in self._used_abstracts: | 
| 188                 self.add_message('abstract-class-not-used', node=abstract) | 189                 self.add_message('abstract-class-not-used', node=abstract) | 
| 189             elif self._used_abstracts[abstract] < 2: | 190             elif self._used_abstracts[abstract] < 2: | 
| 190                 self.add_message('abstract-class-little-used', node=abstract, | 191                 self.add_message('abstract-class-little-used', node=abstract, | 
| 191                                  args=self._used_abstracts[abstract]) | 192                                  args=self._used_abstracts[abstract]) | 
| 192         for iface in self._ifaces: | 193         for iface in self._ifaces: | 
| 193             if not iface in self._used_ifaces: | 194             if not iface in self._used_ifaces: | 
| 194                 self.add_message('interface-not-implemented', node=iface) | 195                 self.add_message('interface-not-implemented', node=iface) | 
| 195 | 196 | 
| 196     @check_messages('too-many-ancestors', 'too-many-instance-attributes', | 197     @check_messages('too-many-ancestors', 'too-many-instance-attributes', | 
| 197                     'too-few-public-methods', 'too-many-public-methods', | 198                     'too-few-public-methods', 'too-many-public-methods', | 
| 198                     'abstract-class-not-used', 'abstract-class-little-used', | 199                     'abstract-class-not-used', 'abstract-class-little-used', | 
| 199                     'interface-not-implemented') | 200                     'interface-not-implemented') | 
| 200     def visit_class(self, node): | 201     def visit_class(self, node): | 
| 201         """check size of inheritance hierarchy and number of instance attributes | 202         """check size of inheritance hierarchy and number of instance attributes | 
| 202         """ | 203         """ | 
| 203         self._inc_branch() |  | 
| 204         # Is the total inheritance hierarchy is 7 or less? | 204         # Is the total inheritance hierarchy is 7 or less? | 
| 205         nb_parents = len(list(node.ancestors())) | 205         nb_parents = len(list(node.ancestors())) | 
| 206         if nb_parents > self.config.max_parents: | 206         if nb_parents > self.config.max_parents: | 
| 207             self.add_message('too-many-ancestors', node=node, | 207             self.add_message('too-many-ancestors', node=node, | 
| 208                              args=(nb_parents, self.config.max_parents)) | 208                              args=(nb_parents, self.config.max_parents)) | 
| 209         # Does the class contain less than 20 attributes for | 209         # Does the class contain less than 20 attributes for | 
| 210         # non-GUI classes (40 for GUI)? | 210         # non-GUI classes (40 for GUI)? | 
| 211         # FIXME detect gui classes | 211         # FIXME detect gui classes | 
| 212         if len(node.instance_attrs) > self.config.max_attributes: | 212         if len(node.instance_attrs) > self.config.max_attributes: | 
| 213             self.add_message('too-many-instance-attributes', node=node, | 213             self.add_message('too-many-instance-attributes', node=node, | 
| (...skipping 20 matching lines...) Expand all  Loading... | 
| 234             except KeyError: | 234             except KeyError: | 
| 235                 self._used_abstracts[parent] = 1 | 235                 self._used_abstracts[parent] = 1 | 
| 236 | 236 | 
| 237     @check_messages('too-many-ancestors', 'too-many-instance-attributes', | 237     @check_messages('too-many-ancestors', 'too-many-instance-attributes', | 
| 238                     'too-few-public-methods', 'too-many-public-methods', | 238                     'too-few-public-methods', 'too-many-public-methods', | 
| 239                     'abstract-class-not-used', 'abstract-class-little-used', | 239                     'abstract-class-not-used', 'abstract-class-little-used', | 
| 240                     'interface-not-implemented') | 240                     'interface-not-implemented') | 
| 241     def leave_class(self, node): | 241     def leave_class(self, node): | 
| 242         """check number of public methods""" | 242         """check number of public methods""" | 
| 243         nb_public_methods = 0 | 243         nb_public_methods = 0 | 
| 244         special_methods = set() | 244         for method in node.mymethods(): | 
| 245         for method in node.methods(): |  | 
| 246             if not method.name.startswith('_'): | 245             if not method.name.startswith('_'): | 
| 247                 nb_public_methods += 1 | 246                 nb_public_methods += 1 | 
| 248             if method.name.startswith("__"): |  | 
| 249                 special_methods.add(method.name) |  | 
| 250         # Does the class contain less than 20 public methods ? | 247         # Does the class contain less than 20 public methods ? | 
| 251         if nb_public_methods > self.config.max_public_methods: | 248         if nb_public_methods > self.config.max_public_methods: | 
| 252             self.add_message('too-many-public-methods', node=node, | 249             self.add_message('too-many-public-methods', node=node, | 
| 253                              args=(nb_public_methods, | 250                              args=(nb_public_methods, | 
| 254                                    self.config.max_public_methods)) | 251                                    self.config.max_public_methods)) | 
| 255         # stop here for exception, metaclass and interface classes | 252         # stop here for exception, metaclass and interface classes | 
| 256         if node.type != 'class': | 253         if node.type != 'class': | 
| 257             return | 254             return | 
| 258         # Does the class contain more than 5 public methods ? | 255         # Does the class contain more than 5 public methods ? | 
| 259         if nb_public_methods < self.config.min_public_methods: | 256         if nb_public_methods < self.config.min_public_methods: | 
| 260             self.add_message('R0903', node=node, | 257             self.add_message('too-few-public-methods', node=node, | 
| 261                              args=(nb_public_methods, | 258                              args=(nb_public_methods, | 
| 262                                    self.config.min_public_methods)) | 259                                    self.config.min_public_methods)) | 
| 263 | 260 | 
| 264     @check_messages('too-many-return-statements', 'too-many-branches', | 261     @check_messages('too-many-return-statements', 'too-many-branches', | 
| 265                     'too-many-arguments', 'too-many-locals', 'too-many-statement
     s') | 262                     'too-many-arguments', 'too-many-locals', | 
|  | 263                     'too-many-statements') | 
| 266     def visit_function(self, node): | 264     def visit_function(self, node): | 
| 267         """check function name, docstring, arguments, redefinition, | 265         """check function name, docstring, arguments, redefinition, | 
| 268         variable names, max locals | 266         variable names, max locals | 
| 269         """ | 267         """ | 
| 270         self._inc_branch() |  | 
| 271         # init branch and returns counters | 268         # init branch and returns counters | 
| 272         self._returns.append(0) | 269         self._returns.append(0) | 
| 273         self._branches.append(0) |  | 
| 274         # check number of arguments | 270         # check number of arguments | 
| 275         args = node.args.args | 271         args = node.args.args | 
| 276         if args is not None: | 272         if args is not None: | 
| 277             ignored_args_num = len( | 273             ignored_args_num = len( | 
| 278                 [arg for arg in args | 274                 [arg for arg in args | 
| 279                  if self.config.ignored_argument_names.match(arg.name)]) | 275                  if self.config.ignored_argument_names.match(arg.name)]) | 
| 280             argnum = len(args) - ignored_args_num | 276             argnum = len(args) - ignored_args_num | 
| 281             if  argnum > self.config.max_args: | 277             if  argnum > self.config.max_args: | 
| 282                 self.add_message('too-many-arguments', node=node, | 278                 self.add_message('too-many-arguments', node=node, | 
| 283                                  args=(len(args), self.config.max_args)) | 279                                  args=(len(args), self.config.max_args)) | 
| 284         else: | 280         else: | 
| 285             ignored_args_num = 0 | 281             ignored_args_num = 0 | 
| 286         # check number of local variables | 282         # check number of local variables | 
| 287         locnum = len(node.locals) - ignored_args_num | 283         locnum = len(node.locals) - ignored_args_num | 
| 288         if locnum > self.config.max_locals: | 284         if locnum > self.config.max_locals: | 
| 289             self.add_message('too-many-locals', node=node, | 285             self.add_message('too-many-locals', node=node, | 
| 290                              args=(locnum, self.config.max_locals)) | 286                              args=(locnum, self.config.max_locals)) | 
| 291         # init statements counter | 287         # init statements counter | 
| 292         self._stmts = 1 | 288         self._stmts = 1 | 
| 293 | 289 | 
| 294     @check_messages('too-many-return-statements', 'too-many-branches', 'too-many
     -arguments', 'too-many-locals', 'too-many-statements') | 290     @check_messages('too-many-return-statements', 'too-many-branches', | 
|  | 291                     'too-many-arguments', 'too-many-locals', | 
|  | 292                     'too-many-statements') | 
| 295     def leave_function(self, node): | 293     def leave_function(self, node): | 
| 296         """most of the work is done here on close: | 294         """most of the work is done here on close: | 
| 297         checks for max returns, branch, return in __init__ | 295         checks for max returns, branch, return in __init__ | 
| 298         """ | 296         """ | 
| 299         returns = self._returns.pop() | 297         returns = self._returns.pop() | 
| 300         if returns > self.config.max_returns: | 298         if returns > self.config.max_returns: | 
| 301             self.add_message('too-many-return-statements', node=node, | 299             self.add_message('too-many-return-statements', node=node, | 
| 302                              args=(returns, self.config.max_returns)) | 300                              args=(returns, self.config.max_returns)) | 
| 303         branches = self._branches.pop() | 301         branches = self._branches[node] | 
| 304         if branches > self.config.max_branches: | 302         if branches > self.config.max_branches: | 
| 305             self.add_message('too-many-branches', node=node, | 303             self.add_message('too-many-branches', node=node, | 
| 306                              args=(branches, self.config.max_branches)) | 304                              args=(branches, self.config.max_branches)) | 
| 307         # check number of statements | 305         # check number of statements | 
| 308         if self._stmts > self.config.max_statements: | 306         if self._stmts > self.config.max_statements: | 
| 309             self.add_message('too-many-statements', node=node, | 307             self.add_message('too-many-statements', node=node, | 
| 310                              args=(self._stmts, self.config.max_statements)) | 308                              args=(self._stmts, self.config.max_statements)) | 
| 311 | 309 | 
| 312     def visit_return(self, _): | 310     def visit_return(self, _): | 
| 313         """count number of returns""" | 311         """count number of returns""" | 
| 314         if not self._returns: | 312         if not self._returns: | 
| 315             return # return outside function, reported by the base checker | 313             return # return outside function, reported by the base checker | 
| 316         self._returns[-1] += 1 | 314         self._returns[-1] += 1 | 
| 317 | 315 | 
| 318     def visit_default(self, node): | 316     def visit_default(self, node): | 
| 319         """default visit method -> increments the statements counter if | 317         """default visit method -> increments the statements counter if | 
| 320         necessary | 318         necessary | 
| 321         """ | 319         """ | 
| 322         if node.is_statement: | 320         if node.is_statement: | 
| 323             self._stmts += 1 | 321             self._stmts += 1 | 
| 324 | 322 | 
| 325     def visit_tryexcept(self, node): | 323     def visit_tryexcept(self, node): | 
| 326         """increments the branches counter""" | 324         """increments the branches counter""" | 
| 327         branches = len(node.handlers) | 325         branches = len(node.handlers) | 
| 328         if node.orelse: | 326         if node.orelse: | 
| 329             branches += 1 | 327             branches += 1 | 
| 330         self._inc_branch(branches) | 328         self._inc_branch(node, branches) | 
| 331         self._stmts += branches | 329         self._stmts += branches | 
| 332 | 330 | 
| 333     def visit_tryfinally(self, _): | 331     def visit_tryfinally(self, node): | 
| 334         """increments the branches counter""" | 332         """increments the branches counter""" | 
| 335         self._inc_branch(2) | 333         self._inc_branch(node, 2) | 
| 336         self._stmts += 2 | 334         self._stmts += 2 | 
| 337 | 335 | 
| 338     def visit_if(self, node): | 336     def visit_if(self, node): | 
| 339         """increments the branches counter""" | 337         """increments the branches counter""" | 
| 340         branches = 1 | 338         branches = 1 | 
| 341         # don't double count If nodes coming from some 'elif' | 339         # don't double count If nodes coming from some 'elif' | 
| 342         if node.orelse and (len(node.orelse) > 1 or | 340         if node.orelse and (len(node.orelse) > 1 or | 
| 343                             not isinstance(node.orelse[0], If)): | 341                             not isinstance(node.orelse[0], If)): | 
| 344             branches += 1 | 342             branches += 1 | 
| 345         self._inc_branch(branches) | 343         self._inc_branch(node, branches) | 
| 346         self._stmts += branches | 344         self._stmts += branches | 
| 347 | 345 | 
| 348     def visit_while(self, node): | 346     def visit_while(self, node): | 
| 349         """increments the branches counter""" | 347         """increments the branches counter""" | 
| 350         branches = 1 | 348         branches = 1 | 
| 351         if node.orelse: | 349         if node.orelse: | 
| 352             branches += 1 | 350             branches += 1 | 
| 353         self._inc_branch(branches) | 351         self._inc_branch(node, branches) | 
| 354 | 352 | 
| 355     visit_for = visit_while | 353     visit_for = visit_while | 
| 356 | 354 | 
| 357     def _inc_branch(self, branchesnum=1): | 355     def _inc_branch(self, node, branchesnum=1): | 
| 358         """increments the branches counter""" | 356         """increments the branches counter""" | 
| 359         branches = self._branches | 357         self._branches[node.scope()] += branchesnum | 
| 360         for i in xrange(len(branches)): |  | 
| 361             branches[i] += branchesnum |  | 
| 362 | 358 | 
| 363     # FIXME: make a nice report... | 359     # FIXME: make a nice report... | 
| 364 | 360 | 
| 365 def register(linter): | 361 def register(linter): | 
| 366     """required method to auto register this checker """ | 362     """required method to auto register this checker """ | 
| 367     linter.register_checker(MisdesignChecker(linter)) | 363     linter.register_checker(MisdesignChecker(linter)) | 
| OLD | NEW | 
|---|