forked from astral-sh/ruff
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
## Summary Implement [`simplify-math-log`](https://github.com/dosisod/refurb/blob/master/refurb/checks/math/simplify_log.py) as `redundant-log-base` (`FURB163`). Auto-fixes ```python import math math.log(2, 2) ``` to ```python import math math.log2(2) ``` Related to astral-sh#1348. ## Test Plan `cargo test`
- Loading branch information
Showing
8 changed files
with
437 additions
and
0 deletions.
There are no files selected for viewing
47 changes: 47 additions & 0 deletions
47
crates/ruff_linter/resources/test/fixtures/refurb/FURB163.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import math | ||
|
||
from math import e as special_e | ||
from math import log as special_log | ||
|
||
# Errors. | ||
math.log(1, 2) | ||
math.log(1, 10) | ||
math.log(1, math.e) | ||
foo = ... | ||
math.log(foo, 2) | ||
math.log(foo, 10) | ||
math.log(foo, math.e) | ||
math.log(1, special_e) | ||
special_log(1, 2) | ||
special_log(1, 10) | ||
special_log(1, math.e) | ||
special_log(1, special_e) | ||
|
||
# Ok. | ||
math.log2(1) | ||
math.log10(1) | ||
math.log(1) | ||
math.log(1, 3) | ||
math.log(1, math.pi) | ||
|
||
two = 2 | ||
math.log(1, two) | ||
|
||
ten = 10 | ||
math.log(1, ten) | ||
|
||
e = math.e | ||
math.log(1, e) | ||
|
||
math.log2(1, 10) # math.log2 takes only one argument. | ||
math.log10(1, 2) # math.log10 takes only one argument. | ||
|
||
math.log(1, base=2) # math.log does not accept keyword arguments. | ||
|
||
def log(*args): | ||
print(f"Logging: {args}") | ||
|
||
|
||
log(1, 2) | ||
log(1, 10) | ||
log(1, math.e) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
149 changes: 149 additions & 0 deletions
149
crates/ruff_linter/src/rules/refurb/rules/redundant_log_base.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
use anyhow::Result; | ||
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; | ||
use ruff_macros::{derive_message_formats, violation}; | ||
use ruff_python_ast::{self as ast, Expr, Number}; | ||
use ruff_text_size::Ranged; | ||
|
||
use crate::checkers::ast::Checker; | ||
use crate::importer::ImportRequest; | ||
|
||
/// ## What it does | ||
/// Checks for `math.log` calls with a redundant base. | ||
/// | ||
/// ## Why is this bad? | ||
/// The default base of `math.log` is `e`, so specifying it explicitly is | ||
/// redundant. | ||
/// | ||
/// Instead of passing 2 or 10 as the base, use `math.log2` or `math.log10` | ||
/// respectively, as these dedicated variants are typically more accurate | ||
/// than `math.log`. | ||
/// | ||
/// ## Example | ||
/// ```python | ||
/// import math | ||
/// | ||
/// math.log(4, math.e) | ||
/// math.log(4, 2) | ||
/// math.log(4, 10) | ||
/// ``` | ||
/// | ||
/// Use instead: | ||
/// ```python | ||
/// import math | ||
/// | ||
/// math.log(4) | ||
/// math.log2(4) | ||
/// math.log10(4) | ||
/// ``` | ||
/// | ||
/// ## 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) | ||
/// - [Python documentation: `math.log10`](https://docs.python.org/3/library/math.html#math.log10) | ||
/// - [Python documentation: `math.e`](https://docs.python.org/3/library/math.html#math.e) | ||
#[violation] | ||
pub struct RedundantLogBase { | ||
base: Base, | ||
arg: String, | ||
} | ||
|
||
impl Violation for RedundantLogBase { | ||
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; | ||
|
||
#[derive_message_formats] | ||
fn message(&self) -> String { | ||
let RedundantLogBase { base, arg } = self; | ||
let log_function = base.to_log_function(); | ||
format!("Prefer `math.{log_function}({arg})` over `math.log` with a redundant base") | ||
} | ||
|
||
fn fix_title(&self) -> Option<String> { | ||
let RedundantLogBase { base, arg } = self; | ||
let log_function = base.to_log_function(); | ||
Some(format!("Replace with `math.{log_function}({arg})`")) | ||
} | ||
} | ||
|
||
/// FURB163 | ||
pub(crate) fn redundant_log_base(checker: &mut Checker, call: &ast::ExprCall) { | ||
if !call.arguments.keywords.is_empty() { | ||
return; | ||
} | ||
|
||
let [arg, base] = &call.arguments.args.as_slice() else { | ||
return; | ||
}; | ||
|
||
if !checker | ||
.semantic() | ||
.resolve_call_path(&call.func) | ||
.as_ref() | ||
.is_some_and(|call_path| matches!(call_path.as_slice(), ["math", "log"])) | ||
{ | ||
return; | ||
} | ||
|
||
let base = if is_number_literal(base, 2) { | ||
Base::Two | ||
} else if is_number_literal(base, 10) { | ||
Base::Ten | ||
} else if checker | ||
.semantic() | ||
.resolve_call_path(base) | ||
.as_ref() | ||
.is_some_and(|call_path| matches!(call_path.as_slice(), ["math", "e"])) | ||
{ | ||
Base::E | ||
} else { | ||
return; | ||
}; | ||
|
||
let mut diagnostic = Diagnostic::new( | ||
RedundantLogBase { | ||
base, | ||
arg: checker.locator().slice(arg).into(), | ||
}, | ||
call.range(), | ||
); | ||
diagnostic.try_set_fix(|| generate_fix(checker, call, base, arg)); | ||
checker.diagnostics.push(diagnostic); | ||
} | ||
|
||
#[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
enum Base { | ||
E, | ||
Two, | ||
Ten, | ||
} | ||
|
||
impl Base { | ||
fn to_log_function(self) -> &'static str { | ||
match self { | ||
Base::E => "log", | ||
Base::Two => "log2", | ||
Base::Ten => "log10", | ||
} | ||
} | ||
} | ||
|
||
fn is_number_literal(expr: &Expr, value: i8) -> bool { | ||
if let Expr::NumberLiteral(number_literal) = expr { | ||
if let Number::Int(number) = &number_literal.value { | ||
return number.as_i8().is_some_and(|number| number == value); | ||
} | ||
} | ||
false | ||
} | ||
|
||
fn generate_fix(checker: &Checker, call: &ast::ExprCall, base: Base, arg: &Expr) -> Result<Fix> { | ||
let (edit, binding) = checker.importer().get_or_import_symbol( | ||
&ImportRequest::import("math", base.to_log_function()), | ||
call.start(), | ||
checker.semantic(), | ||
)?; | ||
let number = checker.locator().slice(arg); | ||
Ok(Fix::safe_edits( | ||
Edit::range_replacement(format!("{binding}({number})"), call.range()), | ||
[edit], | ||
)) | ||
} |
Oops, something went wrong.