[refurb] Fix FURB163 autofix creating a syntax error for yield expressions (#18756)

crates/ruff_linter/resources/test/fixtures/refurb/FURB163.py

@@ -44,6 +44,13 @@ log(1, math.e)
 math.log(1, 2.0001)
 math.log(1, 10.0001)
 
+# https://github.com/astral-sh/ruff/issues/18747
+def log():
+    yield math.log((yield), math.e)
+
+
+def log():
+    yield math.log((yield from x), math.e)
 
 # see: https://github.com/astral-sh/ruff/issues/18639
 math.log(1, 10 # comment

crates/ruff_linter/src/rules/refurb/rules/redundant_log_base.rs

@@ -1,6 +1,7 @@
 use anyhow::Result;
 use ruff_diagnostics::Applicability;
 use ruff_macros::{ViolationMetadata, derive_message_formats};
+use ruff_python_ast::parenthesize::parenthesized_range;
 use ruff_python_ast::{self as ast, Expr, Number};
 use ruff_text_size::Ranged;
 
@@ -150,10 +151,17 @@ fn generate_fix(checker: &Checker, call: &ast::ExprCall, base: Base, arg: &Expr)
         checker.semantic(),
     )?;
 
-    let number = checker.locator().slice(arg);
+    let arg_range = parenthesized_range(
+        arg.into(),
+        call.into(),
+        checker.comment_ranges(),
+        checker.source(),
+    )
+    .unwrap_or(arg.range());
+    let arg_str = checker.locator().slice(arg_range);
 
     Ok(Fix::applicable_edits(
-        Edit::range_replacement(format!("{binding}({number})"), call.range()),
+        Edit::range_replacement(format!("{binding}({arg_str})"), call.range()),
         [edit],
         if (matches!(base, Base::Two | Base::Ten))
             || arg.is_starred_expr()

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB163_FURB163.py.snap

@@ -167,163 +167,202 @@ FURB163.py:12:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redun
 14 14 | # OK
 15 15 | math.log2(1)
 
-FURB163.py:49:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
+FURB163.py:49:11: FURB163 [*] Prefer `math.log(yield)` over `math.log` with a redundant base
    |
-48 |   # see: https://github.com/astral-sh/ruff/issues/18639
+47 | # https://github.com/astral-sh/ruff/issues/18747
-49 | / math.log(1, 10 # comment
+48 | def log():
-50 | |          )
+49 |     yield math.log((yield), math.e)
-   | |__________^ FURB163
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^ FURB163
-51 |
-52 |   math.log(1,
    |
-   = help: Replace with `math.log10(1)`
+   = help: Replace with `math.log(yield)`
 
-ℹ Unsafe fix
+ℹ Safe fix
 46 46 | 
-47 47 | 
+47 47 | # https://github.com/astral-sh/ruff/issues/18747
-48 48 | # see: https://github.com/astral-sh/ruff/issues/18639
+48 48 | def log():
-49    |-math.log(1, 10 # comment
+49    |-    yield math.log((yield), math.e)
-50    |-         )
+   49 |+    yield math.log((yield))
-   49 |+math.log10(1)
+50 50 | 
-51 50 | 
-52 51 | math.log(1,
-53 52 |          10 # comment
-
-FURB163.py:52:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
-   |
-50 |            )
-51 |
-52 | / math.log(1,
-53 | |          10 # comment
-54 | |          )
-   | |__________^ FURB163
-55 |
-56 |   math.log(1 # comment
-   |
-   = help: Replace with `math.log10(1)`
-
-ℹ Unsafe fix
-49 49 | math.log(1, 10 # comment
-50 50 |          )
 51 51 | 
-52    |-math.log(1,
+52 52 | def log():
-53    |-         10 # comment
+
-54    |-         )
+FURB163.py:53:11: FURB163 [*] Prefer `math.log(yield from x)` over `math.log` with a redundant base
-   52 |+math.log10(1)
+   |
-55 53 | 
+52 | def log():
-56 54 | math.log(1 # comment
+53 |     yield math.log((yield from x), math.e)
-57 55 |          , # comment
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB163
+54 |
+55 | # see: https://github.com/astral-sh/ruff/issues/18639
+   |
+   = help: Replace with `math.log(yield from x)`
+
+ℹ Safe fix
+50 50 | 
+51 51 | 
+52 52 | def log():
+53    |-    yield math.log((yield from x), math.e)
+   53 |+    yield math.log((yield from x))
+54 54 | 
+55 55 | # see: https://github.com/astral-sh/ruff/issues/18639
+56 56 | math.log(1, 10 # comment
 
 FURB163.py:56:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
    |
-54 |            )
+55 |   # see: https://github.com/astral-sh/ruff/issues/18639
-55 |
+56 | / math.log(1, 10 # comment
-56 | / math.log(1 # comment
+57 | |          )
-57 | |          , # comment
-58 | |          10 # comment
-59 | |          )
    | |__________^ FURB163
-60 |
+58 |
-61 |   math.log(
+59 |   math.log(1,
    |
    = help: Replace with `math.log10(1)`
 
 ℹ Unsafe fix
-53 53 |          10 # comment
+53 53 |     yield math.log((yield from x), math.e)
-54 54 |          )
+54 54 | 
-55 55 | 
+55 55 | # see: https://github.com/astral-sh/ruff/issues/18639
-56    |-math.log(1 # comment
+56    |-math.log(1, 10 # comment
-57    |-         , # comment
+57    |-         )
-58    |-         10 # comment
-59    |-         )
    56 |+math.log10(1)
-60 57 | 
+58 57 | 
-61 58 | math.log(
+59 58 | math.log(1,
-62 59 |     1 # comment
+60 59 |          10 # comment
 
-FURB163.py:61:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
+FURB163.py:59:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
    |
-59 |            )
+57 |            )
-60 |
+58 |
-61 | / math.log(
+59 | / math.log(1,
-62 | |     1 # comment
+60 | |          10 # comment
-63 | |     ,
+61 | |          )
-64 | |     10 # comment
+   | |__________^ FURB163
-65 | | )
+62 |
-   | |_^ FURB163
+63 |   math.log(1 # comment
-66 |
-67 |   math.log(4.13e223, 2)
    |
    = help: Replace with `math.log10(1)`
 
 ℹ Unsafe fix
-58 58 |          10 # comment
+56 56 | math.log(1, 10 # comment
-59 59 |          )
+57 57 |          )
-60 60 | 
+58 58 | 
-61    |-math.log(
+59    |-math.log(1,
-62    |-    1 # comment
+60    |-         10 # comment
-63    |-    ,
+61    |-         )
-64    |-    10 # comment
+   59 |+math.log10(1)
-65    |-)
+62 60 | 
-   61 |+math.log10(1)
+63 61 | math.log(1 # comment
-66 62 | 
+64 62 |          , # comment
-67 63 | math.log(4.13e223, 2)
-68 64 | math.log(4.14e223, 10)
 
-FURB163.py:67:1: FURB163 [*] Prefer `math.log2(4.13e223)` over `math.log` with a redundant base
+FURB163.py:63:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
    |
-65 | )
+61 |            )
-66 |
+62 |
-67 | math.log(4.13e223, 2)
+63 | / math.log(1 # comment
+64 | |          , # comment
+65 | |          10 # comment
+66 | |          )
+   | |__________^ FURB163
+67 |
+68 |   math.log(
+   |
+   = help: Replace with `math.log10(1)`
+
+ℹ Unsafe fix
+60 60 |          10 # comment
+61 61 |          )
+62 62 | 
+63    |-math.log(1 # comment
+64    |-         , # comment
+65    |-         10 # comment
+66    |-         )
+   63 |+math.log10(1)
+67 64 | 
+68 65 | math.log(
+69 66 |     1 # comment
+
+FURB163.py:68:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
+   |
+66 |            )
+67 |
+68 | / math.log(
+69 | |     1 # comment
+70 | |     ,
+71 | |     10 # comment
+72 | | )
+   | |_^ FURB163
+73 |
+74 |   math.log(4.13e223, 2)
+   |
+   = help: Replace with `math.log10(1)`
+
+ℹ Unsafe fix
+65 65 |          10 # comment
+66 66 |          )
+67 67 | 
+68    |-math.log(
+69    |-    1 # comment
+70    |-    ,
+71    |-    10 # comment
+72    |-)
+   68 |+math.log10(1)
+73 69 | 
+74 70 | math.log(4.13e223, 2)
+75 71 | math.log(4.14e223, 10)
+
+FURB163.py:74:1: FURB163 [*] Prefer `math.log2(4.13e223)` over `math.log` with a redundant base
+   |
+72 | )
+73 |
+74 | math.log(4.13e223, 2)
    | ^^^^^^^^^^^^^^^^^^^^^ FURB163
-68 | math.log(4.14e223, 10)
+75 | math.log(4.14e223, 10)
    |
    = help: Replace with `math.log2(4.13e223)`
 
 ℹ Unsafe fix
-64 64 |     10 # comment
+71 71 |     10 # comment
-65 65 | )
+72 72 | )
-66 66 | 
+73 73 | 
-67    |-math.log(4.13e223, 2)
+74    |-math.log(4.13e223, 2)
-   67 |+math.log2(4.13e223)
+   74 |+math.log2(4.13e223)
-68 68 | math.log(4.14e223, 10)
+75 75 | math.log(4.14e223, 10)
-69 69 | 
+76 76 | 
-70 70 | 
+77 77 | 
 
-FURB163.py:68:1: FURB163 [*] Prefer `math.log10(4.14e223)` over `math.log` with a redundant base
+FURB163.py:75:1: FURB163 [*] Prefer `math.log10(4.14e223)` over `math.log` with a redundant base
    |
-67 | math.log(4.13e223, 2)
+74 | math.log(4.13e223, 2)
-68 | math.log(4.14e223, 10)
+75 | math.log(4.14e223, 10)
    | ^^^^^^^^^^^^^^^^^^^^^^ FURB163
    |
    = help: Replace with `math.log10(4.14e223)`
 
 ℹ Unsafe fix
-65 65 | )
+72 72 | )
-66 66 | 
+73 73 | 
-67 67 | math.log(4.13e223, 2)
+74 74 | math.log(4.13e223, 2)
-68    |-math.log(4.14e223, 10)
+75    |-math.log(4.14e223, 10)
-   68 |+math.log10(4.14e223)
+   75 |+math.log10(4.14e223)
-69 69 | 
+76 76 | 
-70 70 | 
+77 77 | 
-71 71 | def print_log(*args):
+78 78 | def print_log(*args):
 
-FURB163.py:73:15: FURB163 [*] Prefer `math.log(*args)` over `math.log` with a redundant base
+FURB163.py:80:15: FURB163 [*] Prefer `math.log(*args)` over `math.log` with a redundant base
    |
-71 | def print_log(*args):
+78 | def print_log(*args):
-72 |     try:
+79 |     try:
-73 |         print(math.log(*args, math.e))
+80 |         print(math.log(*args, math.e))
    |               ^^^^^^^^^^^^^^^^^^^^^^^ FURB163
-74 |     except TypeError as e:
+81 |     except TypeError as e:
-75 |         print(repr(e))
+82 |         print(repr(e))
    |
    = help: Replace with `math.log(*args)`
 
 ℹ Unsafe fix
-70 70 | 
+77 77 | 
-71 71 | def print_log(*args):
+78 78 | def print_log(*args):
-72 72 |     try:
+79 79 |     try:
-73    |-        print(math.log(*args, math.e))
+80    |-        print(math.log(*args, math.e))
-   73 |+        print(math.log(*args))
+   80 |+        print(math.log(*args))
-74 74 |     except TypeError as e:
+81 81 |     except TypeError as e:
-75 75 |         print(repr(e))
+82 82 |         print(repr(e))