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