[refurb] Make the fix for FURB163 unsafe for log2, log10, *args, and deleted comments

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

@@ -43,3 +43,33 @@ def log(*args):
 
 math.log(1, 2.0001)
 math.log(1, 10.0001)
+
+
+# see: https://github.com/astral-sh/ruff/issues/18639
+math.log(1, 10 # comment
+         )
+
+math.log(1,
+         10 # comment
+         )
+
+math.log(1 # comment
+         , # comment
+         10 # comment
+         )
+
+math.log(
+    1 # comment
+    ,
+    10 # comment
+)
+
+math.log(4.13e223, 2)
+math.log(4.14e223, 10)
+
+
+def print_log(*args):
+    try:
+        print(math.log(*args, math.e))
+    except TypeError as e:
+        print(repr(e))

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

@@ -1,4 +1,5 @@
 use anyhow::Result;
+use ruff_diagnostics::Applicability;
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 use ruff_python_ast::{self as ast, Expr, Number};
 use ruff_text_size::Ranged;
@@ -36,6 +37,13 @@ use crate::{Edit, Fix, FixAvailability, Violation};
 /// math.log10(4)
 /// ```
 ///
+/// ## Fix safety
+/// This fix is marked unsafe when the argument is a starred expression, as this changes
+/// the call semantics and may raise runtime errors. It is also unsafe if comments are
+/// present within the call, as they will be removed. Additionally, `math.log(x, base)`
+/// and `math.log2(x)` / `math.log10(x)` may differ due to floating-point rounding, so
+/// the fix is also unsafe when making this transformation.
+///
 /// ## References
 /// - [Python documentation: `math.log`](https://docs.python.org/3/library/math.html#math.log)
 /// - [Python documentation: `math.log2`](https://docs.python.org/3/library/math.html#math.log2)
@@ -141,9 +149,19 @@ fn generate_fix(checker: &Checker, call: &ast::ExprCall, base: Base, arg: &Expr)
         call.start(),
         checker.semantic(),
     )?;
+
     let number = checker.locator().slice(arg);
