Skip to content

Commit 3f24895

Browse files
InSyncWithFooInSyncWithFoo
committed
[pylint] Also report when the object isn't a literal (PLE1310)
1 parent a29009e commit 3f24895

File tree

4 files changed

+250
-2
lines changed

4 files changed

+250
-2
lines changed

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

Lines changed: 1 addition & 0 deletions
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

Lines changed: 22 additions & 2 deletions
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,20 @@ 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+
match () {
86+
() if typing::is_string(binding, semantic) => Some(Self::String),
87+
() if typing::is_bytes(binding, semantic) => Some(Self::Bytes),
88+
() => None,
89+
}
90+
}
7991
_ => None,
8092
}
8193
}
@@ -171,7 +183,15 @@ pub(crate) fn bad_str_strip_call(checker: &Checker, call: &ast::ExprCall) {
171183
return;
172184
};
173185

174-
let Some(value_kind) = ValueKind::from(value.as_ref()) else {
186+
let value = value.as_ref();
187+
188+
if checker.settings.preview.is_disabled()
189+
&& !matches!(value, Expr::StringLiteral(_) | Expr::BytesLiteral(_))
190+
{
191+
return;
192+
}
193+
194+
let Some(value_kind) = ValueKind::from(value, checker.semantic()) else {
175195
return;
176196
};
177197

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

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
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 | # OK: Different types
183+
|
184+
185+
bad_str_strip_call.py:101:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?)
186+
|
187+
100 | # False negative
188+
101 | foo.rstrip("//")
189+
| ^^^^ PLE1310
190+
102 | bar.lstrip(b"//")
191+
|
192+
193+
bad_str_strip_call.py:102:12: PLE1310 String `lstrip` call contains duplicate characters (did you mean `removeprefix`?)
194+
|
195+
100 | # False negative
196+
101 | foo.rstrip("//")
197+
102 | bar.lstrip(b"//")
198+
| ^^^^^ PLE1310
199+
103 |
200+
104 | # OK: Not `.[lr]?strip`
201+
|

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

Lines changed: 26 additions & 0 deletions
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::String;
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)