[pylint] Do not report calls when object type and argument type mis…

…match, remove custom escape handling logic (`PLE1310`) (#15984)

## Summary

Resolves #15968.

Previously, these would be considered violations:

```python
b''.strip('//')
''.lstrip('//', foo = "bar")
```

...while these are not:

```python
b''.strip(b'//')
''.strip('\\b\\x08')
```

Ruff will now not report when the types of the object and that of the
argument mismatch, or when there are extra arguments.

## Test Plan

`cargo nextest run` and `cargo insta test`.

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

@@ -64,10 +64,42 @@
 u''.lstrip('http://')
 
 # PLE1310
-b''.rstrip('http://')
+b''.rstrip(b'http://')
 
 # OK
 ''.strip('Hi')
 
 # OK
 ''.strip()
+
+
+### https://github.com/astral-sh/ruff/issues/15968
+
+# Errors: Multiple backslashes
+''.strip('\\b\\x09')
+''.strip(r'\b\x09')
+''.strip('\\\x5C')
+
+# OK: Different types
+b"".strip("//")
+"".strip(b"//")
+
+# OK: Escapes
+'\\test'.strip('\\')
+
+# OK: Extra/missing arguments
+"".strip("//", foo)
+b"".lstrip(b"//", foo = "bar")
+"".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/checkers/ast/analyze/expression.rs

@@ -906,7 +906,7 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
                 pylint::rules::bad_open_mode(checker, call);
             }
             if checker.enabled(Rule::BadStrStripCall) {
-                pylint::rules::bad_str_strip_call(checker, func, args);
+                pylint::rules::bad_str_strip_call(checker, call);
             }
             if checker.enabled(Rule::ShallowCopyEnviron) {
                 pylint::rules::shallow_copy_environ(checker, call);

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

@@ -65,6 +65,22 @@ impl Violation for BadStrStripCall {
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Copy, Clone)]
+pub(crate) enum ValueKind {
+    String,
+    Bytes,
+}
+
+impl ValueKind {
+    fn from(expr: &Expr) -> Option<Self> {
+        match expr {
+            Expr::StringLiteral(_) => Some(Self::String),
+            Expr::BytesLiteral(_) => Some(Self::Bytes),
+            _ => None,
+        }
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
 pub(crate) enum StripKind {
     Strip,
@@ -120,52 +136,66 @@ impl fmt::Display for RemovalKind {
     }
 }
 
-/// Return `true` if a string contains duplicate characters, taking into account
-/// escapes.
-fn has_duplicates(s: &ast::StringLiteralValue) -> bool {
-    let mut escaped = false;
+fn string_has_duplicate_char(string: &ast::StringLiteralValue) -> bool {
+    has_duplicate(string.chars())
+}
+
+fn bytes_has_duplicate_char(bytes: &ast::BytesLiteralValue) -> bool {
+    has_duplicate(bytes.bytes().map(char::from))
+}
+
+/// Return true if a string or byte sequence has a duplicate.
+fn has_duplicate(mut chars: impl Iterator<Item = char>) -> bool {
     let mut seen = FxHashSet::default();
-    for ch in s.chars() {
-        if escaped {
-            escaped = false;
-            let pair = format!("\\{ch}");
-            if !seen.insert(pair) {
-                return true;
-            }
-        } else if ch == '\\' {
-            escaped = true;
-        } else if !seen.insert(ch.to_string()) {
-            return true;
-        }
-    }
-    false
+
+    chars.any(|char| !seen.insert(char))
 }
 
 /// PLE1310
-pub(crate) fn bad_str_strip_call(checker: &Checker, func: &Expr, args: &[Expr]) {
-    if let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func {
-        if matches!(
-            value.as_ref(),
-            Expr::StringLiteral(_) | Expr::BytesLiteral(_)
-        ) {
-            if let Some(strip) = StripKind::from_str(attr.as_str()) {
-                if let Some(arg) = args.first() {
-                    if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &arg {
-                        if has_duplicates(value) {
-                            let removal = if checker.settings.target_version >= PythonVersion::Py39
-                            {
-                                RemovalKind::for_strip(strip)
-                            } else {
-                                None
-                            };
-                            checker.report_diagnostic(Diagnostic::new(
-                                BadStrStripCall { strip, removal },
-                                arg.range(),
-                            ));
-                        }
-                    }
-                }
-            }
+pub(crate) fn bad_str_strip_call(checker: &Checker, call: &ast::ExprCall) {
+    let (func, arguments) = (&call.func, &call.arguments);
+
+    let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() else {
+        return;
+    };
+
+    let Some(strip) = StripKind::from_str(attr.as_str()) else {
+        return;
+    };
+
+    if !arguments.keywords.is_empty() {
+        return;
+    }
+
+    let [arg] = arguments.args.as_ref() else {
+        return;
+    };
+
+    let Some(value_kind) = ValueKind::from(value.as_ref()) else {
+        return;
+    };
+
+    let duplicated = match arg {
+        Expr::StringLiteral(string) if value_kind == ValueKind::String => {
+            string_has_duplicate_char(&string.value)
+        }
+        Expr::BytesLiteral(bytes) if value_kind == ValueKind::Bytes => {
+            bytes_has_duplicate_char(&bytes.value)
         }
+        _ => return,
+    };
+
+    if !duplicated {
+        return;
     }
+
+    let removal = if checker.settings.target_version >= PythonVersion::Py39 {
+        RemovalKind::for_strip(strip)
+    } else {
+        None
+    };
+
+    let diagnostic = Diagnostic::new(BadStrStripCall { strip, removal }, arg.range());
+
+    checker.report_diagnostic(diagnostic);
 }

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

@@ -148,8 +148,36 @@ bad_str_strip_call.py:64:12: PLE1310 String `lstrip` call contains duplicate cha
 bad_str_strip_call.py:67:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?)
    |
 66 | # PLE1310
-67 | b''.rstrip('http://')
-   |            ^^^^^^^^^ 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 | # OK: Different types
+   |