Remove autofix from bad-str-strip-call; add suggestions instead (#2610)

This commit is contained in:
Charlie Marsh 2023-02-06 16:25:20 -05:00 committed by GitHub
parent cee0d0abaa
commit 610f150dd1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 81 additions and 210 deletions

View file

@ -1336,7 +1336,7 @@ For more, see [Pylint](https://pypi.org/project/pylint/) on PyPI.
| PLE0604 | invalid-all-object | Invalid object in `__all__`, must contain only strings | |
| PLE0605 | invalid-all-format | Invalid format for `__all__`, must be `tuple` or `list` | |
| PLE1142 | await-outside-async | `await` should be used within an async function | |
| PLE1310 | bad-str-strip-call | String `{kind}` call contains duplicate characters | 🛠 |
| PLE1310 | bad-str-strip-call | String `{strip}` call contains duplicate characters (did you mean `{removal}`?) | |
#### Refactor (PLR)

View file

@ -2,8 +2,6 @@ use std::fmt;
use rustc_hash::FxHashSet;
use rustpython_ast::{Constant, Expr, ExprKind};
use rustpython_parser::lexer;
use rustpython_parser::lexer::Tok;
use serde::{Deserialize, Serialize};
use ruff_macros::derive_message_formats;
@ -11,10 +9,29 @@ use ruff_macros::derive_message_formats;
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::define_violation;
use crate::fix::Fix;
use crate::registry::Diagnostic;
use crate::rules::pydocstyle::helpers::{leading_quote, trailing_quote};
use crate::violation::AlwaysAutofixableViolation;
use crate::settings::types::PythonVersion;
use crate::violation::Violation;
define_violation!(
pub struct BadStrStripCall {
strip: StripKind,
removal: Option<RemovalKind>,
}
);
impl Violation for BadStrStripCall {
#[derive_message_formats]
fn message(&self) -> String {
let Self { strip, removal } = self;
if let Some(removal) = removal {
format!(
"String `{strip}` call contains duplicate characters (did you mean `{removal}`?)",
)
} else {
format!("String `{strip}` call contains duplicate characters")
}
}
}
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum StripKind {
@ -45,56 +62,30 @@ impl fmt::Display for StripKind {
}
}
define_violation!(
pub struct BadStrStripCall {
kind: StripKind,
}
);
impl AlwaysAutofixableViolation for BadStrStripCall {
#[derive_message_formats]
fn message(&self) -> String {
let Self { kind } = self;
format!("String `{kind}` call contains duplicate characters")
}
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum RemovalKind {
RemovePrefix,
RemoveSuffix,
}
fn autofix_title(&self) -> String {
"Remove duplicate characters".to_string()
impl RemovalKind {
pub fn for_strip(s: &StripKind) -> Option<Self> {
match s {
StripKind::Strip => None,
StripKind::LStrip => Some(Self::RemovePrefix),
StripKind::RStrip => Some(Self::RemoveSuffix),
}
}
}
/// Remove duplicate characters from an escaped string.
fn deduplicate_escaped(s: &str) -> String {
let mut result = String::new();
let mut escaped = false;
let mut seen = FxHashSet::default();
for ch in s.chars() {
if escaped {
escaped = false;
let pair = format!("\\{}", ch);
if !seen.insert(pair) {
continue;
}
} else if ch == '\\' {
escaped = true;
} else if !seen.insert(ch.to_string()) {
continue;
}
result.push(ch);
impl fmt::Display for RemovalKind {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let representation = match self {
Self::RemovePrefix => "removeprefix",
Self::RemoveSuffix => "removesuffix",
};
write!(f, "{representation}")
}
result
}
/// Remove duplicate characters from a raw string.
fn deduplicate_raw(s: &str) -> String {
let mut result = String::new();
let mut seen = FxHashSet::default();
for ch in s.chars() {
if !seen.insert(ch) {
continue;
}
result.push(ch);
}
result
}
/// Return `true` if a string contains duplicate characters, taking into account escapes.
@ -127,64 +118,24 @@ pub fn bad_str_strip_call(checker: &mut Checker, func: &Expr, args: &[Expr]) {
..
}
) {
if let Some(kind) = StripKind::from_str(attr.as_str()) {
if let Some(strip) = StripKind::from_str(attr.as_str()) {
if let Some(arg) = args.get(0) {
if let ExprKind::Constant {
value: Constant::Str(value),
..
} = &arg.node
{
let is_multiline = arg.location.row() != arg.end_location.unwrap().row();
let module_text = checker
.locator
.slice_source_code_range(&Range::from_located(arg));
if !is_multiline
&& lexer::make_tokenizer_located(module_text, arg.location)
.flatten()
.filter(|(_, tok, _)| matches!(tok, Tok::String { .. }))
.nth(1)
.is_none()
{
// If we have a single string (no implicit concatenation), fix it.
let Some(leading_quote) = leading_quote(module_text) else {
return;
if has_duplicates(value) {
let removal = if checker.settings.target_version >= PythonVersion::Py39
{
RemovalKind::for_strip(&strip)
} else {
None
};
let Some(trailing_quote) = trailing_quote(module_text) else {
return;
};
let content = &module_text
[leading_quote.len()..module_text.len() - trailing_quote.len()];
let deduplicated =
if leading_quote.contains('r') || leading_quote.contains('R') {
deduplicate_raw(content)
} else {
deduplicate_escaped(content)
};
if content != deduplicated {
let mut diagnostic = Diagnostic::new(
BadStrStripCall { kind },
Range::from_located(arg),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.amend(Fix::replacement(
format!("{leading_quote}{deduplicated}{trailing_quote}"),
arg.location,
arg.end_location.unwrap(),
));
};
checker.diagnostics.push(diagnostic);
}
} else {
// Otherwise, let's just look for duplicates.
if has_duplicates(value) {
checker.diagnostics.push(Diagnostic::new(
BadStrStripCall { kind },
Range::from_located(arg),
));
}
checker.diagnostics.push(Diagnostic::new(
BadStrStripCall { strip, removal },
Range::from_located(arg),
));
}
}
}

View file

@ -5,177 +5,115 @@ expression: diagnostics
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 2
column: 20
end_location:
row: 2
column: 27
fix:
content:
- "\"Helo\""
location:
row: 2
column: 20
end_location:
row: 2
column: 27
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 5
column: 20
end_location:
row: 5
column: 27
fix:
content:
- "\"Helo\""
location:
row: 5
column: 20
end_location:
row: 5
column: 27
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 8
column: 20
end_location:
row: 8
column: 28
fix:
content:
- "u\"Helo\""
location:
row: 8
column: 20
end_location:
row: 8
column: 28
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 11
column: 20
end_location:
row: 11
column: 28
fix:
content:
- "r\"Helo\""
location:
row: 11
column: 20
end_location:
row: 11
column: 28
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 14
column: 20
end_location:
row: 14
column: 29
fix:
content:
- "\"Helo\\t\""
location:
row: 14
column: 20
end_location:
row: 14
column: 29
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 17
column: 20
end_location:
row: 17
column: 30
fix:
content:
- "r\"Helo\\t\""
location:
row: 17
column: 20
end_location:
row: 17
column: 30
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 20
column: 20
end_location:
row: 20
column: 29
fix:
content:
- "\"Helo\\\\\""
location:
row: 20
column: 20
end_location:
row: 20
column: 29
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 23
column: 20
end_location:
row: 23
column: 30
fix:
content:
- "r\"Helo\\\""
location:
row: 23
column: 20
end_location:
row: 23
column: 30
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 26
column: 20
end_location:
row: 26
column: 29
fix:
content:
- "\"🤣🙃👀😀\""
location:
row: 26
column: 20
end_location:
row: 26
column: 29
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 30
column: 4
@ -187,6 +125,7 @@ expression: diagnostics
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 36
column: 20
@ -198,6 +137,7 @@ expression: diagnostics
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 42
column: 4
@ -209,6 +149,7 @@ expression: diagnostics
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 49
column: 4
@ -220,58 +161,37 @@ expression: diagnostics
- kind:
BadStrStripCall:
kind: Strip
suggestion: ~
location:
row: 61
column: 10
end_location:
row: 61
column: 19
fix:
content:
- "'htp:/'"
location:
row: 61
column: 10
end_location:
row: 61
column: 19
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: LStrip
suggestion: RemovePrefix
location:
row: 64
column: 11
end_location:
row: 64
column: 20
fix:
content:
- "'htp:/'"
location:
row: 64
column: 11
end_location:
row: 64
column: 20
fix: ~
parent: ~
- kind:
BadStrStripCall:
kind: RStrip
suggestion: RemoveSuffix
location:
row: 67
column: 11
end_location:
row: 67
column: 20
fix:
content:
- "'htp:/'"
location:
row: 67
column: 11
end_location:
row: 67
column: 20
fix: ~
parent: ~