Add simplifiable-condition to the refactoring checker

ChangeLog

@@ -58,6 +58,11 @@ Release date: 2020-08-20
 
 * Add check for bool function to `len-as-condition`
 
+* Add `simplifiable-condition` check for extraneous constants in conditionals using and/or.
+
+* Add `condition-evals-to-constant` check for conditionals using and/or that evaluate to a constant.
+
+  Close #3407
 
 What's New in Pylint 2.5.4?
 ===========================

doc/whatsnew/2.6.rst

@@ -17,6 +17,10 @@ New checkers
 
 * Add `raise-missing-from` check for exceptions that should have a cause.
 
+* Add `simplifiable-condition` check for extraneous constants in conditionals using and/or.
+
+* Add `condition-evals-to-constant` check for conditionals using and/or that evaluate to a constant.
+
 Other Changes
 =============
 

pylint/checkers/refactoring.py

@@ -36,6 +36,7 @@
 """Looks for code which can be refactored."""
 import builtins
 import collections
+import copy
 import itertools
 import tokenize
 from functools import reduce
@@ -126,6 +127,8 @@ def _is_test_condition(
     parent = parent or node.parent
     if isinstance(parent, (astroid.While, astroid.If, astroid.IfExp, astroid.Assert)):
         return node is parent.test or parent.test.parent_of(node)
+    if isinstance(parent, astroid.Comprehension):
+        return node in parent.ifs
     return _is_call_of_name(parent, "bool") and parent.parent_of(node)
 
 
@@ -157,6 +160,16 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             "simplify-boolean-expression",
             "Emitted when redundant pre-python 2.5 ternary syntax is used.",
         ),
