diff options
| -rw-r--r-- | ChangeLog | 5 | ||||
| -rw-r--r-- | doc/whatsnew/2.6.rst | 4 | ||||
| -rw-r--r-- | pylint/checkers/refactoring.py | 95 | ||||
| -rw-r--r-- | tests/functional/c/condition_evals_to_constant.py | 46 | ||||
| -rw-r--r-- | tests/functional/c/condition_evals_to_constant.txt | 15 | ||||
| -rw-r--r-- | tests/functional/c/consider_merging_isinstance.py | 2 | ||||
| -rw-r--r-- | tests/functional/l/len_checks.py | 2 | ||||
| -rw-r--r-- | tests/functional/s/simplifiable_condition.py | 36 | ||||
| -rw-r--r-- | tests/functional/s/simplifiable_condition.txt | 12 | ||||
| -rw-r--r-- | tests/functional/too/too_many_boolean_expressions.py | 2 | ||||
| -rw-r--r-- | tests/functional/u/using_constant_test.py | 3 | ||||
| -rw-r--r-- | tests/input/func_typecheck_callfunc_assigment.py | 2 |
12 files changed, 217 insertions, 7 deletions
@@ -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? =========================== diff --git a/doc/whatsnew/2.6.rst b/doc/whatsnew/2.6.rst index 369b3fe99..54239d7f3 100644 --- a/doc/whatsnew/2.6.rst +++ b/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 ============= diff --git a/pylint/checkers/refactoring.py b/pylint/checkers/refactoring.py index 4d93299d9..9a66a71e7 100644 --- a/pylint/checkers/refactoring.py +++ b/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 @@ class RefactoringChecker(checkers.BaseTokenChecker): 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 @@ class RefactoringChecker(checkers.BaseTokenChecker): 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): diff --git a/tests/functional/c/condition_evals_to_constant.py b/tests/functional/c/condition_evals_to_constant.py new file mode 100644 index 000000000..cfd0b00f4 --- /dev/null +++ b/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)) diff --git a/tests/functional/c/condition_evals_to_constant.txt b/tests/functional/c/condition_evals_to_constant.txt new file mode 100644 index 000000000..5ff64887d --- /dev/null +++ b/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' diff --git a/tests/functional/c/consider_merging_isinstance.py b/tests/functional/c/consider_merging_isinstance.py index 34068d9b7..d3387bd5c 100644 --- a/tests/functional/c/consider_merging_isinstance.py +++ b/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(): diff --git a/tests/functional/l/len_checks.py b/tests/functional/l/len_checks.py index d7db14ddd..216a7e672 100644 --- a/tests/functional/l/len_checks.py +++ b/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 diff --git a/tests/functional/s/simplifiable_condition.py b/tests/functional/s/simplifiable_condition.py new file mode 100644 index 000000000..32bf36aca --- /dev/null +++ b/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)) diff --git a/tests/functional/s/simplifiable_condition.txt b/tests/functional/s/simplifiable_condition.txt new file mode 100644 index 000000000..3649550c2 --- /dev/null +++ b/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' diff --git a/tests/functional/too/too_many_boolean_expressions.py b/tests/functional/too/too_many_boolean_expressions.py index 68214ab8e..e8753859c 100644 --- a/tests/functional/too/too_many_boolean_expressions.py +++ b/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] diff --git a/tests/functional/u/using_constant_test.py b/tests/functional/u/using_constant_test.py index d6624daa6..7e902e021 100644 --- a/tests/functional/u/using_constant_test.py +++ b/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 @@ if Class.method: # [using-constant-test] 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 diff --git a/tests/input/func_typecheck_callfunc_assigment.py b/tests/input/func_typecheck_callfunc_assigment.py index 36d476e89..e493014e2 100644 --- a/tests/input/func_typecheck_callfunc_assigment.py +++ b/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', |