From f7520795160af948d65c033e6d36c9c1076e4e0a Mon Sep 17 00:00:00 2001 From: DeviousCardi <115358213+DeviousCardi@users.noreply.github.com> Date: Tue, 8 Sep 2026 07:46:20 +0530 Subject: [PATCH 1/3] [`refurb`] Only mark `math-constant` fix safe for exact literals (FURB152) `math-constant` matches any float literal that rounds to `pi`, `e` or `tau` at 2 to 15 decimal places, so `3.14` and `2.718` are reported alongside full precision spellings. The fix was always marked safe, which meant `ruff check --fix` silently rewrote those shorter literals to `math.pi` and `math.e` and changed the values the program computes with. Measurements, thresholds and test fixtures that happen to start with the same digits were altered without review. Mark the fix safe only when the literal already denotes the same float as the constant, so substituting it cannot change a result, and unsafe otherwise. Move the rule from the `correctness` category to `suspicious`, since matching a constant's leading digits does not establish that the constant was meant. --- .../resources/test/fixtures/refurb/FURB152.py | 10 +++ .../src/rules/refurb/rules/math_constant.rs | 54 +++++++++++-- ...furb__tests__math-constant_FURB152.py.snap | 77 +++++++++++++++++++ 3 files changed, 133 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py index 598d811d75e7c..2f9e0f479c9ae 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py @@ -43,3 +43,13 @@ e = 2.718200000000001 # OK e = 2.7182000000000001 # FURB152 + +# Each of these already denotes exactly the same float as the constant it matches, +# so substituting the constant cannot change a result and the fix is safe. Every +# shorter approximation above computes a different value once rewritten, so those +# fixes are offered as unsafe. +r = 3.141592653589793 # FURB152 + +e = 2.718281828459045 # FURB152 + +t = 6.283185307179586 # FURB152 diff --git a/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs b/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs index 35b8478825df4..2f39f24b95d5b 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs @@ -26,10 +26,24 @@ use crate::{Edit, Fix, FixAvailability, Violation}; /// A = math.pi * r**2 /// ``` /// +/// ## Known problems +/// A literal is matched on its digits alone, so a value that merely happens to +/// begin with the same digits as a mathematical constant is flagged even when it +/// means something else entirely. A measurement that rounds to `3.14`, or a price +/// of `2.718`, is indistinguishable here from an approximation of π or e. +/// +/// ## Fix safety +/// This rule's fix is marked as safe only when the literal already denotes exactly +/// the same float as the constant, so that replacing it cannot change a result. +/// Shorter approximations are rewritten under an unsafe fix, because `math.pi` is +/// not equal to `3.14`: a literal that was deliberately chosen — a rounded +/// measurement, a threshold, a test fixture — computes a different value once it +/// becomes a constant. +/// /// ## References /// - [Python documentation: `math` constants](https://docs.python.org/3/library/math.html#constants) #[derive(ViolationMetadata)] -#[violation_metadata(preview_since = "v0.1.6", category = Category::Correctness)] +#[violation_metadata(preview_since = "v0.1.6", category = Category::Suspicious)] pub(crate) struct MathConstant { literal: String, constant: &'static str, @@ -64,24 +78,39 @@ pub(crate) fn math_constant(checker: &Checker, literal: &ast::ExprNumberLiteral) }, literal.range(), ); - diagnostic.try_set_fix(|| convert_to_constant(literal, constant.name(), checker)); + diagnostic.try_set_fix(|| convert_to_constant(literal, value, constant, checker)); } } fn convert_to_constant( literal: &ast::ExprNumberLiteral, - constant: &'static str, + value: f64, + constant: Constant, checker: &Checker, ) -> Result { let (edit, binding) = checker.importer().get_or_import_symbol( - &ImportRequest::import("math", constant), + &ImportRequest::import("math", constant.name()), literal.start(), checker.semantic(), )?; - Ok(Fix::safe_edits( - Edit::range_replacement(binding, literal.range()), - [edit], - )) + let replacement = Edit::range_replacement(binding, literal.range()); + + // The rule matches any literal that rounds to the constant, so `3.14` is reported + // just as `3.141592653589793` is. Substituting the constant only leaves the + // program computing the same values in the latter case; in the former it silently + // replaces the author's number with a different one. + #[expect( + clippy::float_cmp, + reason = "an exact comparison is the point: only a literal that is already the \ + same float can be replaced without changing what the program computes" + )] + let is_exact = value == constant.value(); + + if is_exact { + Ok(Fix::safe_edits(replacement, [edit])) + } else { + Ok(Fix::unsafe_edits(replacement, [edit])) + } } fn matches_constant(constant: f64, value: f64) -> bool { @@ -126,4 +155,13 @@ impl Constant { Constant::Tau => "tau", } } + + /// The value `math.` evaluates to, for comparing against a matched literal. + fn value(self) -> f64 { + match self { + Constant::Pi => std::f64::consts::PI, + Constant::E => std::f64::consts::E, + Constant::Tau => std::f64::consts::TAU, + } + } } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__math-constant_FURB152.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__math-constant_FURB152.py.snap index e8285f3b8929c..931b1a36ce383 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__math-constant_FURB152.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__math-constant_FURB152.py.snap @@ -20,6 +20,7 @@ help: Use `math.pi` 4 + A = math.pi * r ** 2 # FURB152 5 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `6.28` with `math.tau` --> FURB152.py:5:5 @@ -42,6 +43,7 @@ help: Use `math.tau` 6 + C = math.tau * r # FURB152 7 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `2.71` with `math.e` --> FURB152.py:7:5 @@ -63,6 +65,7 @@ help: Use `math.e` 8 + e = math.e # FURB152 9 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.141` with `math.pi` --> FURB152.py:11:5 @@ -84,6 +87,7 @@ help: Use `math.pi` 12 + r = math.pi # FURB152 13 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.142` with `math.pi` --> FURB152.py:13:5 @@ -105,6 +109,7 @@ help: Use `math.pi` 14 + r = math.pi # FURB152 15 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.1415` with `math.pi` --> FURB152.py:15:5 @@ -126,6 +131,7 @@ help: Use `math.pi` 16 + r = math.pi # FURB152 17 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.1416` with `math.pi` --> FURB152.py:17:5 @@ -147,6 +153,7 @@ help: Use `math.pi` 18 + r = math.pi # FURB152 19 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.141592` with `math.pi` --> FURB152.py:19:5 @@ -168,6 +175,7 @@ help: Use `math.pi` 20 + r = math.pi # FURB152 21 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.141593` with `math.pi` --> FURB152.py:21:5 @@ -189,6 +197,7 @@ help: Use `math.pi` 22 + r = math.pi # FURB152 23 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.14159265` with `math.pi` --> FURB152.py:23:5 @@ -210,6 +219,7 @@ help: Use `math.pi` 24 + r = math.pi # FURB152 25 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.141592653589793238462643383279` with `math.pi` --> FURB152.py:25:5 @@ -252,6 +262,7 @@ help: Use `math.e` 32 + e = math.e # FURB152 33 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `2.7182` with `math.e` --> FURB152.py:33:5 @@ -273,6 +284,7 @@ help: Use `math.e` 34 + e = math.e # FURB152 35 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `2.7183` with `math.e` --> FURB152.py:35:5 @@ -294,6 +306,7 @@ help: Use `math.e` 36 + e = math.e # FURB152 37 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `2.7182000000000001` with `math.e` --> FURB152.py:45:5 @@ -302,6 +315,9 @@ FURB152 [*] Replace `2.7182000000000001` with `math.e` 44 | 45 | e = 2.7182000000000001 # FURB152 | ^^^^^^^^^^^^^^^^^^ +46 | +47 | # Each of these already denotes exactly the same float as the constant it matches, + | help: Use `math.e` | 1 + import math @@ -310,4 +326,65 @@ help: Use `math.e` 45 | - e = 2.7182000000000001 # FURB152 46 + e = math.e # FURB152 +47 | + | +note: This is an unsafe fix and may change runtime behavior + +FURB152 [*] Replace `3.141592653589793` with `math.pi` + --> FURB152.py:51:5 + | +49 | # shorter approximation above computes a different value once rewritten, so those +50 | # fixes are offered as unsafe. +51 | r = 3.141592653589793 # FURB152 + | ^^^^^^^^^^^^^^^^^ +52 | +53 | e = 2.718281828459045 # FURB152 + | +help: Use `math.pi` + | +1 + import math +2 | r = 3.1 # OK +-------------------------------------------------------------------------------- +51 | # fixes are offered as unsafe. + - r = 3.141592653589793 # FURB152 +52 + r = math.pi # FURB152 +53 | + | + +FURB152 [*] Replace `2.718281828459045` with `math.e` + --> FURB152.py:53:5 + | +51 | r = 3.141592653589793 # FURB152 +52 | +53 | e = 2.718281828459045 # FURB152 + | ^^^^^^^^^^^^^^^^^ +54 | +55 | t = 6.283185307179586 # FURB152 + | +help: Use `math.e` + | +1 + import math +2 | r = 3.1 # OK +-------------------------------------------------------------------------------- +53 | + - e = 2.718281828459045 # FURB152 +54 + e = math.e # FURB152 +55 | + | + +FURB152 [*] Replace `6.283185307179586` with `math.tau` + --> FURB152.py:55:5 + | +53 | e = 2.718281828459045 # FURB152 +54 | +55 | t = 6.283185307179586 # FURB152 + | ^^^^^^^^^^^^^^^^^ +help: Use `math.tau` + | +1 + import math +2 | r = 3.1 # OK +-------------------------------------------------------------------------------- +55 | + - t = 6.283185307179586 # FURB152 +56 + t = math.tau # FURB152 | From 36d4a80807cd4f037f908507bf931ed953ee2c41 Mon Sep 17 00:00:00 2001 From: DeviousCardi <115358213+DeviousCardi@users.noreply.github.com> Date: Tue, 6 Oct 2026 08:58:54 +0530 Subject: [PATCH 2/3] [`refurb`] Always mark `math-constant` fix unsafe (FURB152) Per review: drop the exact-literal special case and shorten the fix safety section. --- .../resources/test/fixtures/refurb/FURB152.py | 5 +- .../src/rules/refurb/rules/math_constant.rs | 46 ++++------------ ...furb__tests__math-constant_FURB152.py.snap | 53 ++++++++++--------- 3 files changed, 38 insertions(+), 66 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py index 2f9e0f479c9ae..adbc62114287d 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB152.py @@ -44,10 +44,7 @@ e = 2.7182000000000001 # FURB152 -# Each of these already denotes exactly the same float as the constant it matches, -# so substituting the constant cannot change a result and the fix is safe. Every -# shorter approximation above computes a different value once rewritten, so those -# fixes are offered as unsafe. +# Full-precision literals are still reported, with the same unsafe fix. r = 3.141592653589793 # FURB152 e = 2.718281828459045 # FURB152 diff --git a/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs b/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs index 2f39f24b95d5b..6278035db1c02 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs @@ -33,12 +33,8 @@ use crate::{Edit, Fix, FixAvailability, Violation}; /// of `2.718`, is indistinguishable here from an approximation of π or e. /// /// ## Fix safety -/// This rule's fix is marked as safe only when the literal already denotes exactly -/// the same float as the constant, so that replacing it cannot change a result. -/// Shorter approximations are rewritten under an unsafe fix, because `math.pi` is -/// not equal to `3.14`: a literal that was deliberately chosen — a rounded -/// measurement, a threshold, a test fixture — computes a different value once it -/// becomes a constant. +/// This rule's fix is marked as unsafe, as replacing a literal with the constant +/// can change the result of a calculation. /// /// ## References /// - [Python documentation: `math` constants](https://docs.python.org/3/library/math.html#constants) @@ -78,39 +74,24 @@ pub(crate) fn math_constant(checker: &Checker, literal: &ast::ExprNumberLiteral) }, literal.range(), ); - diagnostic.try_set_fix(|| convert_to_constant(literal, value, constant, checker)); + diagnostic.try_set_fix(|| convert_to_constant(literal, constant.name(), checker)); } } fn convert_to_constant( literal: &ast::ExprNumberLiteral, - value: f64, - constant: Constant, + constant: &'static str, checker: &Checker, ) -> Result { let (edit, binding) = checker.importer().get_or_import_symbol( - &ImportRequest::import("math", constant.name()), + &ImportRequest::import("math", constant), literal.start(), checker.semantic(), )?; - let replacement = Edit::range_replacement(binding, literal.range()); - - // The rule matches any literal that rounds to the constant, so `3.14` is reported - // just as `3.141592653589793` is. Substituting the constant only leaves the - // program computing the same values in the latter case; in the former it silently - // replaces the author's number with a different one. - #[expect( - clippy::float_cmp, - reason = "an exact comparison is the point: only a literal that is already the \ - same float can be replaced without changing what the program computes" - )] - let is_exact = value == constant.value(); - - if is_exact { - Ok(Fix::safe_edits(replacement, [edit])) - } else { - Ok(Fix::unsafe_edits(replacement, [edit])) - } + Ok(Fix::unsafe_edits( + Edit::range_replacement(binding, literal.range()), + [edit], + )) } fn matches_constant(constant: f64, value: f64) -> bool { @@ -155,13 +136,4 @@ impl Constant { Constant::Tau => "tau", } } - - /// The value `math.` evaluates to, for comparing against a matched literal. - fn value(self) -> f64 { - match self { - Constant::Pi => std::f64::consts::PI, - Constant::E => std::f64::consts::E, - Constant::Tau => std::f64::consts::TAU, - } - } } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__math-constant_FURB152.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__math-constant_FURB152.py.snap index 931b1a36ce383..2506daa1308f3 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__math-constant_FURB152.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__math-constant_FURB152.py.snap @@ -241,6 +241,7 @@ help: Use `math.pi` 26 + r = math.pi # FURB152 27 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `2.718` with `math.e` --> FURB152.py:31:5 @@ -316,7 +317,7 @@ FURB152 [*] Replace `2.7182000000000001` with `math.e` 45 | e = 2.7182000000000001 # FURB152 | ^^^^^^^^^^^^^^^^^^ 46 | -47 | # Each of these already denotes exactly the same float as the constant it matches, +47 | # Full-precision literals are still reported, with the same unsafe fix. | help: Use `math.e` | @@ -331,60 +332,62 @@ help: Use `math.e` note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `3.141592653589793` with `math.pi` - --> FURB152.py:51:5 + --> FURB152.py:48:5 | -49 | # shorter approximation above computes a different value once rewritten, so those -50 | # fixes are offered as unsafe. -51 | r = 3.141592653589793 # FURB152 +47 | # Full-precision literals are still reported, with the same unsafe fix. +48 | r = 3.141592653589793 # FURB152 | ^^^^^^^^^^^^^^^^^ -52 | -53 | e = 2.718281828459045 # FURB152 +49 | +50 | e = 2.718281828459045 # FURB152 | help: Use `math.pi` | 1 + import math 2 | r = 3.1 # OK -------------------------------------------------------------------------------- -51 | # fixes are offered as unsafe. +48 | # Full-precision literals are still reported, with the same unsafe fix. - r = 3.141592653589793 # FURB152 -52 + r = math.pi # FURB152 -53 | +49 + r = math.pi # FURB152 +50 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `2.718281828459045` with `math.e` - --> FURB152.py:53:5 + --> FURB152.py:50:5 | -51 | r = 3.141592653589793 # FURB152 -52 | -53 | e = 2.718281828459045 # FURB152 +48 | r = 3.141592653589793 # FURB152 +49 | +50 | e = 2.718281828459045 # FURB152 | ^^^^^^^^^^^^^^^^^ -54 | -55 | t = 6.283185307179586 # FURB152 +51 | +52 | t = 6.283185307179586 # FURB152 | help: Use `math.e` | 1 + import math 2 | r = 3.1 # OK -------------------------------------------------------------------------------- -53 | +50 | - e = 2.718281828459045 # FURB152 -54 + e = math.e # FURB152 -55 | +51 + e = math.e # FURB152 +52 | | +note: This is an unsafe fix and may change runtime behavior FURB152 [*] Replace `6.283185307179586` with `math.tau` - --> FURB152.py:55:5 + --> FURB152.py:52:5 | -53 | e = 2.718281828459045 # FURB152 -54 | -55 | t = 6.283185307179586 # FURB152 +50 | e = 2.718281828459045 # FURB152 +51 | +52 | t = 6.283185307179586 # FURB152 | ^^^^^^^^^^^^^^^^^ help: Use `math.tau` | 1 + import math 2 | r = 3.1 # OK -------------------------------------------------------------------------------- -55 | +52 | - t = 6.283185307179586 # FURB152 -56 + t = math.tau # FURB152 +53 + t = math.tau # FURB152 | +note: This is an unsafe fix and may change runtime behavior From 1a9b9406e9d8dd8c862cccc72b6ba34a62e53d2f Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:34:09 -0400 Subject: [PATCH 3/3] drop known problems --- crates/ruff_linter/src/rules/refurb/rules/math_constant.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs b/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs index 6278035db1c02..78d99fc0242ba 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/math_constant.rs @@ -26,12 +26,6 @@ use crate::{Edit, Fix, FixAvailability, Violation}; /// A = math.pi * r**2 /// ``` /// -/// ## Known problems -/// A literal is matched on its digits alone, so a value that merely happens to -/// begin with the same digits as a mathematical constant is flagged even when it -/// means something else entirely. A measurement that rounds to `3.14`, or a price -/// of `2.718`, is indistinguishable here from an approximation of π or e. -/// /// ## Fix safety /// This rule's fix is marked as unsafe, as replacing a literal with the constant /// can change the result of a calculation.