Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refurb] Skip slice-to-remove-prefix-or-suffix (FURB188) when nontrivial slice step is present #13405

Merged
merged 7 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/refurb/FURB188.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,22 @@ def remove_prefix_comparable_literal_expr() -> None:
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
return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename

def okay_steps():
text = "!x!y!z"
if text.startswith("!"):
text = text[1::1]
if text.startswith("!"):
text = text[1::True]
if text.startswith("!"):
text = text[1::None]
print(text)


# this should be skipped
def ignore_step():
text = "!x!y!z"
if text.startswith("!"):
text = text[1::2]
print(text)
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,27 @@ fn affix_removal_data<'a>(
return None;
}
let slice = slice.as_slice_expr()?;

// Exit early if slice step is...
if slice
.step
.as_deref()
// present and
.is_some_and(|step| match step {
// not equal to 1
ast::Expr::NumberLiteral(ast::ExprNumberLiteral {
value: ast::Number::Int(x),
..
}) => x.as_u8() != Some(1),
// and not equal to `None` or `True`
ast::Expr::NoneLiteral(_)
| ast::Expr::BooleanLiteral(ast::ExprBooleanLiteral { value: true, .. }) => false,
_ => true,
})
{
return None;
};

let compr_test_expr = ast::comparable::ComparableExpr::from(
&test.as_call_expr()?.func.as_attribute_expr()?.value,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ FURB188.py:154:12: FURB188 [*] Prefer `removesuffix` over conditionally replacin
153 |
154 | return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB188
155 |
156 | def okay_steps():
|
= help: Use removesuffix instead of ternary expression conditional upon endswith.

Expand All @@ -175,3 +177,77 @@ FURB188.py:154:12: FURB188 [*] Prefer `removesuffix` over conditionally replacin
153 153 |
154 |- return filename[:-builtins_len(extension)] if filename.endswith(extension) else filename
154 |+ return filename.removesuffix(extension)
155 155 |
156 156 | def okay_steps():
157 157 | text = "!x!y!z"

FURB188.py:158:5: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
|
156 | def okay_steps():
157 | text = "!x!y!z"
158 | if text.startswith("!"):
| _____^
159 | | text = text[1::1]
| |_________________________^ FURB188
160 | if text.startswith("!"):
161 | text = text[1::True]
|
= help: Use removeprefix instead of assignment conditional upon startswith.

ℹ Safe fix
155 155 |
156 156 | def okay_steps():
157 157 | text = "!x!y!z"
158 |- if text.startswith("!"):
159 |- text = text[1::1]
158 |+ text = text.removeprefix("!")
160 159 | if text.startswith("!"):
161 160 | text = text[1::True]
162 161 | if text.startswith("!"):

FURB188.py:160:5: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
|
158 | if text.startswith("!"):
159 | text = text[1::1]
160 | if text.startswith("!"):
| _____^
161 | | text = text[1::True]
| |____________________________^ FURB188
162 | if text.startswith("!"):
163 | text = text[1::None]
|
= help: Use removeprefix instead of assignment conditional upon startswith.

ℹ Safe fix
157 157 | text = "!x!y!z"
158 158 | if text.startswith("!"):
159 159 | text = text[1::1]
160 |- if text.startswith("!"):
161 |- text = text[1::True]
160 |+ text = text.removeprefix("!")
162 161 | if text.startswith("!"):
163 162 | text = text[1::None]
164 163 | print(text)

FURB188.py:162:5: FURB188 [*] Prefer `removeprefix` over conditionally replacing with slice.
|
160 | if text.startswith("!"):
161 | text = text[1::True]
162 | if text.startswith("!"):
| _____^
163 | | text = text[1::None]
| |____________________________^ FURB188
164 | print(text)
|
= help: Use removeprefix instead of assignment conditional upon startswith.

ℹ Safe fix
159 159 | text = text[1::1]
160 160 | if text.startswith("!"):
161 161 | text = text[1::True]
162 |- if text.startswith("!"):
163 |- text = text[1::None]
162 |+ text = text.removeprefix("!")
164 163 | print(text)
165 164 |
166 165 |
Loading