diff options
| -rw-r--r-- | CONTRIBUTORS.txt | 3 | ||||
| -rw-r--r-- | ChangeLog | 9 | ||||
| -rw-r--r-- | pylint/checkers/classes.py | 33 | ||||
| -rw-r--r-- | pylint/checkers/design_analysis.py | 44 | ||||
| -rw-r--r-- | pylint/checkers/raw_metrics.py | 2 | ||||
| -rw-r--r-- | pylint/checkers/variables.py | 4 | ||||
| -rw-r--r-- | pylint/pyreverse/diagrams.py | 15 | ||||
| -rw-r--r-- | pylint/test/functional/non_iterator_returned.py | 21 | ||||
| -rw-r--r-- | pylint/test/functional/non_iterator_returned.txt | 9 | ||||
| -rw-r--r-- | pylint/test/functional/too_many_boolean_expressions.py | 20 | ||||
| -rw-r--r-- | pylint/test/functional/too_many_boolean_expressions.txt | 4 | ||||
| -rw-r--r-- | pylint/test/functional/too_many_nested_blocks.py | 2 | ||||
| -rw-r--r-- | pylint/test/functional/unbalanced_tuple_unpacking.py | 8 | ||||
| -rw-r--r-- | pylint/test/functional/unbalanced_tuple_unpacking.txt | 4 | ||||
| -rw-r--r-- | pylint/test/functional/unpacking_non_sequence.py | 26 | ||||
| -rw-r--r-- | pylint/test/functional/unpacking_non_sequence.txt | 22 | ||||
| -rw-r--r-- | pylint/test/functional/unsubscriptable_value.py | 6 | ||||
| -rw-r--r-- | tox.ini | 3 |
18 files changed, 180 insertions, 55 deletions
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index f4d66b8..8317678 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -74,4 +74,5 @@ Order doesn't matter (not that much, at least ;) * Dmitry Pribysh: multiple-imports, not-iterable, not-a-mapping, various patches. * Laura Medioni (Logilab, on behalf of the CNES): misplaced-comparison-constant, - no-classmethod-decorator, no-staticmethod-decorator, too-many-nested-blocks + no-classmethod-decorator, no-staticmethod-decorator, too-many-nested-blocks, + too-many-boolean-expressions
\ No newline at end of file @@ -2,6 +2,15 @@ ChangeLog for Pylint -------------------- -- + + * Added a new refactoring warning, 'too-many-boolean-expressions', + used when a if statement contains too many boolean expressions, + which makes the code less maintainable and harder to understand. + Closes issue #677. + + * Property methods are shown as attributes instead of functions in + pyreverse class diagrams. Closes Issue #284 + * Add a new refactoring error, 'too-many-nested-blocks', which is emitted when a function or a method has too many nested blocks, which makes the code less readable and harder to understand. Closes issue #668. diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index 3d9144d..5e941c5 100644 --- a/pylint/checkers/classes.py +++ b/pylint/checkers/classes.py @@ -140,6 +140,28 @@ def _has_bare_super_call(fundef_node): return True return False +def _safe_infer_call_result(node, caller, context=None): + """ + Safely infer the return value of a function. + + Returns None if inference failed or if there is some ambiguity (more than + one node has been inferred). Otherwise returns infered value. + """ + try: + inferit = node.infer_call_result(caller, context=context) + value = next(inferit) + except astroid.InferenceError: + return # inference failed + except StopIteration: + return # no values infered + try: + next(inferit) + return # there is ambiguity on the inferred node + except astroid.InferenceError: + return # there is some kind of ambiguity + except StopIteration: + return value + MSGS = { 'F0202': ('Unable to check methods signature (%s / %s)', 'method-check-failed', @@ -1066,13 +1088,10 @@ class SpecialMethodsChecker(BaseChecker): return False def _check_iter(self, node): - try: - infered = node.infer_call_result(node) - except astroid.InferenceError: - return - - if not all(map(self._is_iterator, infered)): - self.add_message('non-iterator-returned', node=node) + infered = _safe_infer_call_result(node, node) + if infered is not None: + if not self._is_iterator(infered): + self.add_message('non-iterator-returned', node=node) def _ancestors_to_call(klass_node, method='__init__'): diff --git a/pylint/checkers/design_analysis.py b/pylint/checkers/design_analysis.py index 432c96a..fc10e3f 100644 --- a/pylint/checkers/design_analysis.py +++ b/pylint/checkers/design_analysis.py @@ -18,7 +18,7 @@ import re from collections import defaultdict -from astroid import If +from astroid import If, BoolOp from pylint.interfaces import IAstroidChecker from pylint.checkers import BaseChecker @@ -64,9 +64,27 @@ MSGS = { 'too-many-statements', 'Used when a function or method has too many statements. You \ should then split it in smaller functions / methods.'), + 'R0916': ('Too many boolean expressions in if statement (%s/%s)', + 'too-many-boolean-expressions', + 'Used when a if statement contains too many boolean ' + 'expressions'), } +def _count_boolean_expressions(bool_op): + """Counts the number of boolean expressions in BoolOp `bool_op` (recursive) + + example: a and (b or c or (d and e)) ==> 5 boolean expressions + """ + nb_bool_expr = 0 + for bool_expr in bool_op.get_children(): + if isinstance(bool_expr, BoolOp): + nb_bool_expr += _count_boolean_expressions(bool_expr) + else: + nb_bool_expr += 1 + return nb_bool_expr + + class MisdesignChecker(BaseChecker): """checks for sign of poor/misdesign: * number of methods, attributes, local variables... @@ -136,6 +154,13 @@ class MisdesignChecker(BaseChecker): 'help' : 'Maximum number of public methods for a class \ (see R0904).'} ), + ('max-bool-expr', + {'default': 5, + 'type': 'int', + 'metavar': '<num>', + 'help': 'Maximum number of boolean expressions in a if ' + 'statement'} + ), ) def __init__(self, linter=None): @@ -275,8 +300,10 @@ class MisdesignChecker(BaseChecker): self._inc_branch(node, 2) self._stmts += 2 + @check_messages('too-many-boolean-expressions') def visit_if(self, node): - """increments the branches counter""" + """increments the branches counter and checks boolean expressions""" + self._check_boolean_expressions(node) branches = 1 # don't double count If nodes coming from some 'elif' if node.orelse and (len(node.orelse) > 1 or @@ -285,6 +312,19 @@ class MisdesignChecker(BaseChecker): self._inc_branch(node, branches) self._stmts += branches + def _check_boolean_expressions(self, node): + """Go through "if" node `node` and counts its boolean expressions + + if the "if" node test is a BoolOp node + """ + condition = node.test + if not isinstance(condition, BoolOp): + return + nb_bool_expr = _count_boolean_expressions(condition) + if nb_bool_expr > self.config.max_bool_expr: + self.add_message('too-many-boolean-expressions', node=condition, + args=(nb_bool_expr, self.config.max_bool_expr)) + def visit_while(self, node): """increments the branches counter""" branches = 1 diff --git a/pylint/checkers/raw_metrics.py b/pylint/checkers/raw_metrics.py index 1cfff83..5846cd1 100644 --- a/pylint/checkers/raw_metrics.py +++ b/pylint/checkers/raw_metrics.py @@ -118,7 +118,7 @@ def get_type(tokens, start_index): i += 1 if line_type is None: line_type = 'empty_lines' - elif i < len(tokens) and tok_type == tokenize.NEWLINE: + elif i < len(tokens) and tokens[i][0] == tokenize.NEWLINE: i += 1 return i, pos[0] - start[0] + 1, line_type diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 6f0f5a3..d813e1f 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -21,6 +21,7 @@ import re from copy import copy import astroid +from astroid import helpers from astroid import modutils from pylint.interfaces import IAstroidChecker, INFERENCE, INFERENCE_FAILURE, HIGH @@ -1033,7 +1034,8 @@ builtins. Remember that you should avoid to define new builtins when possible.' targets = node.targets[0].itered() try: - for infered in node.value.infer(): + infered = helpers.safe_infer(node.value) + if infered is not None: self._check_unpacking(infered, node, targets) except astroid.InferenceError: return diff --git a/pylint/pyreverse/diagrams.py b/pylint/pyreverse/diagrams.py index dfc03f2..ef7fe90 100644 --- a/pylint/pyreverse/diagrams.py +++ b/pylint/pyreverse/diagrams.py @@ -18,10 +18,13 @@ import astroid from pylint.pyreverse.utils import is_interface, FilterMixIn +from pylint.checkers.utils import decorated_with_property + class Figure(object): """base class for counter handling""" + class Relationship(Figure): """a relation ship from an object in the diagram to another """ @@ -41,6 +44,7 @@ class DiagramEntity(Figure): self.title = title self.node = node + class ClassDiagram(Figure, FilterMixIn): """main class diagram handling """ @@ -77,8 +81,13 @@ class ClassDiagram(Figure, FilterMixIn): def get_attrs(self, node): """return visible attributes, possibly with class name""" attrs = [] + properties = [ + (n, m) for n, m in node.items() + if isinstance(m, astroid.FunctionDef) + and decorated_with_property(m) + ] for node_name, ass_nodes in list(node.instance_attrs_type.items()) + \ - list(node.locals_type.items()): + list(node.locals_type.items()) + properties: if not self.show_attr(node_name): continue names = self.class_names(ass_nodes) @@ -91,7 +100,9 @@ class ClassDiagram(Figure, FilterMixIn): """return visible methods""" methods = [ m for m in node.values() - if isinstance(m, astroid.FunctionDef) and self.show_attr(m.name) + if isinstance(m, astroid.FunctionDef) + and not decorated_with_property(m) + and self.show_attr(m.name) ] return sorted(methods, key=lambda n: n.name) diff --git a/pylint/test/functional/non_iterator_returned.py b/pylint/test/functional/non_iterator_returned.py index 804ceee..d2fa758 100644 --- a/pylint/test/functional/non_iterator_returned.py +++ b/pylint/test/functional/non_iterator_returned.py @@ -56,6 +56,19 @@ class FifthGoodIterator(object): def __iter__(self): return IteratorClass +class FileBasedIterator(object): + def __init__(self, path): + self.path = path + self.file = None + + def __iter__(self): + if self.file is not None: + self.file.close() + self.file = open(self.path) + # self file has two infered values: None and <instance of 'file'> + # we don't want to emit error in this case + return self.file + class FirstBadIterator(object): """ __iter__ returns a list """ @@ -80,11 +93,3 @@ class FourthBadIterator(object): def __iter__(self): # [non-iterator-returned] return ThirdBadIterator - -class FifthBadIterator(object): - """All branches should return an iterator.""" - - def __iter__(self): # [non-iterator-returned] - if self: - return 1 - return SecondGoodIterator() diff --git a/pylint/test/functional/non_iterator_returned.txt b/pylint/test/functional/non_iterator_returned.txt index fe3db10..fa1d5be 100644 --- a/pylint/test/functional/non_iterator_returned.txt +++ b/pylint/test/functional/non_iterator_returned.txt @@ -1,5 +1,4 @@ -non-iterator-returned:63:FirstBadIterator.__iter__:__iter__ returns non-iterator -non-iterator-returned:69:SecondBadIterator.__iter__:__iter__ returns non-iterator -non-iterator-returned:75:ThirdBadIterator.__iter__:__iter__ returns non-iterator -non-iterator-returned:81:FourthBadIterator.__iter__:__iter__ returns non-iterator -non-iterator-returned:87:FifthBadIterator.__iter__:__iter__ returns non-iterator
\ No newline at end of file +non-iterator-returned:76:FirstBadIterator.__iter__:__iter__ returns non-iterator +non-iterator-returned:82:SecondBadIterator.__iter__:__iter__ returns non-iterator +non-iterator-returned:88:ThirdBadIterator.__iter__:__iter__ returns non-iterator +non-iterator-returned:94:FourthBadIterator.__iter__:__iter__ returns non-iterator diff --git a/pylint/test/functional/too_many_boolean_expressions.py b/pylint/test/functional/too_many_boolean_expressions.py new file mode 100644 index 0000000..b3dc238 --- /dev/null +++ b/pylint/test/functional/too_many_boolean_expressions.py @@ -0,0 +1,20 @@ +"""Checks for if statements containing too many boolean expressions""" + +# pylint: disable=invalid-name + +x = y = z = 5 +if x > -5 and x < 5 and y > -5 and y < 5 and z > -5 and z < 5: # [too-many-boolean-expressions] + pass +elif True and False and 1 and 2 and 3: + pass +elif True and False and 1 and 2 and 3 and 4 and 5: # [too-many-boolean-expressions] + pass +elif True and (True and True) and (x == 5 or True or True): # [too-many-boolean-expressions] + pass +elif True and (True or (x > -5 and x < 5 and (z > -5 or z < 5))): # [too-many-boolean-expressions] + pass +elif True == True == True == True == True == True: + pass + +if True and False and 1 and 2 and 3: + pass diff --git a/pylint/test/functional/too_many_boolean_expressions.txt b/pylint/test/functional/too_many_boolean_expressions.txt new file mode 100644 index 0000000..0bb0086 --- /dev/null +++ b/pylint/test/functional/too_many_boolean_expressions.txt @@ -0,0 +1,4 @@ +too-many-boolean-expressions:6::Too many boolean expressions in if statement (6/5) +too-many-boolean-expressions:10::Too many boolean expressions in if statement (7/5) +too-many-boolean-expressions:12::Too many boolean expressions in if statement (6/5) +too-many-boolean-expressions:14::Too many boolean expressions in if statement (6/5) diff --git a/pylint/test/functional/too_many_nested_blocks.py b/pylint/test/functional/too_many_nested_blocks.py index 8d371dd..47dbf44 100644 --- a/pylint/test/functional/too_many_nested_blocks.py +++ b/pylint/test/functional/too_many_nested_blocks.py @@ -28,7 +28,7 @@ def my_function(): while True: if True: if True: - print i + yield i nested_func() diff --git a/pylint/test/functional/unbalanced_tuple_unpacking.py b/pylint/test/functional/unbalanced_tuple_unpacking.py index bd21a05..98f5640 100644 --- a/pylint/test/functional/unbalanced_tuple_unpacking.py +++ b/pylint/test/functional/unbalanced_tuple_unpacking.py @@ -46,8 +46,8 @@ def temp(): return [2, 3, 4] def do_stuff7(): - """ This is not right """ - first, second = temp() # [unbalanced-tuple-unpacking] + """ This is not right, but we're not sure """ + first, second = temp() return first + second def temp2(): @@ -74,7 +74,9 @@ class UnbalancedUnpacking(object): # pylint: disable=attribute-defined-outside-init, invalid-name, too-few-public-methods def test(self): """ unpacking in instance attributes """ - self.a, self.b = temp() # [unbalanced-tuple-unpacking] + # we're not sure if temp() returns two or three values + # so we shouldn't emit an error + self.a, self.b = temp() self.a, self.b = temp2() self.a, self.b = unpack() # [unbalanced-tuple-unpacking] diff --git a/pylint/test/functional/unbalanced_tuple_unpacking.txt b/pylint/test/functional/unbalanced_tuple_unpacking.txt index 23038fb..e904209 100644 --- a/pylint/test/functional/unbalanced_tuple_unpacking.txt +++ b/pylint/test/functional/unbalanced_tuple_unpacking.txt @@ -2,7 +2,5 @@ unbalanced-tuple-unpacking:9:do_stuff:"Possible unbalanced tuple unpacking with unbalanced-tuple-unpacking:14:do_stuff1:"Possible unbalanced tuple unpacking with sequence [1, 2, 3]: left side has 2 label(s), right side has 3 value(s)" unbalanced-tuple-unpacking:19:do_stuff2:"Possible unbalanced tuple unpacking with sequence (1, 2, 3): left side has 2 label(s), right side has 3 value(s)" unbalanced-tuple-unpacking:24:do_stuff3:"Possible unbalanced tuple unpacking with sequence (1, 2, 3): left side has 2 label(s), right side has 3 value(s)" -unbalanced-tuple-unpacking:50:do_stuff7:"Possible unbalanced tuple unpacking with sequence defined at line 46: left side has 2 label(s), right side has 3 value(s)" unbalanced-tuple-unpacking:69:do_stuff9:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.unpacking: left side has 2 label(s), right side has 3 value(s)" -unbalanced-tuple-unpacking:77:UnbalancedUnpacking.test:"Possible unbalanced tuple unpacking with sequence defined at line 46: left side has 2 label(s), right side has 3 value(s)" -unbalanced-tuple-unpacking:79:UnbalancedUnpacking.test:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.unpacking: left side has 2 label(s), right side has 3 value(s)"
\ No newline at end of file +unbalanced-tuple-unpacking:81:UnbalancedUnpacking.test:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.unpacking: left side has 2 label(s), right side has 3 value(s)" diff --git a/pylint/test/functional/unpacking_non_sequence.py b/pylint/test/functional/unpacking_non_sequence.py index f449d5b..1e5de23 100644 --- a/pylint/test/functional/unpacking_non_sequence.py +++ b/pylint/test/functional/unpacking_non_sequence.py @@ -75,19 +75,12 @@ a, b = IterClass class NonSeq(object): """ does nothing """ -def bad_unpacking(): - """ one return isn't unpackable """ - if True: - return None - return [1, 2] - a, b = NonSeq() # [unpacking-non-sequence] a, b = ValueError # [unpacking-non-sequence] a, b = None # [unpacking-non-sequence] a, b = 1 # [unpacking-non-sequence] a, b = nonseq # [unpacking-non-sequence] a, b = nonseq() # [unpacking-non-sequence] -a, b = bad_unpacking() # [unpacking-non-sequence] a, b = nonseq_func # [unpacking-non-sequence] class ClassUnpacking(object): @@ -105,5 +98,22 @@ class ClassUnpacking(object): self.a, self.b = NonSeq() # [unpacking-non-sequence] self.a, self.b = ValueError # [unpacking-non-sequence] - self.a, self.b = bad_unpacking() # [unpacking-non-sequence] self.a, c = nonseq_func # [unpacking-non-sequence] + +class TestBase(object): + 'base class with `test` method implementation' + @staticmethod + def test(data): + 'default implementation' + return data + +class Test(TestBase): + 'child class that overrides `test` method' + def __init__(self): + # no error should be emitted here as `test` is overriden in this class + (self.aaa, self.bbb, self.ccc) = self.test(None) + + @staticmethod + def test(data): + 'overridden implementation' + return (1, 2, 3) diff --git a/pylint/test/functional/unpacking_non_sequence.txt b/pylint/test/functional/unpacking_non_sequence.txt index 8d13c44..0a21fab 100644 --- a/pylint/test/functional/unpacking_non_sequence.txt +++ b/pylint/test/functional/unpacking_non_sequence.txt @@ -1,12 +1,10 @@ -unpacking-non-sequence:84::Attempting to unpack a non-sequence defined at line 75 -unpacking-non-sequence:85::Attempting to unpack a non-sequence -unpacking-non-sequence:86::Attempting to unpack a non-sequence None -unpacking-non-sequence:87::Attempting to unpack a non-sequence 1 -unpacking-non-sequence:88::Attempting to unpack a non-sequence defined at line 9 of functional.unpacking -unpacking-non-sequence:89::Attempting to unpack a non-sequence defined at line 11 of functional.unpacking -unpacking-non-sequence:90::Attempting to unpack a non-sequence defined at line 81 -unpacking-non-sequence:91::Attempting to unpack a non-sequence -unpacking-non-sequence:106:ClassUnpacking.test:Attempting to unpack a non-sequence defined at line 75 -unpacking-non-sequence:107:ClassUnpacking.test:Attempting to unpack a non-sequence -unpacking-non-sequence:108:ClassUnpacking.test:Attempting to unpack a non-sequence defined at line 81 -unpacking-non-sequence:109:ClassUnpacking.test:Attempting to unpack a non-sequence +unpacking-non-sequence:78::Attempting to unpack a non-sequence defined at line 75 +unpacking-non-sequence:79::Attempting to unpack a non-sequence +unpacking-non-sequence:80::Attempting to unpack a non-sequence None +unpacking-non-sequence:81::Attempting to unpack a non-sequence 1 +unpacking-non-sequence:82::Attempting to unpack a non-sequence defined at line 9 of functional.unpacking +unpacking-non-sequence:83::Attempting to unpack a non-sequence defined at line 11 of functional.unpacking +unpacking-non-sequence:84::Attempting to unpack a non-sequence +unpacking-non-sequence:99:ClassUnpacking.test:Attempting to unpack a non-sequence defined at line 75 +unpacking-non-sequence:100:ClassUnpacking.test:Attempting to unpack a non-sequence +unpacking-non-sequence:101:ClassUnpacking.test:Attempting to unpack a non-sequence diff --git a/pylint/test/functional/unsubscriptable_value.py b/pylint/test/functional/unsubscriptable_value.py index a6bbecd..221bd17 100644 --- a/pylint/test/functional/unsubscriptable_value.py +++ b/pylint/test/functional/unsubscriptable_value.py @@ -82,3 +82,9 @@ def test(*args, **kwargs): test()[0] test[0] # [unsubscriptable-object] + +# deque +from collections import deque +deq = deque(maxlen=10) +deq.append(42) +deq[0] @@ -14,5 +14,6 @@ commands = pylint -rn --rcfile={toxinidir}/pylintrc {envsitepackagesdir}/pylint deps = hg+https://bitbucket.org/logilab/astroid@master six + commands = python -Wi -m unittest discover -s {envsitepackagesdir}/pylint/test/ -p {posargs:*test_*}.py -changedir = {toxworkdir}
\ No newline at end of file +changedir = {toxworkdir} |