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

5 files changed, 73 insertions, 3 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
diff --git a/ChangeLog b/ChangeLog
index 39b70c5..63aaa04 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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 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/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)