mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-30 22:01:47 +00:00
[refurb] Implement slice-to-remove-prefix-or-suffix
(FURB188
) (#13256)
This commit is contained in:
parent
a98dbcee78
commit
b04948fb72
9 changed files with 816 additions and 0 deletions
154
crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py
vendored
Normal file
154
crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py
vendored
Normal file
|
@ -0,0 +1,154 @@
|
||||||
|
# Test suite from Refurb
|
||||||
|
# See https://github.com/dosisod/refurb/blob/db02242b142285e615a664a8d3324470bb711306/test/data/err_188.py
|
||||||
|
|
||||||
|
# these should match
|
||||||
|
|
||||||
|
def remove_extension_via_slice(filename: str) -> str:
|
||||||
|
if filename.endswith(".txt"):
|
||||||
|
filename = filename[:-4]
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_via_slice_len(filename: str, extension: str) -> str:
|
||||||
|
if filename.endswith(extension):
|
||||||
|
filename = filename[:-len(extension)]
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_via_ternary(filename: str) -> str:
|
||||||
|
return filename[:-4] if filename.endswith(".txt") else filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_via_ternary_with_len(filename: str, extension: str) -> str:
|
||||||
|
return filename[:-len(extension)] if filename.endswith(extension) else filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_prefix(filename: str) -> str:
|
||||||
|
return filename[4:] if filename.startswith("abc-") else filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_prefix_via_len(filename: str, prefix: str) -> str:
|
||||||
|
return filename[len(prefix):] if filename.startswith(prefix) else filename
|
||||||
|
|
||||||
|
|
||||||
|
# these should not
|
||||||
|
|
||||||
|
def remove_extension_with_mismatched_len(filename: str) -> str:
|
||||||
|
if filename.endswith(".txt"):
|
||||||
|
filename = filename[:3]
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_assign_to_different_var(filename: str) -> str:
|
||||||
|
if filename.endswith(".txt"):
|
||||||
|
other_var = filename[:-4]
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_with_multiple_stmts(filename: str) -> str:
|
||||||
|
if filename.endswith(".txt"):
|
||||||
|
print("do some work")
|
||||||
|
|
||||||
|
filename = filename[:-4]
|
||||||
|
|
||||||
|
if filename.endswith(".txt"):
|
||||||
|
filename = filename[:-4]
|
||||||
|
|
||||||
|
print("do some work")
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_from_unrelated_var(filename: str) -> str:
|
||||||
|
xyz = "abc.txt"
|
||||||
|
|
||||||
|
if filename.endswith(".txt"):
|
||||||
|
filename = xyz[:-4]
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_in_elif(filename: str) -> str:
|
||||||
|
if filename:
|
||||||
|
pass
|
||||||
|
|
||||||
|
elif filename.endswith(".txt"):
|
||||||
|
filename = filename[:-4]
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_in_multiple_elif(filename: str) -> str:
|
||||||
|
if filename:
|
||||||
|
pass
|
||||||
|
|
||||||
|
elif filename:
|
||||||
|
pass
|
||||||
|
|
||||||
|
elif filename.endswith(".txt"):
|
||||||
|
filename = filename[:-4]
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_in_if_with_else(filename: str) -> str:
|
||||||
|
if filename.endswith(".txt"):
|
||||||
|
filename = filename[:-4]
|
||||||
|
|
||||||
|
else:
|
||||||
|
pass
|
||||||
|
|
||||||
|
return filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_ternary_name_mismatch(filename: str):
|
||||||
|
xyz = ""
|
||||||
|
|
||||||
|
_ = xyz[:-4] if filename.endswith(".txt") else filename
|
||||||
|
_ = filename[:-4] if xyz.endswith(".txt") else filename
|
||||||
|
_ = filename[:-4] if filename.endswith(".txt") else xyz
|
||||||
|
|
||||||
|
|
||||||
|
def remove_extension_slice_amount_mismatch(filename: str) -> None:
|
||||||
|
extension = ".txt"
|
||||||
|
|
||||||
|
_ = filename[:-1] if filename.endswith(".txt") else filename
|
||||||
|
_ = filename[:-1] if filename.endswith(extension) else filename
|
||||||
|
_ = filename[:-len("")] if filename.endswith(extension) else filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_prefix_size_mismatch(filename: str) -> str:
|
||||||
|
return filename[3:] if filename.startswith("abc-") else filename
|
||||||
|
|
||||||
|
|
||||||
|
def remove_prefix_name_mismatch(filename: str) -> None:
|
||||||
|
xyz = ""
|
||||||
|
|
||||||
|
_ = xyz[4:] if filename.startswith("abc-") else filename
|
||||||
|
_ = filename[4:] if xyz.startswith("abc-") else filename
|
||||||
|
_ = filename[4:] if filename.startswith("abc-") else xyz
|
||||||
|
|
||||||
|
# ---- End of refurb test suite ---- #
|
||||||
|
|
||||||
|
# ---- Begin ruff specific test suite --- #
|
||||||
|
|
||||||
|
# these should be linted
|
||||||
|
|
||||||
|
def remove_suffix_multiple_attribute_expr() -> None:
|
||||||
|
import foo.bar
|
||||||
|
|
||||||
|
SUFFIX = "suffix"
|
||||||
|
|
||||||
|
x = foo.bar.baz[:-len(SUFFIX)] if foo.bar.baz.endswith(SUFFIX) else foo.bar.baz
|
||||||
|
|
||||||
|
def remove_prefix_comparable_literal_expr() -> None:
|
||||||
|
return ("abc" "def")[3:] if ("abc" "def").startswith("abc") else "abc" "def"
|
||||||
|
|
||||||
|
def shadow_builtins(filename: str, extension: str) -> None:
|
||||||
|
from builtins import len as builtins_len
|
||||||
|
|
||||||
|
return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
|
|
@ -1407,6 +1407,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
|
||||||
if checker.enabled(Rule::UselessIfElse) {
|
if checker.enabled(Rule::UselessIfElse) {
|
||||||
ruff::rules::useless_if_else(checker, if_exp);
|
ruff::rules::useless_if_else(checker, if_exp);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::SliceToRemovePrefixOrSuffix) {
|
||||||
|
refurb::rules::slice_to_remove_affix_expr(checker, if_exp);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Expr::ListComp(
|
Expr::ListComp(
|
||||||
comp @ ast::ExprListComp {
|
comp @ ast::ExprListComp {
|
||||||
|
|
|
@ -1178,6 +1178,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
if checker.enabled(Rule::CheckAndRemoveFromSet) {
|
if checker.enabled(Rule::CheckAndRemoveFromSet) {
|
||||||
refurb::rules::check_and_remove_from_set(checker, if_);
|
refurb::rules::check_and_remove_from_set(checker, if_);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::SliceToRemovePrefixOrSuffix) {
|
||||||
|
refurb::rules::slice_to_remove_affix_stmt(checker, if_);
|
||||||
|
}
|
||||||
if checker.enabled(Rule::TooManyBooleanExpressions) {
|
if checker.enabled(Rule::TooManyBooleanExpressions) {
|
||||||
pylint::rules::too_many_boolean_expressions(checker, if_);
|
pylint::rules::too_many_boolean_expressions(checker, if_);
|
||||||
}
|
}
|
||||||
|
|
|
@ -1067,6 +1067,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
(Refurb, "180") => (RuleGroup::Preview, rules::refurb::rules::MetaClassABCMeta),
|
(Refurb, "180") => (RuleGroup::Preview, rules::refurb::rules::MetaClassABCMeta),
|
||||||
(Refurb, "181") => (RuleGroup::Stable, rules::refurb::rules::HashlibDigestHex),
|
(Refurb, "181") => (RuleGroup::Stable, rules::refurb::rules::HashlibDigestHex),
|
||||||
(Refurb, "187") => (RuleGroup::Stable, rules::refurb::rules::ListReverseCopy),
|
(Refurb, "187") => (RuleGroup::Stable, rules::refurb::rules::ListReverseCopy),
|
||||||
|
(Refurb, "188") => (RuleGroup::Preview, rules::refurb::rules::SliceToRemovePrefixOrSuffix),
|
||||||
(Refurb, "192") => (RuleGroup::Preview, rules::refurb::rules::SortedMinMax),
|
(Refurb, "192") => (RuleGroup::Preview, rules::refurb::rules::SortedMinMax),
|
||||||
|
|
||||||
// flake8-logging
|
// flake8-logging
|
||||||
|
|
|
@ -46,6 +46,7 @@ mod tests {
|
||||||
#[test_case(Rule::WriteWholeFile, Path::new("FURB103.py"))]
|
#[test_case(Rule::WriteWholeFile, Path::new("FURB103.py"))]
|
||||||
#[test_case(Rule::FStringNumberFormat, Path::new("FURB116.py"))]
|
#[test_case(Rule::FStringNumberFormat, Path::new("FURB116.py"))]
|
||||||
#[test_case(Rule::SortedMinMax, Path::new("FURB192.py"))]
|
#[test_case(Rule::SortedMinMax, Path::new("FURB192.py"))]
|
||||||
|
#[test_case(Rule::SliceToRemovePrefixOrSuffix, Path::new("FURB188.py"))]
|
||||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||||
let diagnostics = test_path(
|
let diagnostics = test_path(
|
||||||
|
|
|
@ -23,6 +23,7 @@ pub(crate) use repeated_append::*;
|
||||||
pub(crate) use repeated_global::*;
|
pub(crate) use repeated_global::*;
|
||||||
pub(crate) use single_item_membership_test::*;
|
pub(crate) use single_item_membership_test::*;
|
||||||
pub(crate) use slice_copy::*;
|
pub(crate) use slice_copy::*;
|
||||||
|
pub(crate) use slice_to_remove_prefix_or_suffix::*;
|
||||||
pub(crate) use sorted_min_max::*;
|
pub(crate) use sorted_min_max::*;
|
||||||
pub(crate) use type_none_comparison::*;
|
pub(crate) use type_none_comparison::*;
|
||||||
pub(crate) use unnecessary_enumerate::*;
|
pub(crate) use unnecessary_enumerate::*;
|
||||||
|
@ -55,6 +56,7 @@ mod repeated_append;
|
||||||
mod repeated_global;
|
mod repeated_global;
|
||||||
mod single_item_membership_test;
|
mod single_item_membership_test;
|
||||||
mod slice_copy;
|
mod slice_copy;
|
||||||
|
mod slice_to_remove_prefix_or_suffix;
|
||||||
mod sorted_min_max;
|
mod sorted_min_max;
|
||||||
mod type_none_comparison;
|
mod type_none_comparison;
|
||||||
mod unnecessary_enumerate;
|
mod unnecessary_enumerate;
|
||||||
|
|
|
@ -0,0 +1,474 @@
|
||||||
|
use crate::{checkers::ast::Checker, settings::types::PythonVersion};
|
||||||
|
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
|
||||||
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
use ruff_python_ast as ast;
|
||||||
|
use ruff_python_semantic::SemanticModel;
|
||||||
|
use ruff_source_file::Locator;
|
||||||
|
use ruff_text_size::{Ranged, TextLen};
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for the removal of a prefix or suffix from a string by assigning
|
||||||
|
/// the string to a slice after checking `.startswith()` or `.endswith()`, respectively.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// The methods [`str.removeprefix`] and [`str.removesuffix`],
|
||||||
|
/// introduced in Python 3.9, have the same behavior
|
||||||
|
/// and are more readable and efficient.
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// filename[:-4] if filename.endswith(".txt") else filename
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// ```python
|
||||||
|
/// if text.startswith("pre"):
|
||||||
|
/// text = text[3:]
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```python
|
||||||
|
/// filename = filename.removesuffix(".txt")
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// ```python
|
||||||
|
/// text = text.removeprefix("pre")
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// [`str.removeprefix`]: https://docs.python.org/3/library/stdtypes.html#str.removeprefix
|
||||||
|
/// [`str.removesuffix`]: https://docs.python.org/3/library/stdtypes.html#str.removesuffix
|
||||||
|
#[violation]
|
||||||
|
pub struct SliceToRemovePrefixOrSuffix {
|
||||||
|
string: String,
|
||||||
|
affix_kind: AffixKind,
|
||||||
|
stmt_or_expression: StmtOrExpr,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl AlwaysFixableViolation for SliceToRemovePrefixOrSuffix {
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
match self.affix_kind {
|
||||||
|
AffixKind::StartsWith => {
|
||||||
|
format!("Prefer `removeprefix` over conditionally replacing with slice.")
|
||||||
|
}
|
||||||
|
AffixKind::EndsWith => {
|
||||||
|
format!("Prefer `removesuffix` over conditionally replacing with slice.")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn fix_title(&self) -> String {
|
||||||
|
let method_name = self.affix_kind.as_str();
|
||||||
|
let replacement = self.affix_kind.replacement();
|
||||||
|
let context = match self.stmt_or_expression {
|
||||||
|
StmtOrExpr::Statement => "assignment",
|
||||||
|
StmtOrExpr::Expression => "ternary expression",
|
||||||
|
};
|
||||||
|
format!("Use {replacement} instead of {context} conditional upon {method_name}.")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// FURB188
|
||||||
|
pub(crate) fn slice_to_remove_affix_expr(checker: &mut Checker, if_expr: &ast::ExprIf) {
|
||||||
|
if checker.settings.target_version < PythonVersion::Py39 {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if let Some(removal_data) = affix_removal_data_expr(if_expr) {
|
||||||
|
if affix_matches_slice_bound(&removal_data, checker.semantic()) {
|
||||||
|
let kind = removal_data.affix_query.kind;
|
||||||
|
let text = removal_data.text;
|
||||||
|
|
||||||
|
let mut diagnostic = Diagnostic::new(
|
||||||
|
SliceToRemovePrefixOrSuffix {
|
||||||
|
affix_kind: kind,
|
||||||
|
string: checker.locator().slice(text).to_string(),
|
||||||
|
stmt_or_expression: StmtOrExpr::Expression,
|
||||||
|
},
|
||||||
|
if_expr.range,
|
||||||
|
);
|
||||||
|
let replacement =
|
||||||
|
generate_removeaffix_expr(text, &removal_data.affix_query, checker.locator());
|
||||||
|
|
||||||
|
diagnostic.set_fix(Fix::safe_edit(Edit::replacement(
|
||||||
|
replacement,
|
||||||
|
if_expr.start(),
|
||||||
|
if_expr.end(),
|
||||||
|
)));
|
||||||
|
checker.diagnostics.push(diagnostic);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// FURB188
|
||||||
|
pub(crate) fn slice_to_remove_affix_stmt(checker: &mut Checker, if_stmt: &ast::StmtIf) {
|
||||||
|
if checker.settings.target_version < PythonVersion::Py39 {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if let Some(removal_data) = affix_removal_data_stmt(if_stmt) {
|
||||||
|
if affix_matches_slice_bound(&removal_data, checker.semantic()) {
|
||||||
|
let kind = removal_data.affix_query.kind;
|
||||||
|
let text = removal_data.text;
|
||||||
|
|
||||||
|
let mut diagnostic = Diagnostic::new(
|
||||||
|
SliceToRemovePrefixOrSuffix {
|
||||||
|
affix_kind: kind,
|
||||||
|
string: checker.locator().slice(text).to_string(),
|
||||||
|
stmt_or_expression: StmtOrExpr::Statement,
|
||||||
|
},
|
||||||
|
if_stmt.range,
|
||||||
|
);
|
||||||
|
|
||||||
|
let replacement = generate_assignment_with_removeaffix(
|
||||||
|
text,
|
||||||
|
&removal_data.affix_query,
|
||||||
|
checker.locator(),
|
||||||
|
);
|
||||||
|
|
||||||
|
diagnostic.set_fix(Fix::safe_edit(Edit::replacement(
|
||||||
|
replacement,
|
||||||
|
if_stmt.start(),
|
||||||
|
if_stmt.end(),
|
||||||
|
)));
|
||||||
|
checker.diagnostics.push(diagnostic);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Given an expression of the form:
|
||||||
|
///
|
||||||
|
/// ```python
|
||||||
|
/// text[slice] if text.func(affix) else text
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// where `func` is either `startswith` or `endswith`,
|
||||||
|
/// this function collects `text`,`func`, `affix`, and the non-null
|
||||||
|
/// bound of the slice. Otherwise, returns `None`.
|
||||||
|
fn affix_removal_data_expr(if_expr: &ast::ExprIf) -> Option<RemoveAffixData> {
|
||||||
|
let ast::ExprIf {
|
||||||
|
test,
|
||||||
|
body,
|
||||||
|
orelse,
|
||||||
|
range: _,
|
||||||
|
} = if_expr;
|
||||||
|
|
||||||
|
let ast::ExprSubscript { value, slice, .. } = body.as_subscript_expr()?;
|
||||||
|
// Variable names correspond to:
|
||||||
|
// ```python
|
||||||
|
// value[slice] if test else orelse
|
||||||
|
// ```
|
||||||
|
affix_removal_data(value, test, orelse, slice)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Given a statement of the form:
|
||||||
|
///
|
||||||
|
/// ```python
|
||||||
|
/// if text.func(affix):
|
||||||
|
/// text = text[slice]
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// where `func` is either `startswith` or `endswith`,
|
||||||
|
/// this function collects `text`,`func`, `affix`, and the non-null
|
||||||
|
/// bound of the slice. Otherwise, returns `None`.
|
||||||
|
fn affix_removal_data_stmt(if_stmt: &ast::StmtIf) -> Option<RemoveAffixData> {
|
||||||
|
let ast::StmtIf {
|
||||||
|
test,
|
||||||
|
body,
|
||||||
|
elif_else_clauses,
|
||||||
|
range: _,
|
||||||
|
} = if_stmt;
|
||||||
|
|
||||||
|
// Cannot safely transform, e.g.,
|
||||||
|
// ```python
|
||||||
|
// if text.startswith(prefix):
|
||||||
|
// text = text[len(prefix):]
|
||||||
|
// else:
|
||||||
|
// text = "something completely different"
|
||||||
|
// ```
|
||||||
|
if !elif_else_clauses.is_empty() {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Cannot safely transform, e.g.,
|
||||||
|
// ```python
|
||||||
|
// if text.startswith(prefix):
|
||||||
|
// text = f"{prefix} something completely different"
|
||||||
|
// text = text[len(prefix):]
|
||||||
|
// ```
|
||||||
|
let [statement] = body.as_slice() else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Variable names correspond to:
|
||||||
|
// ```python
|
||||||
|
// if test:
|
||||||
|
// else_or_target_name = value[slice]
|
||||||
|
// ```
|
||||||
|
let ast::StmtAssign {
|
||||||
|
value,
|
||||||
|
targets,
|
||||||
|
range: _,
|
||||||
|
} = statement.as_assign_stmt()?;
|
||||||
|
let [target] = targets.as_slice() else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
let ast::ExprSubscript { value, slice, .. } = value.as_subscript_expr()?;
|
||||||
|
|
||||||
|
affix_removal_data(value, test, target, slice)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Suppose given a statement of the form:
|
||||||
|
/// ```python
|
||||||
|
/// if test:
|
||||||
|
/// else_or_target_name = value[slice]
|
||||||
|
/// ```
|
||||||
|
/// or an expression of the form:
|
||||||
|
/// ```python
|
||||||
|
/// value[slice] if test else else_or_target_name
|
||||||
|
/// ```
|
||||||
|
/// This function verifies that
|
||||||
|
/// - `value` and `else_or_target_name`
|
||||||
|
/// are equal to a common name `text`
|
||||||
|
/// - `test` is of the form `text.startswith(prefix)`
|
||||||
|
/// or `text.endswith(suffix)`
|
||||||
|
/// - `slice` has no upper bound in the case of a prefix,
|
||||||
|
/// and no lower bound in the case of a suffix
|
||||||
|
///
|
||||||
|
/// If these conditions are satisfied, the function
|
||||||
|
/// returns the corresponding `RemoveAffixData` object;
|
||||||
|
/// otherwise it returns `None`.
|
||||||
|
fn affix_removal_data<'a>(
|
||||||
|
value: &'a ast::Expr,
|
||||||
|
test: &'a ast::Expr,
|
||||||
|
else_or_target: &'a ast::Expr,
|
||||||
|
slice: &'a ast::Expr,
|
||||||
|
) -> Option<RemoveAffixData<'a>> {
|
||||||
|
let compr_value = ast::comparable::ComparableExpr::from(value);
|
||||||
|
let compr_else_or_target = ast::comparable::ComparableExpr::from(else_or_target);
|
||||||
|
if compr_value != compr_else_or_target {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let slice = slice.as_slice_expr()?;
|
||||||
|
let compr_test_expr = ast::comparable::ComparableExpr::from(
|
||||||
|
&test.as_call_expr()?.func.as_attribute_expr()?.value,
|
||||||
|
);
|
||||||
|
let func_name = test
|
||||||
|
.as_call_expr()?
|
||||||
|
.func
|
||||||
|
.as_attribute_expr()?
|
||||||
|
.attr
|
||||||
|
.id
|
||||||
|
.as_str();
|
||||||
|
|
||||||
|
let func_args = &test.as_call_expr()?.arguments.args;
|
||||||
|
|
||||||
|
let [affix] = func_args.as_ref() else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
if compr_value != compr_test_expr || compr_test_expr != compr_else_or_target {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let (affix_kind, bound) = match func_name {
|
||||||
|
"startswith" if slice.upper.is_none() => (AffixKind::StartsWith, slice.lower.as_ref()?),
|
||||||
|
"endswith" if slice.lower.is_none() => (AffixKind::EndsWith, slice.upper.as_ref()?),
|
||||||
|
_ => return None,
|
||||||
|
};
|
||||||
|
Some(RemoveAffixData {
|
||||||
|
text: value,
|
||||||
|
bound,
|
||||||
|
affix_query: AffixQuery {
|
||||||
|
kind: affix_kind,
|
||||||
|
affix,
|
||||||
|
},
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Tests whether the slice of the given string actually removes the
|
||||||
|
/// detected affix.
|
||||||
|
///
|
||||||
|
/// For example, in the situation
|
||||||
|
///
|
||||||
|
/// ```python
|
||||||
|
/// text[:bound] if text.endswith(suffix) else text
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// This function verifies that `bound == -len(suffix)` in two cases:
|
||||||
|
/// - `suffix` is a string literal and `bound` is a number literal
|
||||||
|
/// - `suffix` is an expression and `bound` is
|
||||||
|
/// exactly `-len(suffix)` (as AST nodes, prior to evaluation.)
|
||||||
|
fn affix_matches_slice_bound(data: &RemoveAffixData, semantic: &SemanticModel) -> bool {
|
||||||
|
let RemoveAffixData {
|
||||||
|
text: _,
|
||||||
|
bound,
|
||||||
|
affix_query: AffixQuery { kind, affix },
|
||||||
|
} = *data;
|
||||||
|
|
||||||
|
match (kind, bound, affix) {
|
||||||
|
(
|
||||||
|
AffixKind::StartsWith,
|
||||||
|
ast::Expr::NumberLiteral(ast::ExprNumberLiteral {
|
||||||
|
value: num,
|
||||||
|
range: _,
|
||||||
|
}),
|
||||||
|
ast::Expr::StringLiteral(ast::ExprStringLiteral {
|
||||||
|
range: _,
|
||||||
|
value: string_val,
|
||||||
|
}),
|
||||||
|
) => num
|
||||||
|
.as_int()
|
||||||
|
.and_then(ast::Int::as_u32) // Only support prefix removal for size at most `u32::MAX`
|
||||||
|
.is_some_and(|x| x == string_val.to_str().text_len().to_u32()),
|
||||||
|
(
|
||||||
|
AffixKind::StartsWith,
|
||||||
|
ast::Expr::Call(ast::ExprCall {
|
||||||
|
range: _,
|
||||||
|
func,
|
||||||
|
arguments,
|
||||||
|
}),
|
||||||
|
_,
|
||||||
|
) => {
|
||||||
|
arguments.len() == 1
|
||||||
|
&& arguments.find_positional(0).is_some_and(|arg| {
|
||||||
|
let compr_affix = ast::comparable::ComparableExpr::from(affix);
|
||||||
|
let compr_arg = ast::comparable::ComparableExpr::from(arg);
|
||||||
|
compr_affix == compr_arg
|
||||||
|
})
|
||||||
|
&& semantic.match_builtin_expr(func, "len")
|
||||||
|
}
|
||||||
|
(
|
||||||
|
AffixKind::EndsWith,
|
||||||
|
ast::Expr::UnaryOp(ast::ExprUnaryOp {
|
||||||
|
op: ast::UnaryOp::USub,
|
||||||
|
operand,
|
||||||
|
range: _,
|
||||||
|
}),
|
||||||
|
ast::Expr::StringLiteral(ast::ExprStringLiteral {
|
||||||
|
range: _,
|
||||||
|
value: string_val,
|
||||||
|
}),
|
||||||
|
) => operand.as_number_literal_expr().is_some_and(
|
||||||
|
|ast::ExprNumberLiteral { value, .. }| {
|
||||||
|
// Only support prefix removal for size at most `u32::MAX`
|
||||||
|
value
|
||||||
|
.as_int()
|
||||||
|
.and_then(ast::Int::as_u32)
|
||||||
|
.is_some_and(|x| x == string_val.to_str().text_len().to_u32())
|
||||||
|
},
|
||||||
|
),
|
||||||
|
(
|
||||||
|
AffixKind::EndsWith,
|
||||||
|
ast::Expr::UnaryOp(ast::ExprUnaryOp {
|
||||||
|
op: ast::UnaryOp::USub,
|
||||||
|
operand,
|
||||||
|
range: _,
|
||||||
|
}),
|
||||||
|
_,
|
||||||
|
) => operand.as_call_expr().is_some_and(
|
||||||
|
|ast::ExprCall {
|
||||||
|
range: _,
|
||||||
|
func,
|
||||||
|
arguments,
|
||||||
|
}| {
|
||||||
|
arguments.len() == 1
|
||||||
|
&& arguments.find_positional(0).is_some_and(|arg| {
|
||||||
|
let compr_affix = ast::comparable::ComparableExpr::from(affix);
|
||||||
|
let compr_arg = ast::comparable::ComparableExpr::from(arg);
|
||||||
|
compr_affix == compr_arg
|
||||||
|
})
|
||||||
|
&& semantic.match_builtin_expr(func, "len")
|
||||||
|
},
|
||||||
|
),
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Generates the source code string
|
||||||
|
/// ```python
|
||||||
|
/// text = text.removeprefix(prefix)
|
||||||
|
/// ```
|
||||||
|
/// or
|
||||||
|
/// ```python
|
||||||
|
/// text = text.removesuffix(prefix)
|
||||||
|
/// ```
|
||||||
|
/// as appropriate.
|
||||||
|
fn generate_assignment_with_removeaffix(
|
||||||
|
text: &ast::Expr,
|
||||||
|
affix_query: &AffixQuery,
|
||||||
|
locator: &Locator,
|
||||||
|
) -> String {
|
||||||
|
let text_str = locator.slice(text);
|
||||||
|
let affix_str = locator.slice(affix_query.affix);
|
||||||
|
let replacement = affix_query.kind.replacement();
|
||||||
|
format!("{text_str} = {text_str}.{replacement}({affix_str})")
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Generates the source code string
|
||||||
|
/// ```python
|
||||||
|
/// text.removeprefix(prefix)
|
||||||
|
/// ```
|
||||||
|
/// or
|
||||||
|
///
|
||||||
|
/// ```python
|
||||||
|
/// text.removesuffix(suffix)
|
||||||
|
/// ```
|
||||||
|
/// as appropriate.
|
||||||
|
fn generate_removeaffix_expr(
|
||||||
|
text: &ast::Expr,
|
||||||
|
affix_query: &AffixQuery,
|
||||||
|
locator: &Locator,
|
||||||
|
) -> String {
|
||||||
|
let text_str = locator.slice(text);
|
||||||
|
let affix_str = locator.slice(affix_query.affix);
|
||||||
|
let replacement = affix_query.kind.replacement();
|
||||||
|
format!("{text_str}.{replacement}({affix_str})")
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
|
||||||
|
enum StmtOrExpr {
|
||||||
|
Statement,
|
||||||
|
Expression,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
|
||||||
|
enum AffixKind {
|
||||||
|
StartsWith,
|
||||||
|
EndsWith,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl AffixKind {
|
||||||
|
const fn as_str(self) -> &'static str {
|
||||||
|
match self {
|
||||||
|
Self::StartsWith => "startswith",
|
||||||
|
Self::EndsWith => "endswith",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const fn replacement(self) -> &'static str {
|
||||||
|
match self {
|
||||||
|
Self::StartsWith => "removeprefix",
|
||||||
|
Self::EndsWith => "removesuffix",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Components of `startswith(prefix)` or `endswith(suffix)`.
|
||||||
|
#[derive(Debug)]
|
||||||
|
struct AffixQuery<'a> {
|
||||||
|
/// Whether the method called is `startswith` or `endswith`.
|
||||||
|
kind: AffixKind,
|
||||||
|
/// Node representing the prefix or suffix being passed to the string method.
|
||||||
|
affix: &'a ast::Expr,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Ingredients for a statement or expression
|
||||||
|
/// which potentially removes a prefix or suffix from a string.
|
||||||
|
///
|
||||||
|
/// Specifically
|
||||||
|
#[derive(Debug)]
|
||||||
|
struct RemoveAffixData<'a> {
|
||||||
|
/// Node representing the string whose prefix or suffix we want to remove
|
||||||
|
text: &'a ast::Expr,
|
||||||
|
/// Node representing the bound used to slice the string
|
||||||
|
bound: &'a ast::Expr,
|
||||||
|
/// Contains the prefix or suffix used in `text.startswith(prefix)` or `text.endswith(suffix)`
|
||||||
|
affix_query: AffixQuery<'a>,
|
||||||
|
}
|
|
@ -0,0 +1,177 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/refurb/mod.rs
|
||||||
|
---
|
||||||
|
FURB188.py:7:5: FURB188 [*] Prefer `removesuffix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
6 | def remove_extension_via_slice(filename: str) -> str:
|
||||||
|
7 | if filename.endswith(".txt"):
|
||||||
|
| _____^
|
||||||
|
8 | | filename = filename[:-4]
|
||||||
|
| |________________________________^ FURB188
|
||||||
|
9 |
|
||||||
|
10 | return filename
|
||||||
|
|
|
||||||
|
= help: Use removesuffix instead of assignment conditional upon endswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
4 4 | # these should match
|
||||||
|
5 5 |
|
||||||
|
6 6 | def remove_extension_via_slice(filename: str) -> str:
|
||||||
|
7 |- if filename.endswith(".txt"):
|
||||||
|
8 |- filename = filename[:-4]
|
||||||
|
7 |+ filename = filename.removesuffix(".txt")
|
||||||
|
9 8 |
|
||||||
|
10 9 | return filename
|
||||||
|
11 10 |
|
||||||
|
|
||||||
|
FURB188.py:14:5: FURB188 [*] Prefer `removesuffix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
13 | def remove_extension_via_slice_len(filename: str, extension: str) -> str:
|
||||||
|
14 | if filename.endswith(extension):
|
||||||
|
| _____^
|
||||||
|
15 | | filename = filename[:-len(extension)]
|
||||||
|
| |_____________________________________________^ FURB188
|
||||||
|
16 |
|
||||||
|
17 | return filename
|
||||||
|
|
|
||||||
|
= help: Use removesuffix instead of assignment conditional upon endswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
11 11 |
|
||||||
|
12 12 |
|
||||||
|
13 13 | def remove_extension_via_slice_len(filename: str, extension: str) -> str:
|
||||||
|
14 |- if filename.endswith(extension):
|
||||||
|
15 |- filename = filename[:-len(extension)]
|
||||||
|
14 |+ filename = filename.removesuffix(extension)
|
||||||
|
16 15 |
|
||||||
|
17 16 | return filename
|
||||||
|
18 17 |
|
||||||
|
|
||||||
|
FURB188.py:21:12: FURB188 [*] Prefer `removesuffix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
20 | def remove_extension_via_ternary(filename: str) -> str:
|
||||||
|
21 | return filename[:-4] if filename.endswith(".txt") else filename
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
|
||||||
|
|
|
||||||
|
= help: Use removesuffix instead of ternary expression conditional upon endswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
18 18 |
|
||||||
|
19 19 |
|
||||||
|
20 20 | def remove_extension_via_ternary(filename: str) -> str:
|
||||||
|
21 |- return filename[:-4] if filename.endswith(".txt") else filename
|
||||||
|
21 |+ return filename.removesuffix(".txt")
|
||||||
|
22 22 |
|
||||||
|
23 23 |
|
||||||
|
24 24 | def remove_extension_via_ternary_with_len(filename: str, extension: str) -> str:
|
||||||
|
|
||||||
|
FURB188.py:25:12: FURB188 [*] Prefer `removesuffix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
24 | def remove_extension_via_ternary_with_len(filename: str, extension: str) -> str:
|
||||||
|
25 | return filename[:-len(extension)] if filename.endswith(extension) else filename
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
|
||||||
|
|
|
||||||
|
= help: Use removesuffix instead of ternary expression conditional upon endswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
22 22 |
|
||||||
|
23 23 |
|
||||||
|
24 24 | def remove_extension_via_ternary_with_len(filename: str, extension: str) -> str:
|
||||||
|
25 |- return filename[:-len(extension)] if filename.endswith(extension) else filename
|
||||||
|
25 |+ return filename.removesuffix(extension)
|
||||||
|
26 26 |
|
||||||
|
27 27 |
|
||||||
|
28 28 | def remove_prefix(filename: str) -> str:
|
||||||
|
|
||||||
|
FURB188.py:29:12: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
28 | def remove_prefix(filename: str) -> str:
|
||||||
|
29 | return filename[4:] if filename.startswith("abc-") else filename
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
|
||||||
|
|
|
||||||
|
= help: Use removeprefix instead of ternary expression conditional upon startswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
26 26 |
|
||||||
|
27 27 |
|
||||||
|
28 28 | def remove_prefix(filename: str) -> str:
|
||||||
|
29 |- return filename[4:] if filename.startswith("abc-") else filename
|
||||||
|
29 |+ return filename.removeprefix("abc-")
|
||||||
|
30 30 |
|
||||||
|
31 31 |
|
||||||
|
32 32 | def remove_prefix_via_len(filename: str, prefix: str) -> str:
|
||||||
|
|
||||||
|
FURB188.py:33:12: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
32 | def remove_prefix_via_len(filename: str, prefix: str) -> str:
|
||||||
|
33 | return filename[len(prefix):] if filename.startswith(prefix) else filename
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
|
||||||
|
|
|
||||||
|
= help: Use removeprefix instead of ternary expression conditional upon startswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
30 30 |
|
||||||
|
31 31 |
|
||||||
|
32 32 | def remove_prefix_via_len(filename: str, prefix: str) -> str:
|
||||||
|
33 |- return filename[len(prefix):] if filename.startswith(prefix) else filename
|
||||||
|
33 |+ return filename.removeprefix(prefix)
|
||||||
|
34 34 |
|
||||||
|
35 35 |
|
||||||
|
36 36 | # these should not
|
||||||
|
|
||||||
|
FURB188.py:146:9: FURB188 [*] Prefer `removesuffix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
144 | SUFFIX = "suffix"
|
||||||
|
145 |
|
||||||
|
146 | x = foo.bar.baz[:-len(SUFFIX)] if foo.bar.baz.endswith(SUFFIX) else foo.bar.baz
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
|
||||||
|
147 |
|
||||||
|
148 | def remove_prefix_comparable_literal_expr() -> None:
|
||||||
|
|
|
||||||
|
= help: Use removesuffix instead of ternary expression conditional upon endswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
143 143 |
|
||||||
|
144 144 | SUFFIX = "suffix"
|
||||||
|
145 145 |
|
||||||
|
146 |- x = foo.bar.baz[:-len(SUFFIX)] if foo.bar.baz.endswith(SUFFIX) else foo.bar.baz
|
||||||
|
146 |+ x = foo.bar.baz.removesuffix(SUFFIX)
|
||||||
|
147 147 |
|
||||||
|
148 148 | def remove_prefix_comparable_literal_expr() -> None:
|
||||||
|
149 149 | return ("abc" "def")[3:] if ("abc" "def").startswith("abc") else "abc" "def"
|
||||||
|
|
||||||
|
FURB188.py:149:12: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
148 | def remove_prefix_comparable_literal_expr() -> None:
|
||||||
|
149 | return ("abc" "def")[3:] if ("abc" "def").startswith("abc") else "abc" "def"
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
|
||||||
|
150 |
|
||||||
|
151 | def shadow_builtins(filename: str, extension: str) -> None:
|
||||||
|
|
|
||||||
|
= help: Use removeprefix instead of ternary expression conditional upon startswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
146 146 | x = foo.bar.baz[:-len(SUFFIX)] if foo.bar.baz.endswith(SUFFIX) else foo.bar.baz
|
||||||
|
147 147 |
|
||||||
|
148 148 | def remove_prefix_comparable_literal_expr() -> None:
|
||||||
|
149 |- return ("abc" "def")[3:] if ("abc" "def").startswith("abc") else "abc" "def"
|
||||||
|
149 |+ return "abc" "def".removeprefix("abc")
|
||||||
|
150 150 |
|
||||||
|
151 151 | def shadow_builtins(filename: str, extension: str) -> None:
|
||||||
|
152 152 | from builtins import len as builtins_len
|
||||||
|
|
||||||
|
FURB188.py:154:12: FURB188 [*] Prefer `removesuffix` over conditionally replacing with slice.
|
||||||
|
|
|
||||||
|
152 | from builtins import len as builtins_len
|
||||||
|
153 |
|
||||||
|
154 | return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
|
||||||
|
|
|
||||||
|
= help: Use removesuffix instead of ternary expression conditional upon endswith.
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
151 151 | def shadow_builtins(filename: str, extension: str) -> None:
|
||||||
|
152 152 | from builtins import len as builtins_len
|
||||||
|
153 153 |
|
||||||
|
154 |- return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
|
||||||
|
154 |+ return filename.removesuffix(extension)
|
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
@ -3180,6 +3180,7 @@
|
||||||
"FURB180",
|
"FURB180",
|
||||||
"FURB181",
|
"FURB181",
|
||||||
"FURB187",
|
"FURB187",
|
||||||
|
"FURB188",
|
||||||
"FURB19",
|
"FURB19",
|
||||||
"FURB192",
|
"FURB192",
|
||||||
"G",
|
"G",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue