[refurb] Implement math-constant (FURB152) (#8727)

## Summary

Implements
[FURB152](https://github.com/dosisod/refurb/blob/master/docs/checks.md#furb152-use-math-constant)
that checks for literals that are similar to constants in `math` module,
for example:

```python
A = 3.141592 * r ** 2
```

Use instead:
```python
A = math.pi * r ** 2
```

Related to #1348.
This commit is contained in:
Tuomas Siipola 2023-11-17 19:37:44 +02:00 committed by GitHub
parent b7dbb9062c
commit 2faac1e7a8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 175 additions and 1 deletions

View file

@ -0,0 +1,7 @@
r = 3.1 # OK
A = 3.14 * r ** 2 # FURB152
C = 6.28 * r # FURB152
e = 2.71 # FURB152

View file

@ -1245,10 +1245,13 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
refurb::rules::single_item_membership_test(checker, expr, left, ops, comparators); refurb::rules::single_item_membership_test(checker, expr, left, ops, comparators);
} }
} }
Expr::NumberLiteral(_) => { Expr::NumberLiteral(number_literal @ ast::ExprNumberLiteral { .. }) => {
if checker.source_type.is_stub() && checker.enabled(Rule::NumericLiteralTooLong) { if checker.source_type.is_stub() && checker.enabled(Rule::NumericLiteralTooLong) {
flake8_pyi::rules::numeric_literal_too_long(checker, expr); flake8_pyi::rules::numeric_literal_too_long(checker, expr);
} }
if checker.enabled(Rule::MathConstant) {
refurb::rules::math_constant(checker, number_literal);
}
} }
Expr::BytesLiteral(_) => { Expr::BytesLiteral(_) => {
if checker.source_type.is_stub() && checker.enabled(Rule::StringOrBytesTooLong) { if checker.source_type.is_stub() && checker.enabled(Rule::StringOrBytesTooLong) {

View file

@ -951,6 +951,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Refurb, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap), (Refurb, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap),
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy), (Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
(Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate), (Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
(Refurb, "152") => (RuleGroup::Preview, rules::refurb::rules::MathConstant),
(Refurb, "168") => (RuleGroup::Preview, rules::refurb::rules::IsinstanceTypeNone), (Refurb, "168") => (RuleGroup::Preview, rules::refurb::rules::IsinstanceTypeNone),
(Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison), (Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison),
(Refurb, "171") => (RuleGroup::Preview, rules::refurb::rules::SingleItemMembershipTest), (Refurb, "171") => (RuleGroup::Preview, rules::refurb::rules::SingleItemMembershipTest),

View file

@ -22,6 +22,7 @@ mod tests {
#[test_case(Rule::ReimplementedStarmap, Path::new("FURB140.py"))] #[test_case(Rule::ReimplementedStarmap, Path::new("FURB140.py"))]
#[test_case(Rule::SliceCopy, Path::new("FURB145.py"))] #[test_case(Rule::SliceCopy, Path::new("FURB145.py"))]
#[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))] #[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))]
#[test_case(Rule::MathConstant, Path::new("FURB152.py"))]
#[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))] #[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]
#[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))] #[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))]
#[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))] #[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))]

View file

@ -0,0 +1,90 @@
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, Number};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
/// ## What it does
/// Checks for literals that are similar to constants in `math` module.
///
/// ## Why is this bad?
/// Hard-coding mathematical constants like π increases code duplication,
/// reduces readability, and may lead to a lack of precision.
///
/// ## Example
/// ```python
/// A = 3.141592 * r**2
/// ```
///
/// Use instead:
/// ```python
/// A = math.pi * r**2
/// ```
///
/// ## References
/// - [Python documentation: `math` constants](https://docs.python.org/3/library/math.html#constants)
#[violation]
pub struct MathConstant {
literal: String,
constant: &'static str,
}
impl Violation for MathConstant {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
let MathConstant { literal, constant } = self;
format!("Replace `{literal}` with `math.{constant}`")
}
fn fix_title(&self) -> Option<String> {
let MathConstant { constant, .. } = self;
Some(format!("Use `math.{constant}`"))
}
}
/// FURB152
pub(crate) fn math_constant(checker: &mut Checker, literal: &ast::ExprNumberLiteral) {
let Number::Float(value) = literal.value else {
return;
};
for (real_value, constant) in [
(std::f64::consts::PI, "pi"),
(std::f64::consts::E, "e"),
(std::f64::consts::TAU, "tau"),
] {
if (value - real_value).abs() < 1e-2 {
let mut diagnostic = Diagnostic::new(
MathConstant {
literal: checker.locator().slice(literal).into(),
constant,
},
literal.range(),
);
diagnostic.try_set_fix(|| convert_to_constant(literal, constant, checker));
checker.diagnostics.push(diagnostic);
return;
}
}
}
fn convert_to_constant(
literal: &ast::ExprNumberLiteral,
constant: &'static str,
checker: &Checker,
) -> Result<Fix> {
let (edit, binding) = checker.importer().get_or_import_symbol(
&ImportRequest::import("math", constant),
literal.start(),
checker.semantic(),
)?;
Ok(Fix::safe_edits(
Edit::range_replacement(binding, literal.range()),
[edit],
))
}

View file

@ -3,6 +3,7 @@ pub(crate) use delete_full_slice::*;
pub(crate) use if_expr_min_max::*; pub(crate) use if_expr_min_max::*;
pub(crate) use implicit_cwd::*; pub(crate) use implicit_cwd::*;
pub(crate) use isinstance_type_none::*; pub(crate) use isinstance_type_none::*;
pub(crate) use math_constant::*;
pub(crate) use print_empty_string::*; pub(crate) use print_empty_string::*;
pub(crate) use read_whole_file::*; pub(crate) use read_whole_file::*;
pub(crate) use reimplemented_starmap::*; pub(crate) use reimplemented_starmap::*;
@ -17,6 +18,7 @@ mod delete_full_slice;
mod if_expr_min_max; mod if_expr_min_max;
mod implicit_cwd; mod implicit_cwd;
mod isinstance_type_none; mod isinstance_type_none;
mod math_constant;
mod print_empty_string; mod print_empty_string;
mod read_whole_file; mod read_whole_file;
mod reimplemented_starmap; mod reimplemented_starmap;

View file

@ -0,0 +1,67 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
---
FURB152.py:3:5: FURB152 [*] Replace `3.14` with `math.pi`
|
1 | r = 3.1 # OK
2 |
3 | A = 3.14 * r ** 2 # FURB152
| ^^^^ FURB152
4 |
5 | C = 6.28 * r # FURB152
|
= help: Use `math.pi`
Safe fix
1 |+import math
1 2 | r = 3.1 # OK
2 3 |
3 |-A = 3.14 * r ** 2 # FURB152
4 |+A = math.pi * r ** 2 # FURB152
4 5 |
5 6 | C = 6.28 * r # FURB152
6 7 |
FURB152.py:5:5: FURB152 [*] Replace `6.28` with `math.tau`
|
3 | A = 3.14 * r ** 2 # FURB152
4 |
5 | C = 6.28 * r # FURB152
| ^^^^ FURB152
6 |
7 | e = 2.71 # FURB152
|
= help: Use `math.tau`
Safe fix
1 |+import math
1 2 | r = 3.1 # OK
2 3 |
3 4 | A = 3.14 * r ** 2 # FURB152
4 5 |
5 |-C = 6.28 * r # FURB152
6 |+C = math.tau * r # FURB152
6 7 |
7 8 | e = 2.71 # FURB152
FURB152.py:7:5: FURB152 [*] Replace `2.71` with `math.e`
|
5 | C = 6.28 * r # FURB152
6 |
7 | e = 2.71 # FURB152
| ^^^^ FURB152
|
= help: Use `math.e`
Safe fix
1 |+import math
1 2 | r = 3.1 # OK
2 3 |
3 4 | A = 3.14 * r ** 2 # FURB152
4 5 |
5 6 | C = 6.28 * r # FURB152
6 7 |
7 |-e = 2.71 # FURB152
8 |+e = math.e # FURB152

View file

@ -1159,6 +1159,7 @@ mod tests {
Rule::TooManyPublicMethods, Rule::TooManyPublicMethods,
Rule::UndocumentedWarn, Rule::UndocumentedWarn,
Rule::UnnecessaryEnumerate, Rule::UnnecessaryEnumerate,
Rule::MathConstant,
]; ];
#[allow(clippy::needless_pass_by_value)] #[allow(clippy::needless_pass_by_value)]

2
ruff.schema.json generated
View file

@ -2828,6 +2828,8 @@
"FURB140", "FURB140",
"FURB145", "FURB145",
"FURB148", "FURB148",
"FURB15",
"FURB152",
"FURB16", "FURB16",
"FURB168", "FURB168",
"FURB169", "FURB169",