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