[pylint] Better inference for str.strip (PLE310) (#16671)

## Summary
This PR stabilizes the behavior introduced in
https://github.com/astral-sh/ruff/pull/15985

The new behavior improves the inference of `str.strip` calls:

* before: The rule only considered calls on string or byte literals
(`"abcd".strip`)
* now: The rule also catches calls to `strip` on object where the type
is known to be a `str` or `bytes` (e.g. `a = "abc"; a.strip("//")`)


The new behavior shipped as part of Ruff 0.9.6 on the 10th of Feb which
is a little more than a month ago.
There have been now new issues or PRs related to the new behavior.
This commit is contained in:
Micha Reiser 2025-03-13 09:35:41 +01:00
parent 04ad562afd
commit d8159e816f
4 changed files with 28 additions and 235 deletions

View file

@ -16,10 +16,10 @@ mod tests {
use crate::registry::Rule; use crate::registry::Rule;
use crate::rules::{flake8_tidy_imports, pylint}; use crate::rules::{flake8_tidy_imports, pylint};
use crate::assert_messages;
use crate::settings::types::PreviewMode; use crate::settings::types::PreviewMode;
use crate::settings::LinterSettings; use crate::settings::LinterSettings;
use crate::test::test_path; use crate::test::test_path;
use crate::{assert_messages, settings};
#[test_case(Rule::SingledispatchMethod, Path::new("singledispatch_method.py"))] #[test_case(Rule::SingledispatchMethod, Path::new("singledispatch_method.py"))]
#[test_case( #[test_case(
@ -440,22 +440,4 @@ mod tests {
assert_messages!(diagnostics); assert_messages!(diagnostics);
Ok(()) Ok(())
} }
#[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("pylint").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Enabled,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
} }

View file

@ -187,12 +187,6 @@ pub(crate) fn bad_str_strip_call(checker: &Checker, call: &ast::ExprCall) {
let value = &**value; let value = &**value;
if checker.settings.preview.is_disabled()
&& !matches!(value, Expr::StringLiteral(_) | Expr::BytesLiteral(_))
{
return;
}
let Some(value_kind) = ValueKind::from(value, checker.semantic()) else { let Some(value_kind) = ValueKind::from(value, checker.semantic()) else {
return; return;
}; };

View file

@ -181,3 +181,30 @@ bad_str_strip_call.py:81:10: PLE1310 String `strip` call contains duplicate char
82 | 82 |
83 | # Errors: Type inference 83 | # Errors: Type inference
| |
bad_str_strip_call.py:85:9: PLE1310 String `strip` call contains duplicate characters
|
83 | # Errors: Type inference
84 | b = b''
85 | b.strip(b'//')
| ^^^^^ PLE1310
86 |
87 | # Errors: Type inference (preview)
|
bad_str_strip_call.py:89:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?)
|
87 | # Errors: Type inference (preview)
88 | foo: str = ""; bar: bytes = b""
89 | foo.rstrip("//")
| ^^^^ PLE1310
90 | bar.lstrip(b"//")
|
bad_str_strip_call.py:90:12: PLE1310 String `lstrip` call contains duplicate characters (did you mean `removeprefix`?)
|
88 | foo: str = ""; bar: bytes = b""
89 | foo.rstrip("//")
90 | bar.lstrip(b"//")
| ^^^^^ PLE1310
|

View file

