[pylint] Also report when the object isn't a literal (PLE1310)

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

@@ -80,6 +80,16 @@
 ''.strip(r'\b\x09')
 ''.strip('\\\x5C')
 
+# Errors: Type inference
+b = b''
+b.strip(b'//')
+
+# Errors: Type inference (preview)
+foo: str = ""; bar: bytes = b""
+foo.rstrip("//")
+bar.lstrip(b"//")
+
+
 # OK: Different types
 b"".strip("//")
 "".strip(b"//")
@@ -93,13 +103,8 @@
 "".rstrip()
 
 # OK: Not literals
-foo: str = ""; bar: bytes = b""
 "".strip(foo)
 b"".strip(bar)
 
-# False negative
-foo.rstrip("//")
-bar.lstrip(b"//")
-
 # OK: Not `.[lr]?strip`
 "".mobius_strip("")

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

@@ -441,6 +441,7 @@ mod tests {
         Path::new("repeated_equality_comparison.py")
     )]
     #[test_case(Rule::InvalidEnvvarDefault, Path::new("invalid_envvar_default.py"))]
+    #[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(
             "preview__{}_{}",

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

@@ -5,6 +5,8 @@ use rustc_hash::FxHashSet;
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_semantic::analyze::typing;
+use ruff_python_semantic::SemanticModel;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -72,10 +74,22 @@ pub(crate) enum ValueKind {
 }
 
 impl ValueKind {
-    fn from(expr: &Expr) -> Option<Self> {
+    fn from(expr: &Expr, semantic: &SemanticModel) -> Option<Self> {
         match expr {
             Expr::StringLiteral(_) => Some(Self::String),
             Expr::BytesLiteral(_) => Some(Self::Bytes),
+            Expr::Name(name) => {
+                let binding_id = semantic.only_binding(name)?;
+                let binding = semantic.binding(binding_id);
+
+                if typing::is_string(binding, semantic) {
+                    Some(Self::String)
+                } else if typing::is_bytes(binding, semantic) {
+                    Some(Self::Bytes)
+                } else {
+                    None
+                }
+            }
             _ => None,
         }
     }
@@ -171,7 +185,15 @@ pub(crate) fn bad_str_strip_call(checker: &Checker, call: &ast::ExprCall) {
         return;
     };
 
-    let Some(value_kind) = ValueKind::from(value.as_ref()) else {
+    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 {
         return;
     };
 

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

@@ -179,5 +179,5 @@ bad_str_strip_call.py:81:10: PLE1310 String `strip` call contains duplicate char
 81 | ''.strip('\\\x5C')
    |          ^^^^^^^^ PLE1310
 82 |
-83 | # OK: Different types
+83 | # Errors: Type inference
    |

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

@@ -0,0 +1,210 @@
+---
+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
+   |

crates/ruff_python_semantic/src/analyze/typing.rs

@@ -741,6 +741,22 @@ impl BuiltinTypeChecker for SetChecker {
     const EXPR_TYPE: PythonType = PythonType::Set;
 }
 
+struct StringChecker;
+
+impl BuiltinTypeChecker for StringChecker {
+    const BUILTIN_TYPE_NAME: &'static str = "str";
+    const TYPING_NAME: Option<&'static str> = None;
+    const EXPR_TYPE: PythonType = PythonType::String;
+}
+
+struct BytesChecker;
+
+impl BuiltinTypeChecker for BytesChecker {
+    const BUILTIN_TYPE_NAME: &'static str = "bytes";
+    const TYPING_NAME: Option<&'static str> = None;
+    const EXPR_TYPE: PythonType = PythonType::Bytes;
+}
+
 struct TupleChecker;
 
 impl BuiltinTypeChecker for TupleChecker {
@@ -984,6 +1000,16 @@ pub fn is_float(binding: &Binding, semantic: &SemanticModel) -> bool {
     check_type::<FloatChecker>(binding, semantic)
 }
 
+/// Test whether the given binding can be considered an instance of `str`.
+pub fn is_string(binding: &Binding, semantic: &SemanticModel) -> bool {
+    check_type::<StringChecker>(binding, semantic)
+}
+
+/// Test whether the given binding can be considered an instance of `bytes`.
+pub fn is_bytes(binding: &Binding, semantic: &SemanticModel) -> bool {
+    check_type::<BytesChecker>(binding, semantic)
+}
+
 /// Test whether the given binding can be considered a set.
 ///
 /// For this, we check what value might be associated with it through it's initialization and