Skip to content

Commit 07cf885

Browse files
InSyncWithFooInSyncWithFoo
authored
[pylint] Also report when the object isn't a literal (PLE1310) (#15985)
## Summary Follow-up to #15984. Previously, `PLE1310` would only report when the object is a literal: ```python 'a'.strip('//') # error foo = '' foo.strip('//') # no error ``` After this change, objects whose type can be inferred to be either `str` or `bytes` will also be reported in preview. ## Test Plan `cargo nextest run` and `cargo insta test`.
1 parent c089896 commit 07cf885

File tree

6 files changed

+272
-8
lines changed

6 files changed

+272
-8
lines changed

‎crates/ruff_linter/resources/test/fixtures/pylint/bad_str_strip_call.py

+10-5
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,16 @@
8080
''.strip(r'\b\x09')
8181
''.strip('\\\x5C')
8282

83+
# Errors: Type inference
84+
b = b''
85+
b.strip(b'//')
86+
87+
# Errors: Type inference (preview)
88+
foo: str = ""; bar: bytes = b""
89+
foo.rstrip("//")
90+
bar.lstrip(b"//")
91+
92+
8393
# OK: Different types
8494
b"".strip("//")
8595
"".strip(b"//")
@@ -93,13 +103,8 @@
93103
"".rstrip()
94104

95105
# OK: Not literals
96-
foo: str = ""; bar: bytes = b""
97106
"".strip(foo)
98107
b"".strip(bar)
99108

100-
# False negative
101-
foo.rstrip("//")
102-
bar.lstrip(b"//")
103-
104109
# OK: Not `.[lr]?strip`
105110
"".mobius_strip("")

