Skip to content

Commit

Permalink
Make math-constant rule more targeted (#9054)
Browse files Browse the repository at this point in the history
## Summary

We now only flag `math.pi` if the value is in `[3.14, 3.15)`, and apply
similar rules to the other constants.

Closes #9049.
  • Loading branch information
charliermarsh authored Dec 8, 2023
1 parent d0d88d9 commit e043bd4
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 17 deletions.
8 changes: 8 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,11 @@
C = 6.28 * r # FURB152

e = 2.71 # FURB152

r = 3.15 # OK

r = 3.141 # FURB152

r = 3.1415 # FURB152

e = 2.7 # OK
58 changes: 41 additions & 17 deletions crates/ruff_linter/src/rules/refurb/rules/math_constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,17 @@ pub(crate) fn math_constant(checker: &mut Checker, literal: &ast::ExprNumberLite
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;
}

if let Some(constant) = Constant::from_value(value) {
let mut diagnostic = Diagnostic::new(
MathConstant {
literal: checker.locator().slice(literal).into(),
constant: constant.name(),
},
literal.range(),
);
diagnostic.try_set_fix(|| convert_to_constant(literal, constant.name(), checker));
checker.diagnostics.push(diagnostic);
}
}

Expand All @@ -88,3 +82,33 @@ fn convert_to_constant(
[edit],
))
}

#[derive(Debug, Clone, Copy)]
enum Constant {
Pi,
E,
Tau,
}

impl Constant {
#[allow(clippy::approx_constant)]
fn from_value(value: f64) -> Option<Self> {
if (3.14..3.15).contains(&value) {
Some(Self::Pi)
} else if (2.71..2.72).contains(&value) {
Some(Self::E)
} else if (6.28..6.29).contains(&value) {
Some(Self::Tau)
} else {
None
}
}

fn name(self) -> &'static str {
match self {
Constant::Pi => "pi",
Constant::E => "e",
Constant::Tau => "tau",
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,16 @@ FURB152.py:5:5: FURB152 [*] Replace `6.28` with `math.tau`
6 |+C = math.tau * r # FURB152
6 7 |
7 8 | e = 2.71 # FURB152
8 9 |

FURB152.py:7:5: FURB152 [*] Replace `2.71` with `math.e`
|
5 | C = 6.28 * r # FURB152
6 |
7 | e = 2.71 # FURB152
| ^^^^ FURB152
8 |
9 | r = 3.15 # OK
|
= help: Use `math.e`

Expand All @@ -63,5 +66,59 @@ FURB152.py:7:5: FURB152 [*] Replace `2.71` with `math.e`
6 7 |
7 |-e = 2.71 # FURB152
8 |+e = math.e # FURB152
8 9 |
9 10 | r = 3.15 # OK
10 11 |

FURB152.py:11:5: FURB152 [*] Replace `3.141` with `math.pi`
|
9 | r = 3.15 # OK
10 |
11 | r = 3.141 # FURB152
| ^^^^^ FURB152
12 |
13 | r = 3.1415 # FURB152
|
= help: Use `math.pi`

Safe fix
1 |+import math
1 2 | r = 3.1 # OK
2 3 |
3 4 | A = 3.14 * r ** 2 # FURB152
--------------------------------------------------------------------------------
8 9 |
9 10 | r = 3.15 # OK
10 11 |
11 |-r = 3.141 # FURB152
12 |+r = math.pi # FURB152
12 13 |
13 14 | r = 3.1415 # FURB152
14 15 |

FURB152.py:13:5: FURB152 [*] Replace `3.1415` with `math.pi`
|
11 | r = 3.141 # FURB152
12 |
13 | r = 3.1415 # FURB152
| ^^^^^^ FURB152
14 |
15 | e = 2.7 # OK
|
= help: Use `math.pi`

Safe fix
1 |+import math
1 2 | r = 3.1 # OK
2 3 |
3 4 | A = 3.14 * r ** 2 # FURB152
--------------------------------------------------------------------------------
10 11 |
11 12 | r = 3.141 # FURB152
12 13 |
13 |-r = 3.1415 # FURB152
14 |+r = math.pi # FURB152
14 15 |
15 16 | e = 2.7 # OK


0 comments on commit e043bd4

Please sign in to comment.