Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLE1310 fails on bytes.strip #15968

Closed
dscorbett opened this issue Feb 5, 2025 · 1 comment · Fixed by #15984
Closed

PLE1310 fails on bytes.strip #15968

dscorbett opened this issue Feb 5, 2025 · 1 comment · Fixed by #15984
Labels
bug Something isn't working rule Implementing or modifying a lint rule

Comments

@dscorbett
Copy link

Description

bad-str-strip-call (PLE1310) in Ruff 0.9.4 flags a bytes.strip call when its argument is a str, but that is a false positive because bytes.strip does not accept a str argument.

$ cat >ple1310.py <<'# EOF'
b"".strip("//")
# EOF

$ python ple1310.py 2>&1 | tail -n 1
TypeError: a bytes-like object is required, not 'str'

$ ruff --isolated check --select PLE1310 ple1310.py --output-format concise
ple1310.py:1:11: PLE1310 String `strip` call contains duplicate characters
Found 1 error.

PLE1310 does not flag a bytes.strip call when its argument is a bytes, which a false negative.

$ echo 'b"".strip(b"//")' | ruff --isolated check --select PLE1310 -
All checks passed!
@dylwil3 dylwil3 added bug Something isn't working rule Implementing or modifying a lint rule labels Feb 5, 2025
@InSyncWithFoo
Copy link
Contributor

There seems to be another, escaping-related bug:

''.strip('\\b\\x08')  # No error, even though `\` is repeated twice

dylwil3 pushed a commit that referenced this issue Feb 7, 2025
…match, remove custom escape handling logic (`PLE1310`) (#15984)

## Summary

Resolves #15968.

Previously, these would be considered violations:

```python
b''.strip('//')
''.lstrip('//', foo = "bar")
```

...while these are not:

```python
b''.strip(b'//')
''.strip('\\b\\x08')
```

Ruff will now not report when the types of the object and that of the
argument mismatch, or when there are extra arguments.

## Test Plan

`cargo nextest run` and `cargo insta test`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule Implementing or modifying a lint rule
Projects
None yet
3 participants