‎crates/ruff_linter/src/rules/pylint/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,7 @@ mod tests {
441441
Path::new("repeated_equality_comparison.py")
442442
)]
443443
#[test_case(Rule::InvalidEnvvarDefault, Path::new("invalid_envvar_default.py"))]
444+
#[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"))]
444445
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
445446
let snapshot = format!(
446447
"preview__{}_{}",

‎crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs

+24-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use rustc_hash::FxHashSet;
55

66
use ruff_diagnostics::{Diagnostic, Violation};
77
use ruff_macros::{derive_message_formats, ViolationMetadata};
8+
use ruff_python_semantic::analyze::typing;
9+
use ruff_python_semantic::SemanticModel;
810
use ruff_text_size::Ranged;
911

1012
use crate::checkers::ast::Checker;
@@ -72,10 +74,22 @@ pub(crate) enum ValueKind {
7274
}
7375

7476
impl ValueKind {
75-
fn from(expr: &Expr) -> Option<Self> {
77+
fn from(expr: &Expr, semantic: &SemanticModel) -> Option<Self> {
7678
match expr {
7779
Expr::StringLiteral(_) => Some(Self::String),
7880
Expr::BytesLiteral(_) => Some(Self::Bytes),
81+
Expr::Name(name) => {
82+
let binding_id = semantic.only_binding(name)?;
83+
let binding = semantic.binding(binding_id);
84+
85+
if typing::is_string(binding, semantic) {
86+
Some(Self::String)
87+
} else if typing::is_bytes(binding, semantic) {
88+
Some(Self::Bytes)
89+
} else {
90+
None
91+
}
92+
}
7993
_ => None,
8094
}
8195
}
@@ -171,7 +185,15 @@ pub(crate) fn bad_str_strip_call(checker: &Checker, call: &ast::ExprCall) {
171185
return;
172186
};
173187

174-
let Some(value_kind) = ValueKind::from(value.as_ref()) else {
188+
let value = &**value;
189+
190+
if checker.settings.preview.is_disabled()
191+
&& !matches!(value, Expr::StringLiteral(_) | Expr::BytesLiteral(_))
192+
{
193+
return;
194+
}
195+
196+
let Some(value_kind) = ValueKind::from(value, checker.semantic()) else {
175197
return;
176198
};
177199

‎crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1310_bad_str_strip_call.py.snap

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,5 +179,5 @@ bad_str_strip_call.py:81:10: PLE1310 String `strip` call contains duplicate char
179179
81 | ''.strip('\\\x5C')
180180
| ^^^^^^^^ PLE1310
181181
82 |
182-
83 | # OK: Different types
182+
83 | # Errors: Type inference
183183
|

‎crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE1310_bad_str_strip_call.py.snap

+210
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pylint/mod.rs
3+
---
4+
bad_str_strip_call.py:2:21: PLE1310 String `strip` call contains duplicate characters
5+
|
6+
1 | # PLE1310
7+
2 | "Hello World".strip("Hello")
8+
| ^^^^^^^ PLE1310
9+
3 |
10+
4 | # PLE1310
11+
|
12+
13+
bad_str_strip_call.py:5:21: PLE1310 String `strip` call contains duplicate characters
14+
|
15+
4 | # PLE1310
16+
5 | "Hello World".strip("Hello")
17+
| ^^^^^^^ PLE1310
18+
6 |
19+
7 | # PLE1310
20+
|
21+
22+
bad_str_strip_call.py:8:21: PLE1310 String `strip` call contains duplicate characters
23+
|
24+
7 | # PLE1310
25+
8 | "Hello World".strip(u"Hello")
26+
| ^^^^^^^^ PLE1310
27+
9 |
28+
10 | # PLE1310
29+
|
30+
31+
bad_str_strip_call.py:11:21: PLE1310 String `strip` call contains duplicate characters
32+
|
33+
10 | # PLE1310
34+
11 | "Hello World".strip(r"Hello")
35+
| ^^^^^^^^ PLE1310
36+
12 |
37+
13 | # PLE1310
38+
|
39+
40+
bad_str_strip_call.py:14:21: PLE1310 String `strip` call contains duplicate characters
41+
|
42+
13 | # PLE1310
43+
14 | "Hello World".strip("Hello\t")
44+
| ^^^^^^^^^ PLE1310
45+
15 |
46+
16 | # PLE1310
47+
|
48+
49+
bad_str_strip_call.py:17:21: PLE1310 String `strip` call contains duplicate characters
50+
|
51+
16 | # PLE1310
52+
17 | "Hello World".strip(r"Hello\t")
53+
| ^^^^^^^^^^ PLE1310
54+
18 |
55+
19 | # PLE1310
56+
|
57+
58+
bad_str_strip_call.py:20:21: PLE1310 String `strip` call contains duplicate characters
59+
|
60+
19 | # PLE1310
61+
20 | "Hello World".strip("Hello\\")
62+
| ^^^^^^^^^ PLE1310
63+
21 |
64+
22 | # PLE1310
65+
|
66+
67+
bad_str_strip_call.py:23:21: PLE1310 String `strip` call contains duplicate characters
68+
|
69+
22 | # PLE1310
70+
23 | "Hello World".strip(r"Hello\\")
71+
| ^^^^^^^^^^ PLE1310
72+
24 |
73+
25 | # PLE1310
74+
|
75+
76+
bad_str_strip_call.py:26:21: PLE1310 String `strip` call contains duplicate characters
77+
|
78+
25 | # PLE1310
79+
26 | "Hello World".strip("🤣🤣🤣🤣🙃👀😀")
80+
| ^^^^^^^^^^^^^^^^ PLE1310
81+
27 |
82+
28 | # PLE1310
83+
|
84+
85+
bad_str_strip_call.py:30:5: PLE1310 String `strip` call contains duplicate characters
86+
|
87+
28 | # PLE1310
88+
29 | "Hello World".strip(
89+
30 | / """
90+
31 | | there are a lot of characters to strip
91+
32 | | """
92+
| |___^ PLE1310
93+
33 | )
94+
|
95+
96+
bad_str_strip_call.py:36:21: PLE1310 String `strip` call contains duplicate characters
97+
|
98+
35 | # PLE1310
99+
36 | "Hello World".strip("can we get a long " \
100+
| _____________________^
101+
37 | | "string of characters to strip " \
102+
38 | | "please?")
103+
| |_____________________________^ PLE1310
104+
39 |
105+
40 | # PLE1310
106+
|
107+
108+
bad_str_strip_call.py:42:5: PLE1310 String `strip` call contains duplicate characters
109+
|
110+
40 | # PLE1310
111+
41 | "Hello World".strip(
112+
42 | / "can we get a long "
113+
43 | | "string of characters to strip "
114+
44 | | "please?"
115+
| |_____________^ PLE1310
116+
45 | )
117+
|
118+
119+
bad_str_strip_call.py:49:5: PLE1310 String `strip` call contains duplicate characters
120+
|
121+
47 | # PLE1310
122+
48 | "Hello World".strip(
123+
49 | / "can \t we get a long"
124+
50 | | "string \t of characters to strip"
125+
51 | | "please?"
126+
| |_____________^ PLE1310
127+
52 | )
128+
|
129+
130+
bad_str_strip_call.py:61:11: PLE1310 String `strip` call contains duplicate characters
131+
|
132+
60 | # PLE1310
133+
61 | u''.strip('http://')
134+
| ^^^^^^^^^ PLE1310
135+
62 |
136+
63 | # PLE1310
137+
|
138+
139+
bad_str_strip_call.py:64:12: PLE1310 String `lstrip` call contains duplicate characters (did you mean `removeprefix`?)
140+
|
141+
63 | # PLE1310
142+
64 | u''.lstrip('http://')
143+
| ^^^^^^^^^ PLE1310
144+
65 |
145+
66 | # PLE1310
146+
|
147+
148+
bad_str_strip_call.py:67:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?)
149+
|
150+
66 | # PLE1310
151+
67 | b''.rstrip(b'http://')
152+
| ^^^^^^^^^^ PLE1310
153+
68 |
154+
69 | # OK
155+
|
156+
157+
bad_str_strip_call.py:79:10: PLE1310 String `strip` call contains duplicate characters
158+
|
159+
78 | # Errors: Multiple backslashes
160+
79 | ''.strip('\\b\\x09')
161+
| ^^^^^^^^^^ PLE1310
162+
80 | ''.strip(r'\b\x09')
163+
81 | ''.strip('\\\x5C')
164+
|
165+
166+
bad_str_strip_call.py:80:10: PLE1310 String `strip` call contains duplicate characters
167+
|
168+
78 | # Errors: Multiple backslashes
169+
79 | ''.strip('\\b\\x09')
170+
80 | ''.strip(r'\b\x09')
171+
| ^^^^^^^^^ PLE1310
172+
81 | ''.strip('\\\x5C')
173+
|
174+
175+
bad_str_strip_call.py:81:10: PLE1310 String `strip` call contains duplicate characters
176+
|
177+
79 | ''.strip('\\b\\x09')
178+
80 | ''.strip(r'\b\x09')
179+
81 | ''.strip('\\\x5C')
180+
| ^^^^^^^^ PLE1310
181+
82 |
182+
83 | # Errors: Type inference
183+
|
184+
185+
bad_str_strip_call.py:85:9: PLE1310 String `strip` call contains duplicate characters
186+
|
187+
83 | # Errors: Type inference
188+
84 | b = b''
189+
85 | b.strip(b'//')
190+
| ^^^^^ PLE1310
191+
86 |
192+
87 | # Errors: Type inference (preview)
193+
|
194+
195+
bad_str_strip_call.py:89:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?)
196+
|
197+
87 | # Errors: Type inference (preview)
198+
88 | foo: str = ""; bar: bytes = b""
199+
89 | foo.rstrip("//")
200+
| ^^^^ PLE1310
201+
90 | bar.lstrip(b"//")
202+
|
203+
204+
bad_str_strip_call.py:90:12: PLE1310 String `lstrip` call contains duplicate characters (did you mean `removeprefix`?)
205+
|
206+
88 | foo: str = ""; bar: bytes = b""
207+
89 | foo.rstrip("//")
208+
90 | bar.lstrip(b"//")
209+
| ^^^^^ PLE1310
210+
|

‎crates/ruff_python_semantic/src/analyze/typing.rs

+26
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,22 @@ impl BuiltinTypeChecker for SetChecker {
741741
const EXPR_TYPE: PythonType = PythonType::Set;
742742
}
743743

744+
struct StringChecker;
745+
746+
impl BuiltinTypeChecker for StringChecker {
747+
const BUILTIN_TYPE_NAME: &'static str = "str";
748+
const TYPING_NAME: Option<&'static str> = None;
749+
const EXPR_TYPE: PythonType = PythonType::String;
750+
}
751+
752+
struct BytesChecker;
753+
754+
impl BuiltinTypeChecker for BytesChecker {
755+
const BUILTIN_TYPE_NAME: &'static str = "bytes";
756+
const TYPING_NAME: Option<&'static str> = None;
757+
const EXPR_TYPE: PythonType = PythonType::Bytes;
758+
}
759+
744760
struct TupleChecker;
745761

746762
impl BuiltinTypeChecker for TupleChecker {
@@ -984,6 +1000,16 @@ pub fn is_float(binding: &Binding, semantic: &SemanticModel) -> bool {
9841000
check_type::<FloatChecker>(binding, semantic)
9851001
}
9861002

1003+
/// Test whether the given binding can be considered an instance of `str`.
1004+
pub fn is_string(binding: &Binding, semantic: &SemanticModel) -> bool {
1005+
check_type::<StringChecker>(binding, semantic)
1006+
}
1007+
1008+
/// Test whether the given binding can be considered an instance of `bytes`.
1009+
pub fn is_bytes(binding: &Binding, semantic: &SemanticModel) -> bool {
1010+
check_type::<BytesChecker>(binding, semantic)
1011+
}
1012+
9871013
/// Test whether the given binding can be considered a set.
9881014
///
9891015
/// For this, we check what value might be associated with it through it's initialization and

0 commit comments

Comments
 (0)