Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions pylint/checkers/refactoring/implicit_booleaness_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,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"):
Expand All @@ -186,6 +187,8 @@ 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)
Comment on lines 188 to +189
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems highly likely that optimization or code deduplication can be found by re-using check_compare_to_str_or_zero here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what i understood i have added _extract_comparison_operands() helper method that both _check_compare_to_str_or_zero() and _check_len_comparison_with_zero() now use.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have issued new commits resolving this


def _check_compare_to_str_or_zero(self, node: nodes.Compare) -> None:
# Skip check for chained comparisons
Expand Down Expand Up @@ -406,6 +409,143 @@ 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.
"""
if len(node.ops) != 1:
return

operator, right_operand = node.ops[0]
left_operand = node.left

# 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()

# 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 (
len_arg_name
if self._in_boolean_context(node)
else f"bool({len_arg_name})"
)
if operator in {"<", "<=", ">=", ">"}:
return f"not {len_arg_name}"
if constant_value == 1:
if operator == ">=":
return (
len_arg_name
if self._in_boolean_context(node)
else f"bool({len_arg_name})"
)
if operator == "<":
return f"not {len_arg_name}"
elif constant_value == 0:
if operator == "==":
return f"not {len_arg_name}"
if operator == "!=":
return (
len_arg_name
if self._in_boolean_context(node)
else f"bool({len_arg_name})"
)
if operator == ">":
return (
len_arg_name
if self._in_boolean_context(node)
else f"bool({len_arg_name})"
)
if operator in {"<", "<=", ">="}:
return f"not {len_arg_name}"
elif constant_value == 1:
if operator == "<=":
return f"not {len_arg_name}"
if operator == ">":
return (
len_arg_name
if self._in_boolean_context(node)
else f"bool({len_arg_name})"
)

return None

@staticmethod
def base_names_of_instance(
node: util.UninferableBase | bases.Instance,
Expand Down
22 changes: 11 additions & 11 deletions tests/functional/u/use/use_implicit_booleaness_not_len.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,25 @@
if True or len('TEST'): # [use-implicit-booleaness-not-len]
pass

if len('TEST') == 0: # Should be fine
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to check the initial reasoning for this before changing, I didn't have this context when labelling the initial issue a false negative.

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
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)")
Expand Down
10 changes: 10 additions & 0 deletions tests/functional/u/use/use_implicit_booleaness_not_len.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,22 @@ 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
use-implicit-booleaness-not-len:64:6:64:17::Do not use `len(SEQUENCE)` without comparison to determine if a sequence is empty:INFERENCE
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
Expand All @@ -20,6 +29,7 @@ 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
Loading