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

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;
@@ -79,10 +81,20 @@ 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);
+
+                match () {
+                    () if typing::is_string(binding, semantic) => Some(Self::String),
+                    () if typing::is_bytes(binding, semantic) => Some(Self::Bytes),
+                    _ => None,
+                }
+            }
             _ => None,
         }
     }
@@ -188,7 +200,15 @@ pub(crate) fn bad_str_strip_call(checker: &mut Checker, call: &ast::ExprCall) {
         return;
     };
 
-    let Some(value_kind) = ValueKind::from(value.as_ref()) else {
+    let value = value.as_ref();
+
+    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__preview__PLE1310_bad_str_strip_call.py.snap

@@ -0,0 +1,201 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+bad_str_strip_call.py:2:21: PLE1310 String `strip` call contains duplicate character 'l'
+  |
+1 | # PLE1310
+2 | "Hello World".strip("Hello")
+  |                     ^^^^^^^ PLE1310
+3 |
+4 | # PLE1310
+  |
+
+bad_str_strip_call.py:5:21: PLE1310 String `strip` call contains duplicate character 'l'
+  |
+4 | # PLE1310
+5 | "Hello World".strip("Hello")
+  |                     ^^^^^^^ PLE1310
+6 |
+7 | # PLE1310
+  |
+
+bad_str_strip_call.py:8:21: PLE1310 String `strip` call contains duplicate character 'l'
+   |
+ 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 character 'l'
+   |
+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 character 'l'
+   |
+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 character 'l'
+   |
+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 character 'l'
+   |
+19 | # PLE1310
+20 | "Hello World".strip("Hello\\")
+   |                     ^^^^^^^^^ PLE1310
+21 |
+22 | # PLE1310
+   |
+
+bad_str_strip_call.py:23:21: PLE1310 String `strip` call contains duplicate character 'l'
+   |
+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 character '🤣'
+   |
+25 | # PLE1310
+26 | "Hello World".strip("🤣🤣🤣🤣🙃👀😀")
+   |                     ^^^^^^^^^^^^^^^^ PLE1310
+27 |
+28 | # PLE1310
+   |
+
+bad_str_strip_call.py:30:5: PLE1310 String `strip` call contains duplicate character 'e'
+   |
+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 character ' '
+   |
+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 character ' '
+   |
+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 character ' '
+   |
+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 character 't'
+   |
+60 | # PLE1310
+61 | u''.strip('http://')
+   |           ^^^^^^^^^ PLE1310
+62 |
+63 | # PLE1310
+   |
+
+bad_str_strip_call.py:64:12: PLE1310 String `lstrip` call contains duplicate character 't' (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 character 't' (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 character '\\'
+   |
+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 character '\\'
+   |
+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 character '\\'
+   |
+79 | ''.strip('\\b\\x09')
+80 | ''.strip(r'\b\x09')
+81 | ''.strip('\\\x5C')
+   |          ^^^^^^^^ PLE1310
+82 |
+83 | # OK: Different types
+   |
+
+bad_str_strip_call.py:98:12: PLE1310 String `rstrip` call contains duplicate character '/' (did you mean `removesuffix`?)
+   |
+97 | # False negative
+98 | foo.rstrip("//")
+   |            ^^^^ PLE1310
+99 | bar.lstrip(b"//")
+   |
+
+bad_str_strip_call.py:99:12: PLE1310 String `lstrip` call contains duplicate character '/' (did you mean `removeprefix`?)
+    |
+ 97 | # False negative
+ 98 | foo.rstrip("//")
+ 99 | bar.lstrip(b"//")
+    |            ^^^^^ PLE1310
+100 |
+101 | # OK: Not `.[lr]?strip`
+    |

crates/ruff_python_semantic/src/analyze/typing.rs

@@ -740,6 +740,22 @@ impl BuiltinTypeChecker for FloatChecker {
     const EXPR_TYPE: PythonType = PythonType::Number(NumberLike::Float);
 }
 
+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::String;
+}
+
 pub struct IoBaseChecker;
 
 impl TypeChecker for IoBaseChecker {
@@ -959,6 +975,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