OLD | NEW |
(Empty) | |
| 1 # Copyright (c) 2003-2013 LOGILAB S.A. (Paris, FRANCE). |
| 2 # http://www.logilab.fr/ -- mailto:contact@logilab.fr |
| 3 # |
| 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 |
| 6 # Foundation; either version 2 of the License, or (at your option) any later |
| 7 # version. |
| 8 # |
| 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 |
| 11 # FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. |
| 12 # |
| 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., |
| 15 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. |
| 16 """check for signs of poor design""" |
| 17 |
| 18 import re |
| 19 from collections import defaultdict |
| 20 |
| 21 from astroid import Function, If, InferenceError |
| 22 |
| 23 from pylint.interfaces import IAstroidChecker |
| 24 from pylint.checkers import BaseChecker |
| 25 from pylint.checkers.utils import check_messages |
| 26 |
| 27 # regexp for ignored argument name |
| 28 IGNORED_ARGUMENT_NAMES = re.compile('_.*') |
| 29 |
| 30 |
| 31 def class_is_abstract(klass): |
| 32 """return true if the given class node should be considered as an abstract |
| 33 class |
| 34 """ |
| 35 for attr in klass.values(): |
| 36 if isinstance(attr, Function): |
| 37 if attr.is_abstract(pass_is_abstract=False): |
| 38 return True |
| 39 return False |
| 40 |
| 41 |
| 42 MSGS = { |
| 43 'R0901': ('Too many ancestors (%s/%s)', |
| 44 'too-many-ancestors', |
| 45 'Used when class has too many parent classes, try to reduce \ |
| 46 this to get a simpler (and so easier to use) class.'), |
| 47 'R0902': ('Too many instance attributes (%s/%s)', |
| 48 'too-many-instance-attributes', |
| 49 'Used when class has too many instance attributes, try to reduce \ |
| 50 this to get a simpler (and so easier to use) class.'), |
| 51 'R0903': ('Too few public methods (%s/%s)', |
| 52 'too-few-public-methods', |
| 53 'Used when class has too few public methods, so be sure it\'s \ |
| 54 really worth it.'), |
| 55 'R0904': ('Too many public methods (%s/%s)', |
| 56 'too-many-public-methods', |
| 57 'Used when class has too many public methods, try to reduce \ |
| 58 this to get a simpler (and so easier to use) class.'), |
| 59 |
| 60 'R0911': ('Too many return statements (%s/%s)', |
| 61 'too-many-return-statements', |
| 62 'Used when a function or method has too many return statement, \ |
| 63 making it hard to follow.'), |
| 64 'R0912': ('Too many branches (%s/%s)', |
| 65 'too-many-branches', |
| 66 'Used when a function or method has too many branches, \ |
| 67 making it hard to follow.'), |
| 68 'R0913': ('Too many arguments (%s/%s)', |
| 69 'too-many-arguments', |
| 70 'Used when a function or method takes too many arguments.'), |
| 71 'R0914': ('Too many local variables (%s/%s)', |
| 72 'too-many-locals', |
| 73 'Used when a function or method has too many local variables.'), |
| 74 'R0915': ('Too many statements (%s/%s)', |
| 75 'too-many-statements', |
| 76 'Used when a function or method has too many statements. You \ |
| 77 should then split it in smaller functions / methods.'), |
| 78 |
| 79 'R0921': ('Abstract class not referenced', |
| 80 'abstract-class-not-used', |
| 81 'Used when an abstract class is not used as ancestor anywhere.'), |
| 82 'R0922': ('Abstract class is only referenced %s times', |
| 83 'abstract-class-little-used', |
| 84 'Used when an abstract class is used less than X times as \ |
| 85 ancestor.'), |
| 86 'R0923': ('Interface not implemented', |
| 87 'interface-not-implemented', |
| 88 'Used when an interface class is not implemented anywhere.'), |
| 89 } |
| 90 |
| 91 |
| 92 class MisdesignChecker(BaseChecker): |
| 93 """checks for sign of poor/misdesign: |
| 94 * number of methods, attributes, local variables... |
| 95 * size, complexity of functions, methods |
| 96 """ |
| 97 |
| 98 __implements__ = (IAstroidChecker,) |
| 99 |
| 100 # configuration section name |
| 101 name = 'design' |
| 102 # messages |
| 103 msgs = MSGS |
| 104 priority = -2 |
| 105 # configuration options |
| 106 options = (('max-args', |
| 107 {'default' : 5, 'type' : 'int', 'metavar' : '<int>', |
| 108 'help': 'Maximum number of arguments for function / method'} |
| 109 ), |
| 110 ('ignored-argument-names', |
| 111 {'default' : IGNORED_ARGUMENT_NAMES, |
| 112 'type' :'regexp', 'metavar' : '<regexp>', |
| 113 'help' : 'Argument names that match this expression will be ' |
| 114 'ignored. Default to name with leading underscore'} |
| 115 ), |
| 116 ('max-locals', |
| 117 {'default' : 15, 'type' : 'int', 'metavar' : '<int>', |
| 118 'help': 'Maximum number of locals for function / method body'} |
| 119 ), |
| 120 ('max-returns', |
| 121 {'default' : 6, 'type' : 'int', 'metavar' : '<int>', |
| 122 'help': 'Maximum number of return / yield for function / ' |
| 123 'method body'} |
| 124 ), |
| 125 ('max-branches', |
| 126 {'default' : 12, 'type' : 'int', 'metavar' : '<int>', |
| 127 'help': 'Maximum number of branch for function / method body'} |
| 128 ), |
| 129 ('max-statements', |
| 130 {'default' : 50, 'type' : 'int', 'metavar' : '<int>', |
| 131 'help': 'Maximum number of statements in function / method ' |
| 132 'body'} |
| 133 ), |
| 134 ('max-parents', |
| 135 {'default' : 7, |
| 136 'type' : 'int', |
| 137 'metavar' : '<num>', |
| 138 'help' : 'Maximum number of parents for a class (see R0901).'} |
| 139 ), |
| 140 ('max-attributes', |
| 141 {'default' : 7, |
| 142 'type' : 'int', |
| 143 'metavar' : '<num>', |
| 144 'help' : 'Maximum number of attributes for a class \ |
| 145 (see R0902).'} |
| 146 ), |
| 147 ('min-public-methods', |
| 148 {'default' : 2, |
| 149 'type' : 'int', |
| 150 'metavar' : '<num>', |
| 151 'help' : 'Minimum number of public methods for a class \ |
| 152 (see R0903).'} |
| 153 ), |
| 154 ('max-public-methods', |
| 155 {'default' : 20, |
| 156 'type' : 'int', |
| 157 'metavar' : '<num>', |
| 158 'help' : 'Maximum number of public methods for a class \ |
| 159 (see R0904).'} |
| 160 ), |
| 161 ) |
| 162 |
| 163 def __init__(self, linter=None): |
| 164 BaseChecker.__init__(self, linter) |
| 165 self.stats = None |
| 166 self._returns = None |
| 167 self._branches = None |
| 168 self._used_abstracts = None |
| 169 self._used_ifaces = None |
| 170 self._abstracts = None |
| 171 self._ifaces = None |
| 172 self._stmts = 0 |
| 173 |
| 174 def open(self): |
| 175 """initialize visit variables""" |
| 176 self.stats = self.linter.add_stats() |
| 177 self._returns = [] |
| 178 self._branches = defaultdict(int) |
| 179 self._used_abstracts = {} |
| 180 self._used_ifaces = {} |
| 181 self._abstracts = [] |
| 182 self._ifaces = [] |
| 183 |
| 184 # Check 'R0921', 'R0922', 'R0923' |
| 185 def close(self): |
| 186 """check that abstract/interface classes are used""" |
| 187 for abstract in self._abstracts: |
| 188 if not abstract in self._used_abstracts: |
| 189 self.add_message('abstract-class-not-used', node=abstract) |
| 190 elif self._used_abstracts[abstract] < 2: |
| 191 self.add_message('abstract-class-little-used', node=abstract, |
| 192 args=self._used_abstracts[abstract]) |
| 193 for iface in self._ifaces: |
| 194 if not iface in self._used_ifaces: |
| 195 self.add_message('interface-not-implemented', node=iface) |
| 196 |
| 197 @check_messages('too-many-ancestors', 'too-many-instance-attributes', |
| 198 'too-few-public-methods', 'too-many-public-methods', |
| 199 'abstract-class-not-used', 'abstract-class-little-used', |
| 200 'interface-not-implemented') |
| 201 def visit_class(self, node): |
| 202 """check size of inheritance hierarchy and number of instance attributes |
| 203 """ |
| 204 # Is the total inheritance hierarchy is 7 or less? |
| 205 nb_parents = len(list(node.ancestors())) |
| 206 if nb_parents > self.config.max_parents: |
| 207 self.add_message('too-many-ancestors', node=node, |
| 208 args=(nb_parents, self.config.max_parents)) |
| 209 # Does the class contain less than 20 attributes for |
| 210 # non-GUI classes (40 for GUI)? |
| 211 # FIXME detect gui classes |
| 212 if len(node.instance_attrs) > self.config.max_attributes: |
| 213 self.add_message('too-many-instance-attributes', node=node, |
| 214 args=(len(node.instance_attrs), |
| 215 self.config.max_attributes)) |
| 216 # update abstract / interface classes structures |
| 217 if class_is_abstract(node): |
| 218 self._abstracts.append(node) |
| 219 elif node.type == 'interface' and node.name != 'Interface': |
| 220 self._ifaces.append(node) |
| 221 for parent in node.ancestors(False): |
| 222 if parent.name == 'Interface': |
| 223 continue |
| 224 self._used_ifaces[parent] = 1 |
| 225 try: |
| 226 for iface in node.interfaces(): |
| 227 self._used_ifaces[iface] = 1 |
| 228 except InferenceError: |
| 229 # XXX log ? |
| 230 pass |
| 231 for parent in node.ancestors(): |
| 232 try: |
| 233 self._used_abstracts[parent] += 1 |
| 234 except KeyError: |
| 235 self._used_abstracts[parent] = 1 |
| 236 |
| 237 @check_messages('too-many-ancestors', 'too-many-instance-attributes', |
| 238 'too-few-public-methods', 'too-many-public-methods', |
| 239 'abstract-class-not-used', 'abstract-class-little-used', |
| 240 'interface-not-implemented') |
| 241 def leave_class(self, node): |
| 242 """check number of public methods""" |
| 243 nb_public_methods = 0 |
| 244 for method in node.mymethods(): |
| 245 if not method.name.startswith('_'): |
| 246 nb_public_methods += 1 |
| 247 # Does the class contain less than 20 public methods ? |
| 248 if nb_public_methods > self.config.max_public_methods: |
| 249 self.add_message('too-many-public-methods', node=node, |
| 250 args=(nb_public_methods, |
| 251 self.config.max_public_methods)) |
| 252 # stop here for exception, metaclass and interface classes |
| 253 if node.type != 'class': |
| 254 return |
| 255 # Does the class contain more than 5 public methods ? |
| 256 if nb_public_methods < self.config.min_public_methods: |
| 257 self.add_message('too-few-public-methods', node=node, |
| 258 args=(nb_public_methods, |
| 259 self.config.min_public_methods)) |
| 260 |
| 261 @check_messages('too-many-return-statements', 'too-many-branches', |
| 262 'too-many-arguments', 'too-many-locals', |
| 263 'too-many-statements') |
| 264 def visit_function(self, node): |
| 265 """check function name, docstring, arguments, redefinition, |
| 266 variable names, max locals |
| 267 """ |
| 268 # init branch and returns counters |
| 269 self._returns.append(0) |
| 270 # check number of arguments |
| 271 args = node.args.args |
| 272 if args is not None: |
| 273 ignored_args_num = len( |
| 274 [arg for arg in args |
| 275 if self.config.ignored_argument_names.match(arg.name)]) |
| 276 argnum = len(args) - ignored_args_num |
| 277 if argnum > self.config.max_args: |
| 278 self.add_message('too-many-arguments', node=node, |
| 279 args=(len(args), self.config.max_args)) |
| 280 else: |
| 281 ignored_args_num = 0 |
| 282 # check number of local variables |
| 283 locnum = len(node.locals) - ignored_args_num |
| 284 if locnum > self.config.max_locals: |
| 285 self.add_message('too-many-locals', node=node, |
| 286 args=(locnum, self.config.max_locals)) |
| 287 # init statements counter |
| 288 self._stmts = 1 |
| 289 |
| 290 @check_messages('too-many-return-statements', 'too-many-branches', |
| 291 'too-many-arguments', 'too-many-locals', |
| 292 'too-many-statements') |
| 293 def leave_function(self, node): |
| 294 """most of the work is done here on close: |
| 295 checks for max returns, branch, return in __init__ |
| 296 """ |
| 297 returns = self._returns.pop() |
| 298 if returns > self.config.max_returns: |
| 299 self.add_message('too-many-return-statements', node=node, |
| 300 args=(returns, self.config.max_returns)) |
| 301 branches = self._branches[node] |
| 302 if branches > self.config.max_branches: |
| 303 self.add_message('too-many-branches', node=node, |
| 304 args=(branches, self.config.max_branches)) |
| 305 # check number of statements |
| 306 if self._stmts > self.config.max_statements: |
| 307 self.add_message('too-many-statements', node=node, |
| 308 args=(self._stmts, self.config.max_statements)) |
| 309 |
| 310 def visit_return(self, _): |
| 311 """count number of returns""" |
| 312 if not self._returns: |
| 313 return # return outside function, reported by the base checker |
| 314 self._returns[-1] += 1 |
| 315 |
| 316 def visit_default(self, node): |
| 317 """default visit method -> increments the statements counter if |
| 318 necessary |
| 319 """ |
| 320 if node.is_statement: |
| 321 self._stmts += 1 |
| 322 |
| 323 def visit_tryexcept(self, node): |
| 324 """increments the branches counter""" |
| 325 branches = len(node.handlers) |
| 326 if node.orelse: |
| 327 branches += 1 |
| 328 self._inc_branch(node, branches) |
| 329 self._stmts += branches |
| 330 |
| 331 def visit_tryfinally(self, node): |
| 332 """increments the branches counter""" |
| 333 self._inc_branch(node, 2) |
| 334 self._stmts += 2 |
| 335 |
| 336 def visit_if(self, node): |
| 337 """increments the branches counter""" |
| 338 branches = 1 |
| 339 # don't double count If nodes coming from some 'elif' |
| 340 if node.orelse and (len(node.orelse) > 1 or |
| 341 not isinstance(node.orelse[0], If)): |
| 342 branches += 1 |
| 343 self._inc_branch(node, branches) |
| 344 self._stmts += branches |
| 345 |
| 346 def visit_while(self, node): |
| 347 """increments the branches counter""" |
| 348 branches = 1 |
| 349 if node.orelse: |
| 350 branches += 1 |
| 351 self._inc_branch(node, branches) |
| 352 |
| 353 visit_for = visit_while |
| 354 |
| 355 def _inc_branch(self, node, branchesnum=1): |
| 356 """increments the branches counter""" |
| 357 self._branches[node.scope()] += branchesnum |
| 358 |
| 359 # FIXME: make a nice report... |
| 360 |
| 361 def register(linter): |
| 362 """required method to auto register this checker """ |
| 363 linter.register_checker(MisdesignChecker(linter)) |
OLD | NEW |