Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.

Loading