diff options
Diffstat (limited to 'pylint')
-rw-r--r-- | pylint/checkers/base.py | 84 | ||||
-rw-r--r-- | pylint/test/functional/simplifiable_if_statement.py | 120 | ||||
-rw-r--r-- | pylint/test/functional/simplifiable_if_statement.txt | 4 |
3 files changed, 207 insertions, 1 deletions
diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index fe2578e..001a70c 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -1468,7 +1468,7 @@ class RecommandationChecker(_BasicChecker): 'consider-using-enumerate', 'Emitted when code that iterates with range and len is ' 'encountered. Such code can be simplified by using the ' - 'enumerate builtin.') + 'enumerate builtin.'), } @staticmethod @@ -1654,6 +1654,10 @@ class ElifChecker(BaseTokenChecker): 'Used when a function or a method has too many nested ' 'blocks. This makes the code less understandable and ' 'maintainable.'), + 'R0102': ('The if statement can be reduced by %s', + 'simplifiable-if-statement', + 'Used when an if statement can be reduced to a boolean ' + 'conversion of the statement\'s test. '), } options = (('max-nested-blocks', {'default' : 5, 'type' : 'int', 'metavar' : '<int>', @@ -1671,6 +1675,83 @@ class ElifChecker(BaseTokenChecker): self._if_counter = 0 self._nested_blocks_msg = None + @staticmethod + def _is_bool_const(node): + return (isinstance(node.value, astroid.Const) + and isinstance(node.value.value, bool)) + + def _is_actual_elif(self, node): + """Check if the given node is an actual elif + + This is a problem we're having with the builtin ast module, + which splits `elif` branches into a separate if statement. + Unfortunately we need to know the exact type in certain + cases. + """ + + if isinstance(node.parent, astroid.If): + orelse = node.parent.orelse + # current if node must directly follow a "else" + if orelse and orelse == [node]: + if self._elifs[self._if_counter]: + return True + return False + + def _check_simplifiable_if(self, node): + """Check if the given if node can be simplified. + + The if statement can be reduced to a boolean expression + in some cases. For instance, if there are two branches + and both of them return a boolean value that depends on + the result of the statement's test, then this can be reduced + to `bool(test)` without losing any functionality. + """ + + if self._is_actual_elif(node): + # Not interested in if statements with multiple branches. + return + if len(node.orelse) != 1 or len(node.body) != 1: + return + + # Check if both branches can be reduced. + first_branch = node.body[0] + else_branch = node.orelse[0] + if isinstance(first_branch, astroid.Return): + if not isinstance(else_branch, astroid.Return): + return + first_branch_is_bool = self._is_bool_const(first_branch) + else_branch_is_bool = self._is_bool_const(else_branch) + reduced_to = "returning bool of test" + elif isinstance(first_branch, astroid.Assign): + if not isinstance(else_branch, astroid.Assign): + return + first_branch_is_bool = self._is_bool_const(first_branch) + else_branch_is_bool = self._is_bool_const(else_branch) + reduced_to = "assigning bool of test" + else: + return + + if not first_branch_is_bool or not else_branch_is_bool: + return + if not first_branch.value.value: + # This is a case that can't be easily simplified and + # if it can be simplified, it will usually result in a + # code that's harder to understand and comprehend. + # Let's take for instance `arg and arg <= 3`. This could theoretically be + # reduced to `not arg or arg > 3`, but the net result is that now the + # condition is harder to understand, because it requires understanding of + # an extra clause: + # * first, there is the negation of truthness with `not arg` + # * the second clause is `arg > 3`, which occurs when arg has a + # a truth value, but it implies that `arg > 3` is equivalent + # with `arg and arg > 3`, which means that the user must + # think about this assumption when evaluating `arg > 3`. + # The original form is easier to grasp. + return + + self.add_message('simplifiable-if-statement', node=node, + args=(reduced_to, )) + def process_tokens(self, tokens): # Process tokens and look for 'if' or 'elif' for _, token, _, _, _ in tokens: @@ -1698,6 +1779,7 @@ class ElifChecker(BaseTokenChecker): @check_messages('too-many-nested-blocks') def visit_if(self, node): + self._check_simplifiable_if(node) self._check_nested_blocks(node) self._if_counter += 1 diff --git a/pylint/test/functional/simplifiable_if_statement.py b/pylint/test/functional/simplifiable_if_statement.py new file mode 100644 index 0000000..48d6dbb --- /dev/null +++ b/pylint/test/functional/simplifiable_if_statement.py @@ -0,0 +1,120 @@ +"""Test that some if statement tests can be simplified."""
+
+# pylint: disable=missing-docstring, invalid-name
+
+
+def test_simplifiable_1(arg):
+ # Simple test that can be replaced by bool(arg)
+ if arg: # [simplifiable-if-statement]
+ return True
+ else:
+ return False
+
+
+def test_simplifiable_2(arg, arg2):
+ # Can be reduced to bool(arg and not arg2)
+ if arg and not arg2: # [simplifiable-if-statement]
+ return True
+ else:
+ return False
+
+
+def test_simplifiable_3(arg, arg2):
+ # Can be reduced to bool(arg and not arg2)
+ if arg and not arg2: # [simplifiable-if-statement]
+ var = True
+ else:
+ var = False
+ return var
+
+
+def test_simplifiable_4(arg):
+ if arg:
+ var = True
+ else:
+ if arg == "arg1": # [simplifiable-if-statement]
+ return True
+ else:
+ return False
+ return var
+
+
+def test_not_necessarily_simplifiable_1(arg, arg2):
+ # Can be reduced to bool(not arg and not arg2) or to
+ # `not all(N)`, which is a bit harder to understand
+ # than `any(N)` when var should be False.
+ if arg or arg2:
+ var = False
+ else:
+ var = True
+ return var
+
+
+def test_not_necessarily_simplifiabile_2(arg):
+ # This could theoretically be reduced to `not arg or arg > 3`
+ # but the net result is that now the condition is harder to understand,
+ # because it requires understanding of an extra clause:
+ # * first, there is the negation of truthness with `not arg`
+ # * the second clause is `arg > 3`, which occurs when arg has a
+ # a truth value, but it implies that `arg > 3` is equivalent
+ # with `arg and arg > 3`, which means that the user must
+ # think about this assumption when evaluating `arg > 3`.
+ # The original form is easier to grasp.
+ if arg and arg <= 3:
+ return False
+ else:
+ return True
+
+
+def test_not_simplifiable_3(arg):
+ if arg:
+ test_not_necessarily_simplifiabile_2(arg)
+ test_not_necessarily_simplifiable_1(arg, arg)
+ return False
+ else:
+ if arg < 3:
+ test_simplifiable_3(arg, 42)
+ return True
+
+
+def test_not_simplifiable_4(arg):
+ # Not interested in multiple elifs
+ if arg == "any":
+ return True
+ elif test_not_simplifiable_3(arg) == arg:
+ return True
+ else:
+ return False
+
+
+def test_not_simplifiable_5(arg):
+ # Different actions in each branch
+ if arg == "any":
+ return True
+ else:
+ var = 42
+ return var
+
+
+def test_not_simplifiable_6(arg):
+ # Different actions in each branch
+ if arg == "any":
+ var = 42
+ else:
+ return True
+ return var
+
+def test_not_simplifiable_7(arg):
+ # Returning something different
+ if arg == "any":
+ return 4
+ else:
+ return 5
+
+
+def test_not_simplifiable_8(arg):
+ # Only one of the branch returns something boolean
+ if arg == "any":
+ return True
+ else:
+ return 0
diff --git a/pylint/test/functional/simplifiable_if_statement.txt b/pylint/test/functional/simplifiable_if_statement.txt new file mode 100644 index 0000000..74bdd16 --- /dev/null +++ b/pylint/test/functional/simplifiable_if_statement.txt @@ -0,0 +1,4 @@ +simplifiable-if-statement:8:test_simplifiable_1:The if statement can be reduced by returning bool of test
+simplifiable-if-statement:16:test_simplifiable_2:The if statement can be reduced by returning bool of test
+simplifiable-if-statement:24:test_simplifiable_3:The if statement can be reduced by assigning bool of test
+simplifiable-if-statement:35:test_simplifiable_4:The if statement can be reduced by returning bool of test
\ No newline at end of file |