diff options
author | Laura Médioni <laura.medioni@logilab.fr> | 2015-10-29 14:33:24 +0100 |
---|---|---|
committer | Laura Médioni <laura.medioni@logilab.fr> | 2015-10-29 14:33:24 +0100 |
commit | 913a32cb0fcf91f58f80c5d1aa9fbe23751af5de (patch) | |
tree | 512690951d1ab8ae4db94430ce6d71573f7688e8 | |
parent | 40bb7e7ae67c3eac10c6287df579982194f81c72 (diff) | |
download | pylint-git-913a32cb0fcf91f58f80c5d1aa9fbe23751af5de.tar.gz |
check the number of boolean expressions in if statement is reasonnable
--max-bool-expr option allows to configure it (by default, up to 5 are
tolerated)
closes issue #677
-rw-r--r-- | CONTRIBUTORS.txt | 3 | ||||
-rw-r--r-- | ChangeLog | 5 | ||||
-rw-r--r-- | pylint/checkers/design_analysis.py | 44 | ||||
-rw-r--r-- | pylint/test/functional/too_many_boolean_expressions.py | 20 | ||||
-rw-r--r-- | pylint/test/functional/too_many_boolean_expressions.txt | 4 |
5 files changed, 73 insertions, 3 deletions
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index f4d66b8f7..83176785d 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 @@ -3,6 +3,11 @@ 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 diff --git a/pylint/checkers/design_analysis.py b/pylint/checkers/design_analysis.py index 432c96a30..fc10e3f96 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/test/functional/too_many_boolean_expressions.py b/pylint/test/functional/too_many_boolean_expressions.py new file mode 100644 index 000000000..b3dc2387e --- /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 000000000..0bb0086c1 --- /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) |