@ -1,210 +0,0 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
bad_str_strip_call.py:2:21: PLE1310 String `strip` call contains duplicate characters
|
1 | # PLE1310
2 | "Hello World".strip("Hello")
| ^^^^^^^ PLE1310
3 |
4 | # PLE1310
|
bad_str_strip_call.py:5:21: PLE1310 String `strip` call contains duplicate characters
|
4 | # PLE1310
5 | "Hello World".strip("Hello")
| ^^^^^^^ PLE1310
6 |
7 | # PLE1310
|
bad_str_strip_call.py:8:21: PLE1310 String `strip` call contains duplicate characters
|
7 | # PLE1310
8 | "Hello World".strip(u"Hello")
| ^^^^^^^^ PLE1310
9 |
10 | # PLE1310
|
bad_str_strip_call.py:11:21: PLE1310 String `strip` call contains duplicate characters
|
10 | # PLE1310
11 | "Hello World".strip(r"Hello")
| ^^^^^^^^ PLE1310
12 |
13 | # PLE1310
|
bad_str_strip_call.py:14:21: PLE1310 String `strip` call contains duplicate characters
|
13 | # PLE1310
14 | "Hello World".strip("Hello\t")
| ^^^^^^^^^ PLE1310
15 |
16 | # PLE1310
|
bad_str_strip_call.py:17:21: PLE1310 String `strip` call contains duplicate characters
|
16 | # PLE1310
17 | "Hello World".strip(r"Hello\t")
| ^^^^^^^^^^ PLE1310
18 |
19 | # PLE1310
|
bad_str_strip_call.py:20:21: PLE1310 String `strip` call contains duplicate characters
|
19 | # PLE1310
20 | "Hello World".strip("Hello\\")
| ^^^^^^^^^ PLE1310
21 |
22 | # PLE1310
|
bad_str_strip_call.py:23:21: PLE1310 String `strip` call contains duplicate characters
|
22 | # PLE1310
23 | "Hello World".strip(r"Hello\\")
| ^^^^^^^^^^ PLE1310
24 |
25 | # PLE1310
|
bad_str_strip_call.py:26:21: PLE1310 String `strip` call contains duplicate characters
|
25 | # PLE1310
26 | "Hello World".strip("🤣🤣🤣🤣🙃👀😀")
| ^^^^^^^^^^^^^^^^ PLE1310
27 |
28 | # PLE1310
|
bad_str_strip_call.py:30:5: PLE1310 String `strip` call contains duplicate characters
|
28 | # PLE1310
29 | "Hello World".strip(
30 | / """
31 | | there are a lot of characters to strip
32 | | """
| |___^ PLE1310
33 | )
|
bad_str_strip_call.py:36:21: PLE1310 String `strip` call contains duplicate characters
|
35 | # PLE1310
36 | "Hello World".strip("can we get a long " \
| _____________________^
37 | | "string of characters to strip " \
38 | | "please?")
| |_____________________________^ PLE1310
39 |
40 | # PLE1310
|
bad_str_strip_call.py:42:5: PLE1310 String `strip` call contains duplicate characters
|
40 | # PLE1310
41 | "Hello World".strip(
42 | / "can we get a long "
43 | | "string of characters to strip "
44 | | "please?"
| |_____________^ PLE1310
45 | )
|
bad_str_strip_call.py:49:5: PLE1310 String `strip` call contains duplicate characters
|
47 | # PLE1310
48 | "Hello World".strip(
49 | / "can \t we get a long"
50 | | "string \t of characters to strip"
51 | | "please?"
| |_____________^ PLE1310
52 | )
|
bad_str_strip_call.py:61:11: PLE1310 String `strip` call contains duplicate characters
|
60 | # PLE1310
61 | u''.strip('http://')
| ^^^^^^^^^ PLE1310
62 |
63 | # PLE1310
|
bad_str_strip_call.py:64:12: PLE1310 String `lstrip` call contains duplicate characters (did you mean `removeprefix`?)
|
63 | # PLE1310
64 | u''.lstrip('http://')
| ^^^^^^^^^ PLE1310
65 |
66 | # PLE1310
|
bad_str_strip_call.py:67:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?)
|
66 | # PLE1310
67 | b''.rstrip(b'http://')
| ^^^^^^^^^^ PLE1310
68 |
69 | # OK
|
bad_str_strip_call.py:79:10: PLE1310 String `strip` call contains duplicate characters
|
78 | # Errors: Multiple backslashes
79 | ''.strip('\\b\\x09')
| ^^^^^^^^^^ PLE1310
80 | ''.strip(r'\b\x09')
81 | ''.strip('\\\x5C')
|
bad_str_strip_call.py:80:10: PLE1310 String `strip` call contains duplicate characters
|
78 | # Errors: Multiple backslashes
79 | ''.strip('\\b\\x09')
80 | ''.strip(r'\b\x09')
| ^^^^^^^^^ PLE1310
81 | ''.strip('\\\x5C')
|
bad_str_strip_call.py:81:10: PLE1310 String `strip` call contains duplicate characters
|
79 | ''.strip('\\b\\x09')
80 | ''.strip(r'\b\x09')
81 | ''.strip('\\\x5C')
| ^^^^^^^^ PLE1310
82 |
83 | # Errors: Type inference
|
bad_str_strip_call.py:85:9: PLE1310 String `strip` call contains duplicate characters
|
83 | # Errors: Type inference
84 | b = b''
85 | b.strip(b'//')
| ^^^^^ PLE1310
86 |
87 | # Errors: Type inference (preview)
|
bad_str_strip_call.py:89:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?)
|
87 | # Errors: Type inference (preview)
88 | foo: str = ""; bar: bytes = b""
89 | foo.rstrip("//")
| ^^^^ PLE1310
90 | bar.lstrip(b"//")
|
bad_str_strip_call.py:90:12: PLE1310 String `lstrip` call contains duplicate characters (did you mean `removeprefix`?)
|
88 | foo: str = ""; bar: bytes = b""
89 | foo.rstrip("//")
90 | bar.lstrip(b"//")
| ^^^^^ PLE1310
|