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 |