-    Ok(Fix::safe_edits(
+
+    Ok(Fix::applicable_edits(
         Edit::range_replacement(format!("{binding}({number})"), call.range()),
         [edit],
+        if (matches!(base, Base::Two | Base::Ten))
+            || arg.is_starred_expr()
+            || checker.comment_ranges().intersects(call.range())
+        {
+            Applicability::Unsafe
+        } else {
+            Applicability::Safe
+        },
     ))
 }

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

@@ -11,7 +11,7 @@ FURB163.py:4:1: FURB163 [*] Prefer `math.log2(1)` over `math.log` with a redunda
   |
   = help: Replace with `math.log2(1)`
 
-ℹ Safe fix
+ℹ Unsafe fix
 1 1 | import math
 2 2 | 
 3 3 | # Errors
@@ -32,7 +32,7 @@ FURB163.py:5:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redund
   |
   = help: Replace with `math.log10(1)`
 
-ℹ Safe fix
+ℹ Unsafe fix
 2 2 | 
 3 3 | # Errors
 4 4 | math.log(1, 2)
@@ -74,7 +74,7 @@ FURB163.py:8:1: FURB163 [*] Prefer `math.log2(foo)` over `math.log` with a redun
    |
    = help: Replace with `math.log2(foo)`
 
-ℹ Safe fix
+ℹ Unsafe fix
 5 5 | math.log(1, 10)
 6 6 | math.log(1, math.e)
 7 7 | foo = ...
@@ -95,7 +95,7 @@ FURB163.py:9:1: FURB163 [*] Prefer `math.log10(foo)` over `math.log` with a redu
    |
    = help: Replace with `math.log10(foo)`
 
-ℹ Safe fix
+ℹ Unsafe fix
 6  6  | math.log(1, math.e)
 7  7  | foo = ...
 8  8  | math.log(foo, 2)
@@ -136,7 +136,7 @@ FURB163.py:11:1: FURB163 [*] Prefer `math.log2(1)` over `math.log` with a redund
    |
    = help: Replace with `math.log2(1)`
 
-ℹ Safe fix
+ℹ Unsafe fix
 8  8  | math.log(foo, 2)
 9  9  | math.log(foo, 10)
 10 10 | math.log(foo, math.e)
@@ -157,7 +157,7 @@ FURB163.py:12:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redun
    |
    = help: Replace with `math.log10(1)`
 
-ℹ Safe fix
+ℹ Unsafe fix
 9  9  | math.log(foo, 10)
 10 10 | math.log(foo, math.e)
 11 11 | math.log(1, 2.0)
@@ -166,3 +166,164 @@ FURB163.py:12:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redun
 13 13 | 
 14 14 | # OK
 15 15 | math.log2(1)
+
+FURB163.py:49:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
+   |
+48 |   # see: https://github.com/astral-sh/ruff/issues/18639
+49 | / math.log(1, 10 # comment
+50 | |          )
+   | |__________^ FURB163
+51 |
+52 |   math.log(1,
+   |
+   = help: Replace with `math.log10(1)`
+
+ℹ Unsafe fix
+46 46 | 
+47 47 | 
+48 48 | # see: https://github.com/astral-sh/ruff/issues/18639
+49    |-math.log(1, 10 # comment
+50    |-         )
+   49 |+math.log10(1)
+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,
+53    |-         10 # comment
+54    |-         )
+   52 |+math.log10(1)
+55 53 | 
+56 54 | math.log(1 # comment
+57 55 |          , # comment
+
+FURB163.py:56:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
+   |
+54 |            )
+55 |
+56 | / math.log(1 # comment
+57 | |          , # comment
+58 | |          10 # comment
+59 | |          )
+   | |__________^ FURB163
+60 |
+61 |   math.log(
+   |
+   = help: Replace with `math.log10(1)`
+
+ℹ Unsafe fix
+53 53 |          10 # comment
+54 54 |          )
+55 55 | 
+56    |-math.log(1 # comment
+57    |-         , # comment
+58    |-         10 # comment
+59    |-         )
+   56 |+math.log10(1)
+60 57 | 
+61 58 | math.log(
+62 59 |     1 # comment
+
+FURB163.py:61:1: FURB163 [*] Prefer `math.log10(1)` over `math.log` with a redundant base
+   |
+59 |            )
+60 |
+61 | / math.log(
+62 | |     1 # comment
+63 | |     ,
+64 | |     10 # comment
+65 | | )
+   | |_^ FURB163
+66 |
+67 |   math.log(4.13e223, 2)
+   |
+   = help: Replace with `math.log10(1)`
+
+ℹ Unsafe fix
+58 58 |          10 # comment
+59 59 |          )
+60 60 | 
+61    |-math.log(
+62    |-    1 # comment
+63    |-    ,
+64    |-    10 # comment
+65    |-)
+   61 |+math.log10(1)
+66 62 | 
+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
+   |
+65 | )
+66 |
+67 | math.log(4.13e223, 2)
+   | ^^^^^^^^^^^^^^^^^^^^^ FURB163
+68 | math.log(4.14e223, 10)
+   |
+   = help: Replace with `math.log2(4.13e223)`
+
+ℹ Unsafe fix
+64 64 |     10 # comment
+65 65 | )
+66 66 | 
+67    |-math.log(4.13e223, 2)
+   67 |+math.log2(4.13e223)
+68 68 | math.log(4.14e223, 10)
+69 69 | 
+70 70 | 
+
+FURB163.py:68:1: FURB163 [*] Prefer `math.log10(4.14e223)` over `math.log` with a redundant base
+   |
+67 | math.log(4.13e223, 2)
+68 | math.log(4.14e223, 10)
+   | ^^^^^^^^^^^^^^^^^^^^^^ FURB163
+   |
+   = help: Replace with `math.log10(4.14e223)`
+
+ℹ Unsafe fix
+65 65 | )
+66 66 | 
+67 67 | math.log(4.13e223, 2)
+68    |-math.log(4.14e223, 10)
+   68 |+math.log10(4.14e223)
+69 69 | 
+70 70 | 
+71 71 | def print_log(*args):
+
+FURB163.py:73:15: FURB163 [*] Prefer `math.log(*args)` over `math.log` with a redundant base
+   |
+71 | def print_log(*args):
+72 |     try:
+73 |         print(math.log(*args, math.e))
+   |               ^^^^^^^^^^^^^^^^^^^^^^^ FURB163
+74 |     except TypeError as e:
+75 |         print(repr(e))
+   |
+   = help: Replace with `math.log(*args)`
+
+ℹ Unsafe fix
+70 70 | 
+71 71 | def print_log(*args):
+72 72 |     try:
+73    |-        print(math.log(*args, math.e))
+   73 |+        print(math.log(*args))
+74 74 |     except TypeError as e:
+75 75 |         print(repr(e))