diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB163.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB163.py index b8aca2ebe8..d87f5b7f64 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB163.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB163.py @@ -43,3 +43,33 @@ log(1, math.e) 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)) diff --git a/crates/ruff_linter/src/rules/refurb/rules/redundant_log_base.rs b/crates/ruff_linter/src/rules/refurb/rules/redundant_log_base.rs index f1063af584..e2b202c86d 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/redundant_log_base.rs +++ b/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 + }, )) } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB163_FURB163.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB163_FURB163.py.snap index 0985c7f1c9..dd5eb1647a 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB163_FURB163.py.snap +++ b/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))