Skip to content

Commit

Permalink
Avoid autofixing within nested f-strings
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed May 18, 2023
1 parent e9c6f16 commit fd16d65
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 115 deletions.
1 change: 1 addition & 0 deletions crates/ruff/resources/test/fixtures/pyupgrade/UP018.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
bytes(b"foo"
b"bar")
bytes("foo")
f"{f'{str()}'}"

# These become string or byte literals
str()
Expand Down
13 changes: 8 additions & 5 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,12 @@ impl<'a> Checker<'a> {
}

impl<'a> Checker<'a> {
/// Return `true` if a patch should be generated under the given autofix
/// `Mode`.
/// Return `true` if a patch should be generated for a given [`Rule`].
pub fn patch(&self, code: Rule) -> bool {
self.settings.rules.should_fix(code)
self.settings.rules.should_fix(code) && !self.ctx.in_nested_f_string()
}

/// Return `true` if a `Rule` is disabled by a `noqa` directive.
/// Return `true` if a [`Rule`] is disabled by a `noqa` directive.
pub fn rule_is_ignored(&self, code: Rule, offset: TextSize) -> bool {
// TODO(charlie): `noqa` directives are mostly enforced in `check_lines.rs`.
// However, in rare cases, we need to check them here. For example, when
Expand Down Expand Up @@ -4141,7 +4140,11 @@ where
}
}
Expr::JoinedStr(_) => {
self.ctx.flags |= ContextFlags::F_STRING;
self.ctx.flags |= if self.ctx.in_f_string() {
ContextFlags::NESTED_F_STRING
} else {
ContextFlags::F_STRING
};
visitor::walk_expr(self, expr);
}
_ => visitor::walk_expr(self, expr),
Expand Down
10 changes: 9 additions & 1 deletion crates/ruff/src/rules/pyupgrade/rules/native_literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ impl AlwaysAutofixableViolation for NativeLiterals {

fn autofix_title(&self) -> String {
let NativeLiterals { literal_type } = self;
format!("Replace with `{literal_type}`")
match literal_type {
LiteralType::Str => "Replace with empty string".to_string(),
LiteralType::Bytes => "Replace with empty bytes".to_string(),
}
}
}

Expand All @@ -57,6 +60,11 @@ pub(crate) fn native_literals(
return;
}

// There's no way to rewrite, e.g., `f"{f'{str()}'}"` within a nested f-string.
if checker.ctx.in_nested_f_string() {
return;
}