+        "R1726": (
+            "Boolean condition '%s' may be simplified to '%s'",
+            "simplifiable-condition",
+            "Emitted when a boolean condition is able to be simplified.",
+        ),
+        "R1727": (
+            "Boolean condition '%s' will always evaluate to '%s'",
+            "condition-evals-to-constant",
+            "Emitted when a boolean condition can be simplified to a constant value.",
+        ),
         "R1702": (
             "Too many nested blocks (%s/%s)",
             "too-many-nested-blocks",
@@ -361,6 +374,7 @@ def _init(self):
         self._elifs = []
         self._nested_blocks_msg = None
         self._reported_swap_nodes = set()
+        self._can_simplify_bool_op = False
 
     def open(self):
         # do this in open since config not fully initialized in __init__
@@ -992,13 +1006,92 @@ def _find_lower_upper_bounds(comparison_node, uses):
                 self.add_message("chained-comparison", node=node)
                 break
 
+    @staticmethod
+    def _apply_boolean_simplification_rules(operator, values):
+        """Removes irrelevant values or returns shortcircuiting values
+
+        This function applies the following two rules:
+        1) an OR expression with True in it will always be true, and the
+           reverse for AND
+
+        2) False values in OR expressions are only relevant if all values are
+           false, and the reverse for AND"""
+        simplified_values = []
+
+        for subnode in values:
+            inferred_bool = None
+            if not next(subnode.nodes_of_class(astroid.Name), False):
+                inferred = utils.safe_infer(subnode)
+                if inferred:
+                    inferred_bool = inferred.bool_value()
+
+            if not isinstance(inferred_bool, bool):
+                simplified_values.append(subnode)
+            elif (operator == "or") == inferred_bool:
+                return [subnode]
+
+        return simplified_values or [astroid.Const(operator == "and")]
+
+    def _simplify_boolean_operation(self, bool_op):
+        """Attempts to simplify a boolean operation
+
+        Recursively applies simplification on the operator terms,
+        and keeps track of whether reductions have been made."""
+        children = list(bool_op.get_children())
+        intermediate = [
+            self._simplify_boolean_operation(child)
+            if isinstance(child, astroid.BoolOp)
+            else child
+            for child in children
+        ]
+        result = self._apply_boolean_simplification_rules(bool_op.op, intermediate)
+        if len(result) < len(children):
+            self._can_simplify_bool_op = True
+        if len(result) == 1:
+            return result[0]
+        simplified_bool_op = copy.copy(bool_op)
+        simplified_bool_op.postinit(result)
+        return simplified_bool_op
+
+    def _check_simplifiable_condition(self, node):
+        """Check if a boolean condition can be simplified.
+
+        Variables will not be simplified, even in the value can be inferred,
+        and expressions like '3 + 4' will remain expanded."""
+        if not _is_test_condition(node):
+            return
+
+        self._can_simplify_bool_op = False
+        simplified_expr = self._simplify_boolean_operation(node)
+
+        if not self._can_simplify_bool_op:
+            return
+
+        if not next(simplified_expr.nodes_of_class(astroid.Name), False):
+            self.add_message(
+                "condition-evals-to-constant",
+                node=node,
+                args=(node.as_string(), simplified_expr.as_string()),
+            )
+        else:
+            self.add_message(
+                "simplifiable-condition",
+                node=node,
+                args=(node.as_string(), simplified_expr.as_string()),
+            )
+
     @utils.check_messages(
-        "consider-merging-isinstance", "consider-using-in", "chained-comparison"
+        "consider-merging-isinstance",
+        "consider-using-in",
+        "chained-comparison",
+        "simplifiable-condition",
+        "condition-evals-to-constant",
     )
     def visit_boolop(self, node):
         self._check_consider_merging_isinstance(node)
         self._check_consider_using_in(node)
         self._check_chained_comparison(node)
+        self._check_simplifiable_condition(node)
 
     @staticmethod
     def _is_simple_assignment(node):

tests/functional/c/condition_evals_to_constant.py

@@ -0,0 +1,46 @@
+"""Test that boolean conditions simplify to a constant value"""
+# pylint: disable=pointless-statement
+from unknown import Unknown  # pylint: disable=import-error
+
+
+def func(_):
+    """Pointless function"""
+
+
+CONSTANT = 100
+OTHER = 200
+
+# Simplifies any boolean expression that is coerced into a True/False value
+bool(CONSTANT or True)  # [condition-evals-to-constant]
+assert CONSTANT or True  # [condition-evals-to-constant]
+if CONSTANT and False:  # [condition-evals-to-constant]
+    pass
+elif CONSTANT and False:  # [condition-evals-to-constant]
+    pass
+while CONSTANT and False:  # [condition-evals-to-constant]
+    break
+1 if CONSTANT or True else 2  # [condition-evals-to-constant]
+z = [x for x in range(10) if x or True]  # [condition-evals-to-constant]
+
+# Simplifies recursively
+assert True or CONSTANT or OTHER  # [condition-evals-to-constant]
+assert (CONSTANT or True) or (CONSTANT or True)  # [condition-evals-to-constant]
+
+# Will try to infer the truthiness of an expression as long as it doesn't contain any variables
+assert 3 + 4 or CONSTANT  # [condition-evals-to-constant]
+assert Unknown or True  # [condition-evals-to-constant]
+
+assert True or True  # [condition-evals-to-constant]
+assert False or False  # [condition-evals-to-constant]
+assert True and True  # [condition-evals-to-constant]
+assert False and False  # [condition-evals-to-constant]
+
+
+# A bare constant that's not inside of a boolean operation will emit `using-constant-test` instead
+if True:  # pylint: disable=using-constant-test
+    pass
+
+# Expressions not in one of the above situations will not emit a message
+CONSTANT or True
+bool(CONSTANT or OTHER)
+bool(func(CONSTANT or True))

tests/functional/c/condition_evals_to_constant.txt

@@ -0,0 +1,15 @@
+condition-evals-to-constant:14::Boolean condition 'CONSTANT or True' will always evaluate to 'True'
+condition-evals-to-constant:15::Boolean condition 'CONSTANT or True' will always evaluate to 'True'
+condition-evals-to-constant:16::Boolean condition 'CONSTANT and False' will always evaluate to 'False'
+condition-evals-to-constant:18::Boolean condition 'CONSTANT and False' will always evaluate to 'False'
+condition-evals-to-constant:20::Boolean condition 'CONSTANT and False' will always evaluate to 'False'
+condition-evals-to-constant:22::Boolean condition 'CONSTANT or True' will always evaluate to 'True'
+condition-evals-to-constant:23::Boolean condition 'x or True' will always evaluate to 'True'
+condition-evals-to-constant:26::Boolean condition 'True or CONSTANT or OTHER' will always evaluate to 'True'
+condition-evals-to-constant:27::Boolean condition 'CONSTANT or True or CONSTANT or True' will always evaluate to 'True'
+condition-evals-to-constant:30::Boolean condition '3 + 4 or CONSTANT' will always evaluate to '3 + 4'
+condition-evals-to-constant:31::Boolean condition 'Unknown or True' will always evaluate to 'True'
+condition-evals-to-constant:33::Boolean condition 'True or True' will always evaluate to 'True'
+condition-evals-to-constant:34::Boolean condition 'False or False' will always evaluate to 'False'
+condition-evals-to-constant:35::Boolean condition 'True and True' will always evaluate to 'True'
+condition-evals-to-constant:36::Boolean condition 'False and False' will always evaluate to 'False'

tests/functional/c/consider_merging_isinstance.py

@@ -1,5 +1,5 @@
 """Checks use of consider-merging-isinstance"""
-# pylint:disable=line-too-long
+# pylint:disable=line-too-long, simplifiable-condition
 
 
 def isinstances():

tests/functional/l/len_checks.py

@@ -1,5 +1,5 @@
 # pylint: disable=too-few-public-methods,import-error, no-absolute-import,missing-docstring, misplaced-comparison-constant
-# pylint: disable=useless-super-delegation,wrong-import-position,invalid-name, wrong-import-order
+# pylint: disable=useless-super-delegation,wrong-import-position,invalid-name, wrong-import-order, condition-evals-to-constant
 
 if len('TEST'):  # [len-as-condition]
     pass

tests/functional/s/simplifiable_condition.py

@@ -0,0 +1,36 @@
+"""Test that boolean conditions can be simplified"""
+# pylint: disable=pointless-statement
+
+
+def func(_):
+    """Pointless function"""
+
+
+CONSTANT = 100
+OTHER = 200
+
+# Simplifies any boolean expression that is coerced into a True/False value
+bool(CONSTANT or False)  # [simplifiable-condition]
+assert CONSTANT or False  # [simplifiable-condition]
+if CONSTANT and True:  # [simplifiable-condition]
+    pass
+elif CONSTANT and True:  # [simplifiable-condition]
+    pass
+while CONSTANT and True:  # [simplifiable-condition]
+    break
+1 if CONSTANT or False else 2  # [simplifiable-condition]
+z = [x for x in range(10) if x or False]  # [simplifiable-condition]
+
+# Simplifies recursively
+assert CONSTANT or (True and False)  # [simplifiable-condition]
+assert True and CONSTANT and OTHER  # [simplifiable-condition]
+assert (CONSTANT or False) and (OTHER or True)  # [simplifiable-condition]
+
+# Will try to infer the truthiness of an expression as long as it doesn't contain any variables
+assert [] or CONSTANT  # [simplifiable-condition]
+assert {} or CONSTANT  # [simplifiable-condition]
+
+# Expressions not in one of the above situations will not emit a message
+CONSTANT or True
+bool(CONSTANT or OTHER)
+bool(func(CONSTANT or True))

tests/functional/s/simplifiable_condition.txt

@@ -0,0 +1,12 @@
+simplifiable-condition:13::Boolean condition 'CONSTANT or False' may be simplified to 'CONSTANT'
+simplifiable-condition:14::Boolean condition 'CONSTANT or False' may be simplified to 'CONSTANT'
+simplifiable-condition:15::Boolean condition 'CONSTANT and True' may be simplified to 'CONSTANT'
+simplifiable-condition:17::Boolean condition 'CONSTANT and True' may be simplified to 'CONSTANT'
+simplifiable-condition:19::Boolean condition 'CONSTANT and True' may be simplified to 'CONSTANT'
+simplifiable-condition:21::Boolean condition 'CONSTANT or False' may be simplified to 'CONSTANT'
+simplifiable-condition:22::Boolean condition 'x or False' may be simplified to 'x'
+simplifiable-condition:25::Boolean condition 'CONSTANT or True and False' may be simplified to 'CONSTANT'
+simplifiable-condition:26::Boolean condition 'True and CONSTANT and OTHER' may be simplified to 'CONSTANT and OTHER'
+simplifiable-condition:27::Boolean condition '(CONSTANT or False) and (OTHER or True)' may be simplified to 'CONSTANT'
+simplifiable-condition:30::Boolean condition '[] or CONSTANT' may be simplified to 'CONSTANT'
+simplifiable-condition:31::Boolean condition '{} or CONSTANT' may be simplified to 'CONSTANT'

tests/functional/too/too_many_boolean_expressions.py

@@ -1,6 +1,6 @@
 """Checks for if statements containing too many boolean expressions"""
 
-# pylint: disable=invalid-name, comparison-with-itself, chained-comparison
+# pylint: disable=invalid-name, comparison-with-itself, chained-comparison, condition-evals-to-constant
 
 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]

tests/functional/u/using_constant_test.py

@@ -1,7 +1,7 @@
 """Verify if constant tests are used inside if statements."""
 # pylint: disable=invalid-name, missing-docstring,too-few-public-methods
 # pylint: disable=no-init,expression-not-assigned, useless-object-inheritance
-# pylint: disable=missing-parentheses-for-call-in-test, unnecessary-comprehension
+# pylint: disable=missing-parentheses-for-call-in-test, unnecessary-comprehension, condition-evals-to-constant
 
 
 import collections
@@ -87,7 +87,6 @@ def test_comprehensions():
 if instance.method:  # [using-constant-test]
     pass
 
-
 # For these, we require to do inference, even though the result can be a
 # constant value. For some of them, we could determine that the test
 # is constant, such as 2 + 3, but the components of the BinOp

tests/input/func_typecheck_callfunc_assigment.py

@@ -1,4 +1,4 @@
-# pylint: disable=useless-return, useless-object-inheritance
+# pylint: disable=useless-return, useless-object-inheritance, condition-evals-to-constant
 """check assignment to function call where the function doesn't return
 
     'E1111': ('Assigning to function call which doesn\'t return',