Skip to content

Commit

Permalink
[refurb] Implement math-constant (FURB152) (#8727)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
siiptuo committed Nov 17, 2023
1 parent b7dbb90 commit 2faac1e
Show file tree
Hide file tree
Showing 9 changed files with 175 additions and 1 deletion.
7 changes: 7 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
r = 3.1 # OK

A = 3.14 * r ** 2 # FURB152

C = 6.28 * r # FURB152

e = 2.71 # FURB152
5 changes: 4 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1245,10 +1245,13 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
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) {
flake8_pyi::rules::numeric_literal_too_long(checker, expr);
}
if checker.enabled(Rule::MathConstant) {
refurb::rules::math_constant(checker, number_literal);
}
}
Expr::BytesLiteral(_) => {
if checker.source_type.is_stub() && checker.enabled(Rule::StringOrBytesTooLong) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
(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, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison),
(Refurb, "171") => (RuleGroup::Preview, rules::refurb::rules::SingleItemMembershipTest),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ mod tests {
#[test_case(Rule::ReimplementedStarmap, Path::new("FURB140.py"))]
#[test_case(Rule::SliceCopy, Path::new("FURB145.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::ImplicitCwd, Path::new("FURB177.py"))]
#[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))]
Expand Down
90 changes: 90 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/math_constant.rs
Original file line number Diff line number Diff line change
@@ -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],
))
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pub(crate) use delete_full_slice::*;
pub(crate) use if_expr_min_max::*;
pub(crate) use implicit_cwd::*;
pub(crate) use isinstance_type_none::*;
pub(crate) use math_constant::*;
pub(crate) use print_empty_string::*;
pub(crate) use read_whole_file::*;
pub(crate) use reimplemented_starmap::*;
Expand All @@ -17,6 +18,7 @@ mod delete_full_slice;
mod if_expr_min_max;
mod implicit_cwd;
mod isinstance_type_none;
mod math_constant;
mod print_empty_string;
mod read_whole_file;
mod reimplemented_starmap;
Expand Down
Original file line number Diff line number Diff line change
@@ -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


1 change: 1 addition & 0 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1159,6 +1159,7 @@ mod tests {
Rule::TooManyPublicMethods,
Rule::UndocumentedWarn,
Rule::UnnecessaryEnumerate,
Rule::MathConstant,
];

#[allow(clippy::needless_pass_by_value)]
Expand Down
2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 2faac1e

Please sign in to comment.