if (id == "str" || id == "bytes") && checker.ctx.is_builtin(id) {
let Some(arg) = args.get(0) else {
let mut diagnostic = Diagnostic::new(NativeLiterals{literal_type:if id == "str" {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,148 +1,148 @@
---
source: crates/ruff/src/rules/pyupgrade/mod.rs
---
UP018.py:20:1: UP018 [*] Unnecessary call to `str`
UP018.py:21:1: UP018 [*] Unnecessary call to `str`
|
20 | # These become string or byte literals
21 | str()
21 | # These become string or byte literals
22 | str()
| ^^^^^ UP018
22 | str("foo")
23 | str("""
23 | str("foo")
24 | str("""
|
= help: Replace with `str`
= help: Replace with empty string

Suggested fix
17 17 | bytes("foo")
18 18 |
19 19 | # These become string or byte literals
20 |-str()
20 |+""
21 21 | str("foo")
22 22 | str("""
23 23 | foo""")
18 18 | f"{f'{str()}'}"
19 19 |
20 20 | # These become string or byte literals
21 |-str()
21 |+""
22 22 | str("foo")
23 23 | str("""
24 24 | foo""")

UP018.py:21:1: UP018 [*] Unnecessary call to `str`
UP018.py:22:1: UP018 [*] Unnecessary call to `str`
|
21 | # These become string or byte literals
22 | str()
23 | str("foo")
22 | # These become string or byte literals
23 | str()
24 | str("foo")
| ^^^^^^^^^^ UP018
24 | str("""
25 | foo""")
25 | str("""
26 | foo""")
|
= help: Replace with `str`
= help: Replace with empty string

Suggested fix
18 18 |
19 19 | # These become string or byte literals
20 20 | str()
21 |-str("foo")
21 |+"foo"
22 22 | str("""
23 23 | foo""")
24 24 | bytes()
19 19 |
20 20 | # These become string or byte literals
21 21 | str()
22 |-str("foo")
22 |+"foo"
23 23 | str("""
24 24 | foo""")
25 25 | bytes()

UP018.py:22:1: UP018 [*] Unnecessary call to `str`
UP018.py:23:1: UP018 [*] Unnecessary call to `str`
|
22 | str()
23 | str("foo")
24 | / str("""
25 | | foo""")
23 | str()
24 | str("foo")
25 | / str("""
26 | | foo""")
| |_______^ UP018
26 | bytes()
27 | bytes(b"foo")
27 | bytes()
28 | bytes(b"foo")
|
= help: Replace with `str`
= help: Replace with empty string

Suggested fix
19 19 | # These become string or byte literals
20 20 | str()
21 21 | str("foo")
22 |-str("""
23 |-foo""")
22 |+"""
23 |+foo"""
24 24 | bytes()
25 25 | bytes(b"foo")
26 26 | bytes(b"""
20 20 | # These become string or byte literals
21 21 | str()
22 22 | str("foo")
23 |-str("""
24 |-foo""")
23 |+"""
24 |+foo"""
25 25 | bytes()
26 26 | bytes(b"foo")
27 27 | bytes(b"""
UP018.py:24:1: UP018 [*] Unnecessary call to `bytes`
UP018.py:25:1: UP018 [*] Unnecessary call to `bytes`
|
24 | str("""
25 | foo""")
26 | bytes()
25 | str("""
26 | foo""")
27 | bytes()
| ^^^^^^^ UP018
27 | bytes(b"foo")
28 | bytes(b"""
28 | bytes(b"foo")
29 | bytes(b"""
|
= help: Replace with `bytes`
= help: Replace with empty bytes

Suggested fix
21 21 | str("foo")
22 22 | str("""
23 23 | foo""")
24 |-bytes()
24 |+b""
25 25 | bytes(b"foo")
26 26 | bytes(b"""
27 27 | foo""")
22 22 | str("foo")
23 23 | str("""
24 24 | foo""")
25 |-bytes()
25 |+b""
26 26 | bytes(b"foo")
27 27 | bytes(b"""
28 28 | foo""")

UP018.py:25:1: UP018 [*] Unnecessary call to `bytes`
UP018.py:26:1: UP018 [*] Unnecessary call to `bytes`
|
25 | foo""")
26 | bytes()
27 | bytes(b"foo")
26 | foo""")
27 | bytes()
28 | bytes(b"foo")
| ^^^^^^^^^^^^^ UP018
28 | bytes(b"""
29 | foo""")
29 | bytes(b"""
30 | foo""")
|
= help: Replace with `bytes`
= help: Replace with empty bytes

Suggested fix
22 22 | str("""
23 23 | foo""")
24 24 | bytes()
25 |-bytes(b"foo")
25 |+b"foo"
26 26 | bytes(b"""
27 27 | foo""")
28 28 | f"{str()}"
23 23 | str("""
24 24 | foo""")
25 25 | bytes()
26 |-bytes(b"foo")
26 |+b"foo"
27 27 | bytes(b"""
28 28 | foo""")
29 29 | f"{str()}"

UP018.py:26:1: UP018 [*] Unnecessary call to `bytes`
UP018.py:27:1: UP018 [*] Unnecessary call to `bytes`
|
26 | bytes()
27 | bytes(b"foo")
28 | / bytes(b"""
29 | | foo""")
27 | bytes()
28 | bytes(b"foo")
29 | / bytes(b"""
30 | | foo""")
| |_______^ UP018
30 | f"{str()}"
31 | f"{str()}"
|
= help: Replace with `bytes`
= help: Replace with empty bytes

Suggested fix
23 23 | foo""")
24 24 | bytes()
25 25 | bytes(b"foo")
26 |-bytes(b"""
27 |-foo""")
26 |+b"""
27 |+foo"""
28 28 | f"{str()}"
24 24 | foo""")
25 25 | bytes()
26 26 | bytes(b"foo")
27 |-bytes(b"""
28 |-foo""")
27 |+b"""
28 |+foo"""
29 29 | f"{str()}"

UP018.py:28:4: UP018 [*] Unnecessary call to `str`
UP018.py:29:4: UP018 [*] Unnecessary call to `str`
|
28 | bytes(b"""
29 | foo""")
30 | f"{str()}"
29 | bytes(b"""
30 | foo""")
31 | f"{str()}"
| ^^^^^ UP018
|
= help: Replace with `str`
= help: Replace with empty string

Suggested fix
25 25 | bytes(b"foo")
26 26 | bytes(b"""
27 27 | foo""")
28 |-f"{str()}"
28 |+f"{''}"
26 26 | bytes(b"foo")
27 27 | bytes(b"""
28 28 | foo""")
29 |-f"{str()}"
29 |+f"{''}"


Loading

0 comments on commit fd16d65

Please sign in to comment.