mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 10:48:32 +00:00
[pylint
] Do not report calls when object type and argument type mismatch, 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`.
This commit is contained in:
parent
d4a5772d96
commit
19f3424a1a
4 changed files with 137 additions and 47 deletions
|
@ -64,10 +64,42 @@ u''.strip('http://')
|
|||
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("")
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue