Skip to content

Commit

Permalink
[pylint] Implement len-test (PLC1802) (#14309)
Browse files Browse the repository at this point in the history
## Summary

This PR implements [`use-implicit-booleaness-not-len` /
`C1802`](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/use-implicit-booleaness-not-len.html)
> For sequences, (strings, lists, tuples), use the fact that empty
sequences are false.

---------

Co-authored-by: xbrtnik1 <[email protected]>
Co-authored-by: xbrtnik1 <[email protected]>
  • Loading branch information
3 people authored Nov 26, 2024
1 parent 9f446fa commit 82c01aa
Show file tree
Hide file tree
Showing 8 changed files with 871 additions and 0 deletions.
234 changes: 234 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pylint/len_as_condition.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
if len('TEST'): # [PLC1802]
pass

if not len('TEST'): # [PLC1802]
pass

z = []
if z and len(['T', 'E', 'S', 'T']): # [PLC1802]
pass

if True or len('TEST'): # [PLC1802]
pass

if len('TEST') == 0: # Should be fine
pass

if len('TEST') < 1: # Should be fine
pass

if len('TEST') <= 0: # Should be fine
pass

if 1 > len('TEST'): # Should be fine
pass

if 0 >= len('TEST'): # Should be fine
pass

if z and len('TEST') == 0: # Should be fine
pass

if 0 == len('TEST') < 10: # Should be fine
pass

# Should be fine
if 0 < 1 <= len('TEST') < 10: # [comparison-of-constants]
pass

if 10 > len('TEST') != 0: # Should be fine
pass

if 10 > len('TEST') > 1 > 0: # Should be fine
pass

if 0 <= len('TEST') < 100: # Should be fine
pass

if z or 10 > len('TEST') != 0: # Should be fine
pass

if z:
pass
elif len('TEST'): # [PLC1802]
pass

if z:
pass
elif not len('TEST'): # [PLC1802]
pass

while len('TEST'): # [PLC1802]
pass

while not len('TEST'): # [PLC1802]
pass

while z and len('TEST'): # [PLC1802]
pass

while not len('TEST') and z: # [PLC1802]
pass

assert len('TEST') > 0 # Should be fine

x = 1 if len('TEST') != 0 else 2 # Should be fine

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

def github_issue_1325():
l = [1, 2, 3]
length = len(l) if l else 0 # Should be fine
return length

def github_issue_1331(*args):
assert False, len(args) # Should be fine

def github_issue_1331_v2(*args):
assert len(args), args # [PLC1802]

def github_issue_1331_v3(*args):
assert len(args) or z, args # [PLC1802]

def github_issue_1331_v4(*args):
assert z and len(args), args # [PLC1802]

def github_issue_1331_v5(**args):
assert z and len(args), args # [PLC1802]

b = bool(len(z)) # [PLC1802]
c = bool(len('TEST') or 42) # [PLC1802]

def github_issue_1879():

class ClassWithBool(list):
def __bool__(self):
return True

class ClassWithoutBool(list):
pass

class ChildClassWithBool(ClassWithBool):
pass

class ChildClassWithoutBool(ClassWithoutBool):
pass

assert len(ClassWithBool())
assert len(ChildClassWithBool())
assert len(ClassWithoutBool()) # unintuitive?, in pylint: [PLC1802]
assert len(ChildClassWithoutBool()) # unintuitive?, in pylint: [PLC1802]
assert len(range(0)) # [PLC1802]
assert len([t + 1 for t in []]) # [PLC1802]
# assert len(u + 1 for u in []) generator has no len
assert len({"1":(v + 1) for v in {}}) # [PLC1802]
assert len(set((w + 1) for w in set())) # [PLC1802]


import numpy
numpy_array = numpy.array([0])
if len(numpy_array) > 0:
print('numpy_array')
if len(numpy_array):
print('numpy_array')
if numpy_array:
print('b')

import pandas as pd
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:
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)")

def function_returning_list(r):
if r==1:
return [1]
return [2]

def function_returning_int(r):
if r==1:
return 1
return 2

def function_returning_generator(r):
for i in [r, 1, 2, 3]:
yield i

def function_returning_comprehension(r):
return [x+1 for x in [r, 1, 2, 3]]

def function_returning_function(r):
return function_returning_generator(r)

assert len(function_returning_list(z)) # [PLC1802] differs from pylint
assert len(function_returning_int(z))
# This should raise a PLC1802 once astroid can infer it
# See https://github.com/pylint-dev/pylint/pull/3821#issuecomment-743771514
assert len(function_returning_generator(z))
assert len(function_returning_comprehension(z))
assert len(function_returning_function(z))


def github_issue_4215():
# Test undefined variables
# https://github.com/pylint-dev/pylint/issues/4215
if len(undefined_var): # [undefined-variable]
pass
if len(undefined_var2[0]): # [undefined-variable]
pass


def f(cond:bool):
x = [1,2,3]
if cond:
x = [4,5,6]
if len(x): # this should be addressed
print(x)

def g(cond:bool):
x = [1,2,3]
if cond:
x = [4,5,6]
if len(x): # this should be addressed
print(x)
del x

def h(cond:bool):
x = [1,2,3]
x = 123
if len(x): # ok
print(x)

def outer():
x = [1,2,3]
def inner(x:int):
return x+1
if len(x): # [PLC1802]
print(x)

def redefined():
x = 123
x = [1, 2, 3]
if len(x): # this should be addressed
print(x)

global_seq = [1, 2, 3]

def i():
global global_seq
if len(global_seq): # ok
print(global_seq)

def j():
if False:
x = [1, 2, 3]
if len(x): # [PLC1802] should be fine
print(x)
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::SuperWithoutBrackets) {
pylint::rules::super_without_brackets(checker, func);
}
if checker.enabled(Rule::LenTest) {
pylint::rules::len_test(checker, call);
}
if checker.enabled(Rule::BitCount) {
refurb::rules::bit_count(checker, call);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "C0208") => (RuleGroup::Stable, rules::pylint::rules::IterationOverSet),
(Pylint, "C0414") => (RuleGroup::Stable, rules::pylint::rules::UselessImportAlias),
(Pylint, "C0415") => (RuleGroup::Preview, rules::pylint::rules::ImportOutsideTopLevel),
(Pylint, "C1802") => (RuleGroup::Preview, rules::pylint::rules::LenTest),
(Pylint, "C1901") => (RuleGroup::Preview, rules::pylint::rules::CompareToEmptyString),
(Pylint, "C2401") => (RuleGroup::Stable, rules::pylint::rules::NonAsciiName),
(Pylint, "C2403") => (RuleGroup::Stable, rules::pylint::rules::NonAsciiImportName),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ mod tests {
Rule::BadStaticmethodArgument,
Path::new("bad_staticmethod_argument.py")
)]
#[test_case(Rule::LenTest, Path::new("len_as_condition.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
Loading

0 comments on commit 82c01aa

Please sign in to comment.