diff --git a/doc/data/messages/u/use-implicit-booleaness-not-len/bad.py b/doc/data/messages/u/use-implicit-booleaness-not-len/bad.py index 722282a593..0cdbb83e78 100644 --- a/doc/data/messages/u/use-implicit-booleaness-not-len/bad.py +++ b/doc/data/messages/u/use-implicit-booleaness-not-len/bad.py @@ -2,3 +2,13 @@ if len(fruits): # [use-implicit-booleaness-not-len] print(fruits) + +# Now also catches comparisons with zero +if len(fruits) == 0: # [use-implicit-booleaness-not-len] + print("empty") + +if len(fruits) > 0: # [use-implicit-booleaness-not-len] + print("has items") + +if len(fruits) != 0: # [use-implicit-booleaness-not-len] + print("not empty") diff --git a/doc/data/messages/u/use-implicit-booleaness-not-len/good.py b/doc/data/messages/u/use-implicit-booleaness-not-len/good.py index 0e84921487..14a7c5f444 100644 --- a/doc/data/messages/u/use-implicit-booleaness-not-len/good.py +++ b/doc/data/messages/u/use-implicit-booleaness-not-len/good.py @@ -2,3 +2,13 @@ if fruits: print(fruits) + +# Use implicit booleaness instead of comparing with zero +if not fruits: + print("empty") + +if fruits: + print("has items") + +if fruits: + print("not empty") diff --git a/pylint/checkers/refactoring/implicit_booleaness_checker.py b/pylint/checkers/refactoring/implicit_booleaness_checker.py index f3a3c6c964..a39543c8cb 100644 --- a/pylint/checkers/refactoring/implicit_booleaness_checker.py +++ b/pylint/checkers/refactoring/implicit_booleaness_checker.py @@ -4,8 +4,6 @@ from __future__ import annotations -import itertools - import astroid from astroid import bases, nodes, util @@ -176,6 +174,7 @@ def visit_unaryop(self, node: nodes.UnaryOp) -> None: "use-implicit-booleaness-not-comparison", "use-implicit-booleaness-not-comparison-to-string", "use-implicit-booleaness-not-comparison-to-zero", + "use-implicit-booleaness-not-len", ) def visit_compare(self, node: nodes.Compare) -> None: if self.linter.is_message_enabled("use-implicit-booleaness-not-comparison"): @@ -186,21 +185,29 @@ def visit_compare(self, node: nodes.Compare) -> None: "use-implicit-booleaness-not-comparison-to-str" ): self._check_compare_to_str_or_zero(node) + if self.linter.is_message_enabled("use-implicit-booleaness-not-len"): + self._check_len_comparison_with_zero(node) - def _check_compare_to_str_or_zero(self, node: nodes.Compare) -> None: - # Skip check for chained comparisons + def _extract_comparison_operands( + self, node: nodes.Compare + ) -> tuple[nodes.NodeNG, str, nodes.NodeNG] | None: + """Extract left operand, operator, and right operand from a comparison. + + Returns None if this is a chained comparison. + """ if len(node.ops) != 1: + return None + operator, right_operand = node.ops[0] + left_operand = node.left + return left_operand, operator, right_operand + + def _check_compare_to_str_or_zero(self, node: nodes.Compare) -> None: + operands = self._extract_comparison_operands(node) + if operands is None: return + left_operand, operator, right_operand = operands negation_redundant_ops = {"!=", "is not"} - # note: nodes.Compare has the left most operand in node.left - # while the rest are a list of tuples in node.ops - # the format of the tuple is ('compare operator sign', node) - # here we squash everything into `ops` to make it easier for processing later - ops: list[tuple[str, nodes.NodeNG]] = [("", node.left), *node.ops] - iter_ops = iter(ops) - all_ops = list(itertools.chain(*iter_ops)) - _, left_operand, operator, right_operand = all_ops if operator not in self._operators: return @@ -406,6 +413,128 @@ def _in_boolean_context(self, node: nodes.Compare) -> bool: return False + def _check_len_comparison_with_zero(self, node: nodes.Compare) -> None: + """Check for len() comparisons with zero that can be simplified using implicit + booleaness. + """ + operands = self._extract_comparison_operands(node) + if operands is None: + return + left_operand, operator, right_operand = operands + + # Check if one side is len() call and other is 0 or 1 + len_node = None + constant_value = None + is_len_on_left = False + + if utils.is_call_of_name(left_operand, "len"): + len_node = left_operand + is_len_on_left = True + if isinstance(right_operand, nodes.Const) and right_operand.value in {0, 1}: + constant_value = right_operand.value + elif utils.is_call_of_name(right_operand, "len"): + len_node = right_operand + is_len_on_left = False + if isinstance(left_operand, nodes.Const) and left_operand.value in {0, 1}: + constant_value = left_operand.value + + if len_node is None or constant_value is None: + return + + # Check if the comparison should be flagged + # The comparison could be nested in boolean operations, e.g. `if z or len(x) > 0:` + parent = node.parent + has_bool_op = False + while isinstance(parent, nodes.BoolOp): + has_bool_op = True + parent = parent.parent + + # Flag if it's in a test condition, OR directly returned (without being in a BoolOp) + is_test_cond = utils.is_test_condition(node, parent) + is_direct_return = isinstance(parent, nodes.Return) and not has_bool_op + + if not (is_test_cond or is_direct_return): + return + + len_arg = len_node.args[0] + + try: + instance = next(len_arg.infer()) + except astroid.InferenceError: + return + + # Check if this is a comprehension (special case handled separately) + if isinstance(len_arg, (nodes.ListComp, nodes.SetComp, nodes.DictComp)): + return + + mother_classes = self.base_names_of_instance(instance) + affected_by_pep8 = any( + t in mother_classes for t in ("str", "tuple", "list", "set") + ) + is_range = "range" in mother_classes + + # Only proceed if it's a sequence type and doesn't have custom __bool__ + if not (affected_by_pep8 or is_range or self.instance_has_bool(instance)): + return + + suggestion = self._get_len_comparison_suggestion( + operator, constant_value, is_len_on_left, node, len_arg + ) + if suggestion: + self.add_message( + "use-implicit-booleaness-not-len", + node=node, + confidence=HIGH, + ) + + def _get_len_comparison_suggestion( + self, + operator: str, + constant_value: int, + is_len_on_left: bool, + node: nodes.Compare, + len_arg: nodes.NodeNG, + ) -> str | None: + """Get the appropriate suggestion for len() comparisons with zero.""" + len_arg_name = len_arg.as_string() + + # Helper to get the appropriate positive suggestion + def get_positive_suggestion() -> str: + return ( + len_arg_name + if self._in_boolean_context(node) + else f"bool({len_arg_name})" + ) + + # Patterns that should be flagged + if is_len_on_left: + if constant_value == 0: + if operator == "==": + return f"not {len_arg_name}" + if operator == "!=": + return get_positive_suggestion() + if operator in {"<", "<=", ">=", ">"}: + return f"not {len_arg_name}" + elif constant_value == 1: + if operator == ">=": + return get_positive_suggestion() + if operator == "<": + return f"not {len_arg_name}" + elif constant_value == 0: + if operator == "==": + return f"not {len_arg_name}" + if operator in {"!=", ">"}: + return get_positive_suggestion() + if operator in {"<", "<=", ">="}: + return f"not {len_arg_name}" + elif constant_value == 1: + if operator == "<=": + return get_positive_suggestion() + if operator == ">": + return f"not {len_arg_name}" + + return None + @staticmethod def base_names_of_instance( node: util.UninferableBase | bases.Instance, diff --git a/test_original_issue.py b/test_original_issue.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/functional/u/use/use_implicit_booleaness_not_len.py b/tests/functional/u/use/use_implicit_booleaness_not_len.py index 4a1063e757..516cf12cff 100644 --- a/tests/functional/u/use/use_implicit_booleaness_not_len.py +++ b/tests/functional/u/use/use_implicit_booleaness_not_len.py @@ -14,25 +14,25 @@ if True or len('TEST'): # [use-implicit-booleaness-not-len] pass -if len('TEST') == 0: # Should be fine +if len('TEST') == 0: # [use-implicit-booleaness-not-len] pass -if len('TEST') < 1: # Should be fine +if len('TEST') < 1: # [use-implicit-booleaness-not-len] pass -if len('TEST') <= 0: # Should be fine +if len('TEST') <= 0: # [use-implicit-booleaness-not-len] pass -if 1 > len('TEST'): # Should be fine +if 1 > len('TEST'): # [use-implicit-booleaness-not-len] pass -if 0 >= len('TEST'): # Should be fine +if 0 >= len('TEST'): # [use-implicit-booleaness-not-len] pass -if z and len('TEST') == 0: # Should be fine +if z and len('TEST') == 0: # [use-implicit-booleaness-not-len] pass -if 0 == len('TEST') < 10: # Should be fine +if 0 == len('TEST') < 10: # Should be fine (chained comparison) pass # Should be fine @@ -73,16 +73,16 @@ while not len('TEST') and z: # [use-implicit-booleaness-not-len] pass -assert len('TEST') > 0 # Should be fine +assert len('TEST') > 0 # [use-implicit-booleaness-not-len] -x = 1 if len('TEST') != 0 else 2 # Should be fine +x = 1 if len('TEST') != 0 else 2 # [use-implicit-booleaness-not-len] f_o_o = len('TEST') or 42 # Should be fine a = x and len(x) # Should be fine def some_func(): - return len('TEST') > 0 # Should be fine + return len('TEST') > 0 # [use-implicit-booleaness-not-len] def github_issue_1325(): l = [1, 2, 3] @@ -143,7 +143,7 @@ class ChildClassWithoutBool(ClassWithoutBool): pandas_df = pd.DataFrame() if len(pandas_df): print("this works, but pylint tells me not to use len() without comparison") - if len(pandas_df) > 0: + if len(pandas_df) > 0: # [use-implicit-booleaness-not-len] print("this works and pylint likes it, but it's not the solution intended by PEP-8") if pandas_df: print("this does not work (truth value of dataframe is ambiguous)") @@ -185,6 +185,40 @@ def github_issue_4215(): if len(undefined_var2[0]): # [undefined-variable] pass +def test_additional_coverage(): + """Test cases for additional code coverage.""" + # Test >= with 0 (always true, should flag) + if len('TEST') >= 0: # [use-implicit-booleaness-not-len] + pass + + # Test > with 0 in assignment (non-boolean context) - Should be fine + result = len('TEST') > 0 # Creating boolean variable for later use is OK + + # Test constant on left with <= (equivalent to len < 1) + if 1 <= len('TEST'): # [use-implicit-booleaness-not-len] + pass + + # Test >= with 1 in ternary expression + test_var = 1 if len('TEST') >= 1 else 0 # [use-implicit-booleaness-not-len] + + # Test > with constant 1 on the right + if 1 > len('TEST'): # [use-implicit-booleaness-not-len] + pass + + # Test <= with 1 on the right (constant on right) + if len('TEST') <= 1: # Should be fine - checking if len is 0 or 1, not just empty + pass + + # Test > operator with 1 on the left + if 1 < len('TEST'): # Should be fine - checking if len > 1, not just non-empty + pass + + # Test with range (should flag) + if len(range(10)) > 0: # [use-implicit-booleaness-not-len] + pass + + return result, test_var + # pylint: disable=len-as-condition if len('TEST'): diff --git a/tests/functional/u/use/use_implicit_booleaness_not_len.txt b/tests/functional/u/use/use_implicit_booleaness_not_len.txt index 78078751cd..7e3eda46b6 100644 --- a/tests/functional/u/use/use_implicit_booleaness_not_len.txt +++ b/tests/functional/u/use/use_implicit_booleaness_not_len.txt @@ -2,6 +2,12 @@ use-implicit-booleaness-not-len:4:3:4:14::Do not use `len(SEQUENCE)` without com use-implicit-booleaness-not-len:7:3:7:18::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH use-implicit-booleaness-not-len:11:9:11:34::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE use-implicit-booleaness-not-len:14:11:14:22::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE +use-implicit-booleaness-not-len:17:3:17:19::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:20:3:20:18::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:23:3:23:19::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:26:3:26:18::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:29:3:29:19::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:32:9:32:25::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH comparison-of-constants:39:3:39:28::"Comparison between constants: ""0 < 1"" has a constant value":HIGH use-implicit-booleaness-not-len:56:5:56:16::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE use-implicit-booleaness-not-len:61:5:61:20::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH @@ -9,6 +15,9 @@ use-implicit-booleaness-not-len:64:6:64:17::Do not use `len(SEQUENCE)` without c use-implicit-booleaness-not-len:67:6:67:21::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH use-implicit-booleaness-not-len:70:12:70:23::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE use-implicit-booleaness-not-len:73:6:73:21::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:76:7:76:22::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:78:9:78:25::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:85:11:85:26:some_func:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH use-implicit-booleaness-not-len:96:11:96:20:github_issue_1331_v2:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE use-implicit-booleaness-not-len:99:11:99:20:github_issue_1331_v3:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE use-implicit-booleaness-not-len:102:17:102:26:github_issue_1331_v4:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE @@ -20,6 +29,12 @@ use-implicit-booleaness-not-len:126:11:126:24:github_issue_1879:Do not use `len( use-implicit-booleaness-not-len:127:11:127:35:github_issue_1879:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH use-implicit-booleaness-not-len:129:11:129:41:github_issue_1879:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH use-implicit-booleaness-not-len:130:11:130:43:github_issue_1879:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE +use-implicit-booleaness-not-len:146:7:146:25:github_issue_1879:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH use-implicit-booleaness-not-len:171:11:171:42:github_issue_1879:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE undefined-variable:183:11:183:24:github_issue_4215:Undefined variable 'undefined_var':UNDEFINED undefined-variable:185:11:185:25:github_issue_4215:Undefined variable 'undefined_var2':UNDEFINED +use-implicit-booleaness-not-len:191:7:191:23:test_additional_coverage:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:198:7:198:23:test_additional_coverage:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:202:20:202:36:test_additional_coverage:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:205:7:205:22:test_additional_coverage:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH +use-implicit-booleaness-not-len:217:7:217:25:test_additional_coverage:Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:HIGH