diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py new file mode 100644 index 0000000000000..3437d5c56bec4 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py @@ -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 \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index ba8bb634a903a..24d8c53dd339e 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1407,6 +1407,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::UselessIfElse) { 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( comp @ ast::ExprListComp { diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 4f73f7fe3764c..6e6e572993f23 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1178,6 +1178,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::CheckAndRemoveFromSet) { 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) { pylint::rules::too_many_boolean_expressions(checker, if_); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 25c150dfe556d..e463c338269ca 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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, "181") => (RuleGroup::Stable, rules::refurb::rules::HashlibDigestHex), (Refurb, "187") => (RuleGroup::Stable, rules::refurb::rules::ListReverseCopy), + (Refurb, "188") => (RuleGroup::Preview, rules::refurb::rules::SliceToRemovePrefixOrSuffix), (Refurb, "192") => (RuleGroup::Preview, rules::refurb::rules::SortedMinMax), // flake8-logging diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index e12f8d3b04ca3..6f438ba632e6f 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -46,6 +46,7 @@ mod tests { #[test_case(Rule::WriteWholeFile, Path::new("FURB103.py"))] #[test_case(Rule::FStringNumberFormat, Path::new("FURB116.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<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index acb57bbc55a74..aceaba6d2445c 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -23,6 +23,7 @@ pub(crate) use repeated_append::*; pub(crate) use repeated_global::*; pub(crate) use single_item_membership_test::*; pub(crate) use slice_copy::*; +pub(crate) use slice_to_remove_prefix_or_suffix::*; pub(crate) use sorted_min_max::*; pub(crate) use type_none_comparison::*; pub(crate) use unnecessary_enumerate::*; @@ -55,6 +56,7 @@ mod repeated_append; mod repeated_global; mod single_item_membership_test; mod slice_copy; +mod slice_to_remove_prefix_or_suffix; mod sorted_min_max; mod type_none_comparison; mod unnecessary_enumerate; diff --git a/crates/ruff_linter/src/rules/refurb/rules/slice_to_remove_prefix_or_suffix.rs b/crates/ruff_linter/src/rules/refurb/rules/slice_to_remove_prefix_or_suffix.rs new file mode 100644 index 0000000000000..e3fc11bfb6a36 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/slice_to_remove_prefix_or_suffix.rs @@ -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 { + 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 { + 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> { + 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>, +} diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB188_FURB188.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB188_FURB188.py.snap new file mode 100644 index 0000000000000..3103e5f2723ad --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB188_FURB188.py.snap @@ -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) diff --git a/ruff.schema.json b/ruff.schema.json index c39a68ebd6aa7..ed2f77e1dafdf 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3180,6 +3180,7 @@ "FURB180", "FURB181", "FURB187", + "FURB188", "FURB19", "FURB192", "G",