From ad933c26600532c3e0676183310666bc54c835ee Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Mon, 15 Jul 2024 08:22:27 -0400 Subject: [PATCH 01/19] Define new rule --- .../rules/ruff/rules/float_literal_decimal.rs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs diff --git a/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs b/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs new file mode 100644 index 0000000000000..7637f3c4dcd1e --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs @@ -0,0 +1,76 @@ +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast}; +use ruff_text_size::{Ranged, TextRange}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for `Decimal` calls passing a float literal. +/// +/// ## Why is this bad? +/// Float literals are non-deterministic and can lead to unexpected results. The `Decimal` class is designed to handle +/// numbers with fixed-point precision, so a string literal should be used instead. +/// +/// ## Example +/// +/// ```python +/// num = Decimal(1.2345) +/// ``` +/// +/// Use instead: +/// ```python +/// num = Decimal("1.2345") +/// ``` +#[violation] +pub struct FloatLiteralDecimal; + +impl AlwaysFixableViolation for FloatLiteralDecimal { + #[derive_message_formats] + fn message(&self) -> String { + format!(r#"`Decimal()` called with float literal argument"#) + } + + fn fix_title(&self) -> String { + "Use a string literal instead".into() + } +} + +/// RUF032: Decimal() called with float literal argument +pub(crate) fn float_literal_decimal_syntax(checker: &mut Checker, call: &ast::ExprCall) { + if !checker + .semantic() + .resolve_qualified_name(call.func.as_ref()) + .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["decimal", "Decimal"])) + { + return; + } + if let Some(arg) = call.arguments.args.get(0) { + if let ast::Expr::NumberLiteral(ast::ExprNumberLiteral { + value: ast::Number::Float(_), + .. + }) = arg + { + let diagnostic = Diagnostic::new(FloatLiteralDecimal, arg.range()).with_fix( + fix_float_literal(arg.range(), checker.generator().expr(arg)), + ); + checker.diagnostics.push(diagnostic); + } else if let ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) = arg { + if let ast::Expr::NumberLiteral(ast::ExprNumberLiteral { + value: ast::Number::Float(_), + .. + }) = operand.as_ref() + { + let diagnostic = Diagnostic::new(FloatLiteralDecimal, arg.range()).with_fix( + fix_float_literal(arg.range(), checker.generator().expr(arg)), + ); + checker.diagnostics.push(diagnostic); + } + } + } +} + +fn fix_float_literal(range: TextRange, float_literal: String) -> Fix { + let content = format!("\"{}\"", float_literal); + Fix::unsafe_edit(Edit::range_replacement(content, range)) +} From 65a7387bf12601470c9d90b9915dc994729b2970 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Mon, 15 Jul 2024 12:33:17 +0000 Subject: [PATCH 02/19] Add float_literal_decimal to ruff module --- crates/ruff_linter/src/rules/ruff/rules/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 83f351520143d..6f6994dd20a14 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -5,6 +5,7 @@ pub(crate) use asyncio_dangling_task::*; pub(crate) use collection_literal_concatenation::*; pub(crate) use default_factory_kwarg::*; pub(crate) use explicit_f_string_type_conversion::*; +pub(crate) use float_literal_decimal::*; pub(crate) use function_call_in_dataclass_default::*; pub(crate) use implicit_optional::*; pub(crate) use incorrectly_parenthesized_tuple_in_subscript::*; @@ -38,6 +39,7 @@ mod collection_literal_concatenation; mod confusables; mod default_factory_kwarg; mod explicit_f_string_type_conversion; +mod float_literal_decimal; mod function_call_in_dataclass_default; mod helpers; mod implicit_optional; From ea12aaf4476706829d6aa36695215c5d43469ad5 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Mon, 15 Jul 2024 12:41:12 +0000 Subject: [PATCH 03/19] Add error codes --- crates/ruff_linter/src/codes.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 9aad7a11f260b..b1b46c6c5182b 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -958,6 +958,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "029") => (RuleGroup::Preview, rules::ruff::rules::UnusedAsync), (Ruff, "030") => (RuleGroup::Preview, rules::ruff::rules::AssertWithPrintMessage), (Ruff, "031") => (RuleGroup::Preview, rules::ruff::rules::IncorrectlyParenthesizedTupleInSubscript), + (Ruff, "032") => (RuleGroup::Preview, rules::ruff::rules::FloatLiteralDecimal), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), From 52b61fa90a097ec016cfbd7b1311d8701d4451a6 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Thu, 15 Aug 2024 14:32:03 -0400 Subject: [PATCH 04/19] Call new lint rule --- crates/ruff_linter/src/checkers/ast/analyze/expression.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index a47f94723efed..7d6f09f89a583 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1011,6 +1011,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::UnnecessaryIterableAllocationForFirstElement) { ruff::rules::unnecessary_iterable_allocation_for_first_element(checker, expr); } + if checker.enabled(Rule::FloatLiteralDecimal) { + ruff::rules::float_literal_decimal_syntax(checker, call); + } if checker.enabled(Rule::IntOnSlicedStr) { refurb::rules::int_on_sliced_str(checker, call); } From 44d86e2bda98a670b6abcf74b3ea639546baaab6 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Thu, 15 Aug 2024 10:48:40 -0400 Subject: [PATCH 05/19] Add test fixture for rule --- .../resources/test/fixtures/ruff/RUF032.py | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF032.py diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF032.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF032.py new file mode 100644 index 0000000000000..4b2146aace2bf --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF032.py @@ -0,0 +1,120 @@ +import decimal + +# Tests with fully qualified import +decimal.Decimal(0) + +decimal.Decimal(0.0) # Should error + +decimal.Decimal("0.0") + +decimal.Decimal(10) + +decimal.Decimal(10.0) # Should error + +decimal.Decimal("10.0") + +decimal.Decimal(-10) + +decimal.Decimal(-10.0) # Should error + +decimal.Decimal("-10.0") + +a = 10.0 + +decimal.Decimal(a) + + +# Tests with relative import +from decimal import Decimal + + +val = Decimal(0) + +val = Decimal(0.0) # Should error + +val = Decimal("0.0") + +val = Decimal(10) + +val = Decimal(10.0) # Should error + +val = Decimal("10.0") + +val = Decimal(-10) + +val = Decimal(-10.0) # Should error + +val = Decimal("-10.0") + +a = 10.0 + +val = Decimal(a) + + +# Tests with shadowed name +class Decimal(): + value: float | int | str + + def __init__(self, value: float | int | str) -> None: + self.value = value + + +val = Decimal(0.0) + +val = Decimal("0.0") + +val = Decimal(10.0) + +val = Decimal("10.0") + +val = Decimal(-10.0) + +val = Decimal("-10.0") + +a = 10.0 + +val = Decimal(a) + + +# Retest with fully qualified import + +val = decimal.Decimal(0.0) # Should error + +val = decimal.Decimal("0.0") + +val = decimal.Decimal(10.0) # Should error + +val = decimal.Decimal("10.0") + +val = decimal.Decimal(-10.0) # Should error + +val = decimal.Decimal("-10.0") + +a = 10.0 + +val = decimal.Decimal(a) + + +class decimal(): + class Decimal(): + value: float | int | str + + def __init__(self, value: float | int | str) -> None: + self.value = value + + +val = decimal.Decimal(0.0) + +val = decimal.Decimal("0.0") + +val = decimal.Decimal(10.0) + +val = decimal.Decimal("10.0") + +val = decimal.Decimal(-10.0) + +val = decimal.Decimal("-10.0") + +a = 10.0 + +val = decimal.Decimal(a) From 276704cc52ba854d2a85b53852938423e09e6cbd Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Thu, 15 Aug 2024 13:22:34 -0400 Subject: [PATCH 06/19] Add test case --- crates/ruff_linter/src/rules/ruff/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 9d63fa3069a52..e531d3d28d0dc 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -57,6 +57,7 @@ mod tests { #[test_case(Rule::UnusedAsync, Path::new("RUF029.py"))] #[test_case(Rule::AssertWithPrintMessage, Path::new("RUF030.py"))] #[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))] + #[test_case(Rule::FloatLiteralDecimal, Path::new("RUF032.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); From f4fe59f8716632d7f4a1f22c97a08e38f020ed1b Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Thu, 15 Aug 2024 13:50:15 -0400 Subject: [PATCH 07/19] Add test snapshot --- ..._rules__ruff__tests__RUF032_RUF032.py.snap | 191 ++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF032_RUF032.py.snap diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF032_RUF032.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF032_RUF032.py.snap new file mode 100644 index 0000000000000..c21499b3f490f --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF032_RUF032.py.snap @@ -0,0 +1,191 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF032.py:6:17: RUF032 [*] `Decimal()` called with float literal argument + | +4 | decimal.Decimal(0) +5 | +6 | decimal.Decimal(0.0) # Should error + | ^^^ RUF032 +7 | +8 | decimal.Decimal("0.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +3 3 | # Tests with fully qualified import +4 4 | decimal.Decimal(0) +5 5 | +6 |-decimal.Decimal(0.0) # Should error + 6 |+decimal.Decimal("0.0") # Should error +7 7 | +8 8 | decimal.Decimal("0.0") +9 9 | + +RUF032.py:12:17: RUF032 [*] `Decimal()` called with float literal argument + | +10 | decimal.Decimal(10) +11 | +12 | decimal.Decimal(10.0) # Should error + | ^^^^ RUF032 +13 | +14 | decimal.Decimal("10.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +9 9 | +10 10 | decimal.Decimal(10) +11 11 | +12 |-decimal.Decimal(10.0) # Should error + 12 |+decimal.Decimal("10.0") # Should error +13 13 | +14 14 | decimal.Decimal("10.0") +15 15 | + +RUF032.py:18:17: RUF032 [*] `Decimal()` called with float literal argument + | +16 | decimal.Decimal(-10) +17 | +18 | decimal.Decimal(-10.0) # Should error + | ^^^^^ RUF032 +19 | +20 | decimal.Decimal("-10.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +15 15 | +16 16 | decimal.Decimal(-10) +17 17 | +18 |-decimal.Decimal(-10.0) # Should error + 18 |+decimal.Decimal("-10.0") # Should error +19 19 | +20 20 | decimal.Decimal("-10.0") +21 21 | + +RUF032.py:33:15: RUF032 [*] `Decimal()` called with float literal argument + | +31 | val = Decimal(0) +32 | +33 | val = Decimal(0.0) # Should error + | ^^^ RUF032 +34 | +35 | val = Decimal("0.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +30 30 | +31 31 | val = Decimal(0) +32 32 | +33 |-val = Decimal(0.0) # Should error + 33 |+val = Decimal("0.0") # Should error +34 34 | +35 35 | val = Decimal("0.0") +36 36 | + +RUF032.py:39:15: RUF032 [*] `Decimal()` called with float literal argument + | +37 | val = Decimal(10) +38 | +39 | val = Decimal(10.0) # Should error + | ^^^^ RUF032 +40 | +41 | val = Decimal("10.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +36 36 | +37 37 | val = Decimal(10) +38 38 | +39 |-val = Decimal(10.0) # Should error + 39 |+val = Decimal("10.0") # Should error +40 40 | +41 41 | val = Decimal("10.0") +42 42 | + +RUF032.py:45:15: RUF032 [*] `Decimal()` called with float literal argument + | +43 | val = Decimal(-10) +44 | +45 | val = Decimal(-10.0) # Should error + | ^^^^^ RUF032 +46 | +47 | val = Decimal("-10.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +42 42 | +43 43 | val = Decimal(-10) +44 44 | +45 |-val = Decimal(-10.0) # Should error + 45 |+val = Decimal("-10.0") # Should error +46 46 | +47 47 | val = Decimal("-10.0") +48 48 | + +RUF032.py:81:23: RUF032 [*] `Decimal()` called with float literal argument + | +79 | # Retest with fully qualified import +80 | +81 | val = decimal.Decimal(0.0) # Should error + | ^^^ RUF032 +82 | +83 | val = decimal.Decimal("0.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +78 78 | +79 79 | # Retest with fully qualified import +80 80 | +81 |-val = decimal.Decimal(0.0) # Should error + 81 |+val = decimal.Decimal("0.0") # Should error +82 82 | +83 83 | val = decimal.Decimal("0.0") +84 84 | + +RUF032.py:85:23: RUF032 [*] `Decimal()` called with float literal argument + | +83 | val = decimal.Decimal("0.0") +84 | +85 | val = decimal.Decimal(10.0) # Should error + | ^^^^ RUF032 +86 | +87 | val = decimal.Decimal("10.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +82 82 | +83 83 | val = decimal.Decimal("0.0") +84 84 | +85 |-val = decimal.Decimal(10.0) # Should error + 85 |+val = decimal.Decimal("10.0") # Should error +86 86 | +87 87 | val = decimal.Decimal("10.0") +88 88 | + +RUF032.py:89:23: RUF032 [*] `Decimal()` called with float literal argument + | +87 | val = decimal.Decimal("10.0") +88 | +89 | val = decimal.Decimal(-10.0) # Should error + | ^^^^^ RUF032 +90 | +91 | val = decimal.Decimal("-10.0") + | + = help: Use a string literal instead + +ℹ Unsafe fix +86 86 | +87 87 | val = decimal.Decimal("10.0") +88 88 | +89 |-val = decimal.Decimal(-10.0) # Should error + 89 |+val = decimal.Decimal("-10.0") # Should error +90 90 | +91 91 | val = decimal.Decimal("-10.0") +92 92 | From 4f29a8f6f1f9299ad3307b9f1f57f0a5b1952bf5 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Thu, 15 Aug 2024 13:35:20 -0400 Subject: [PATCH 08/19] Update schema --- ruff.schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/ruff.schema.json b/ruff.schema.json index c1abd4e001ab5..d5f69b0b13d1f 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3737,6 +3737,7 @@ "RUF03", "RUF030", "RUF031", + "RUF032", "RUF1", "RUF10", "RUF100", From 6dff975ec9f28d4696f2c259af87435143fe7758 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Thu, 15 Aug 2024 15:03:18 -0400 Subject: [PATCH 09/19] Fix clippy errors --- .../src/rules/ruff/rules/float_literal_decimal.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs b/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs index 7637f3c4dcd1e..9b2f92c54706a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs @@ -36,7 +36,7 @@ impl AlwaysFixableViolation for FloatLiteralDecimal { } } -/// RUF032: Decimal() called with float literal argument +/// RUF032: `Decimal()` called with float literal argument pub(crate) fn float_literal_decimal_syntax(checker: &mut Checker, call: &ast::ExprCall) { if !checker .semantic() @@ -45,14 +45,14 @@ pub(crate) fn float_literal_decimal_syntax(checker: &mut Checker, call: &ast::Ex { return; } - if let Some(arg) = call.arguments.args.get(0) { + if let Some(arg) = call.arguments.args.first() { if let ast::Expr::NumberLiteral(ast::ExprNumberLiteral { value: ast::Number::Float(_), .. }) = arg { let diagnostic = Diagnostic::new(FloatLiteralDecimal, arg.range()).with_fix( - fix_float_literal(arg.range(), checker.generator().expr(arg)), + fix_float_literal(arg.range(), &checker.generator().expr(arg)), ); checker.diagnostics.push(diagnostic); } else if let ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) = arg { @@ -62,7 +62,7 @@ pub(crate) fn float_literal_decimal_syntax(checker: &mut Checker, call: &ast::Ex }) = operand.as_ref() { let diagnostic = Diagnostic::new(FloatLiteralDecimal, arg.range()).with_fix( - fix_float_literal(arg.range(), checker.generator().expr(arg)), + fix_float_literal(arg.range(), &checker.generator().expr(arg)), ); checker.diagnostics.push(diagnostic); } @@ -70,7 +70,7 @@ pub(crate) fn float_literal_decimal_syntax(checker: &mut Checker, call: &ast::Ex } } -fn fix_float_literal(range: TextRange, float_literal: String) -> Fix { - let content = format!("\"{}\"", float_literal); +fn fix_float_literal(range: TextRange, float_literal: &str) -> Fix { + let content = format!("\"{float_literal}\""); Fix::unsafe_edit(Edit::range_replacement(content, range)) } From d67c1abcdfbb1171262a417356cb21ef0525a8b6 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Fri, 16 Aug 2024 13:40:52 -0400 Subject: [PATCH 10/19] Rename rule to FloatLiteralDecimal -> DecimalFromFloatLiteral --- .../ruff_linter/src/checkers/ast/analyze/expression.rs | 4 ++-- crates/ruff_linter/src/codes.rs | 2 +- crates/ruff_linter/src/rules/ruff/mod.rs | 2 +- ...iteral_decimal.rs => decimal_from_float_literal.rs} | 10 +++++----- crates/ruff_linter/src/rules/ruff/rules/mod.rs | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) rename crates/ruff_linter/src/rules/ruff/rules/{float_literal_decimal.rs => decimal_from_float_literal.rs} (85%) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 7d6f09f89a583..4f2f46661fb40 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1011,8 +1011,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::UnnecessaryIterableAllocationForFirstElement) { ruff::rules::unnecessary_iterable_allocation_for_first_element(checker, expr); } - if checker.enabled(Rule::FloatLiteralDecimal) { - ruff::rules::float_literal_decimal_syntax(checker, call); + if checker.enabled(Rule::DecimalFromFloatLiteral) { + ruff::rules::decimal_from_float_literal_syntax(checker, call); } if checker.enabled(Rule::IntOnSlicedStr) { refurb::rules::int_on_sliced_str(checker, call); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index b1b46c6c5182b..03ae695bfa82d 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -958,7 +958,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "029") => (RuleGroup::Preview, rules::ruff::rules::UnusedAsync), (Ruff, "030") => (RuleGroup::Preview, rules::ruff::rules::AssertWithPrintMessage), (Ruff, "031") => (RuleGroup::Preview, rules::ruff::rules::IncorrectlyParenthesizedTupleInSubscript), - (Ruff, "032") => (RuleGroup::Preview, rules::ruff::rules::FloatLiteralDecimal), + (Ruff, "032") => (RuleGroup::Preview, rules::ruff::rules::DecimalFromFloatLiteral), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index e531d3d28d0dc..d02c9f7f0e252 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -57,7 +57,7 @@ mod tests { #[test_case(Rule::UnusedAsync, Path::new("RUF029.py"))] #[test_case(Rule::AssertWithPrintMessage, Path::new("RUF030.py"))] #[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))] - #[test_case(Rule::FloatLiteralDecimal, Path::new("RUF032.py"))] + #[test_case(Rule::DecimalFromFloatLiteral, Path::new("RUF032.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs similarity index 85% rename from crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs rename to crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 9b2f92c54706a..2386e6f6ee5f9 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/float_literal_decimal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -23,9 +23,9 @@ use crate::checkers::ast::Checker; /// num = Decimal("1.2345") /// ``` #[violation] -pub struct FloatLiteralDecimal; +pub struct DecimalFromFloatLiteral; -impl AlwaysFixableViolation for FloatLiteralDecimal { +impl AlwaysFixableViolation for DecimalFromFloatLiteral { #[derive_message_formats] fn message(&self) -> String { format!(r#"`Decimal()` called with float literal argument"#) @@ -37,7 +37,7 @@ impl AlwaysFixableViolation for FloatLiteralDecimal { } /// RUF032: `Decimal()` called with float literal argument -pub(crate) fn float_literal_decimal_syntax(checker: &mut Checker, call: &ast::ExprCall) { +pub(crate) fn decimal_from_float_literal_syntax(checker: &mut Checker, call: &ast::ExprCall) { if !checker .semantic() .resolve_qualified_name(call.func.as_ref()) @@ -51,7 +51,7 @@ pub(crate) fn float_literal_decimal_syntax(checker: &mut Checker, call: &ast::Ex .. }) = arg { - let diagnostic = Diagnostic::new(FloatLiteralDecimal, arg.range()).with_fix( + let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( fix_float_literal(arg.range(), &checker.generator().expr(arg)), ); checker.diagnostics.push(diagnostic); @@ -61,7 +61,7 @@ pub(crate) fn float_literal_decimal_syntax(checker: &mut Checker, call: &ast::Ex .. }) = operand.as_ref() { - let diagnostic = Diagnostic::new(FloatLiteralDecimal, arg.range()).with_fix( + let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( fix_float_literal(arg.range(), &checker.generator().expr(arg)), ); checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 6f6994dd20a14..4d34f7cff80eb 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -3,9 +3,9 @@ pub(crate) use assert_with_print_message::*; pub(crate) use assignment_in_assert::*; pub(crate) use asyncio_dangling_task::*; pub(crate) use collection_literal_concatenation::*; +pub(crate) use decimal_from_float_literal::*; pub(crate) use default_factory_kwarg::*; pub(crate) use explicit_f_string_type_conversion::*; -pub(crate) use float_literal_decimal::*; pub(crate) use function_call_in_dataclass_default::*; pub(crate) use implicit_optional::*; pub(crate) use incorrectly_parenthesized_tuple_in_subscript::*; @@ -37,9 +37,9 @@ mod assignment_in_assert; mod asyncio_dangling_task; mod collection_literal_concatenation; mod confusables; +mod decimal_from_float_literal; mod default_factory_kwarg; mod explicit_f_string_type_conversion; -mod float_literal_decimal; mod function_call_in_dataclass_default; mod helpers; mod implicit_optional; From b6da1eb7529033edb062ede01e97695b58d64d00 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Fri, 16 Aug 2024 13:54:50 -0400 Subject: [PATCH 11/19] Update docs for rule --- .../src/rules/ruff/rules/decimal_from_float_literal.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 2386e6f6ee5f9..371e4dd9d744f 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -9,7 +9,7 @@ use crate::checkers::ast::Checker; /// Checks for `Decimal` calls passing a float literal. /// /// ## Why is this bad? -/// Float literals are non-deterministic and can lead to unexpected results. The `Decimal` class is designed to handle +/// Float literals have limited precision that can lead to unexpected results. The `Decimal` class is designed to handle /// numbers with fixed-point precision, so a string literal should be used instead. /// /// ## Example @@ -21,6 +21,10 @@ use crate::checkers::ast::Checker; /// Use instead: /// ```python /// num = Decimal("1.2345") +/// +/// ## Fix Safety +/// This rule's fix is marked as unsafe because it changes the underlying value of the `Decimal` instance that is +/// constructed. This can lead to unexpected behavior if your program relies on the previous imprecise value. /// ``` #[violation] pub struct DecimalFromFloatLiteral; From a7bd847ca67e221c8d88849bc828d6d26f1d65da Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Fri, 16 Aug 2024 13:55:03 -0400 Subject: [PATCH 12/19] Do faster checks first --- .../ruff/rules/decimal_from_float_literal.rs | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 371e4dd9d744f..776e2bba14ebe 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -42,6 +42,10 @@ impl AlwaysFixableViolation for DecimalFromFloatLiteral { /// RUF032: `Decimal()` called with float literal argument pub(crate) fn decimal_from_float_literal_syntax(checker: &mut Checker, call: &ast::ExprCall) { + let Some(arg) = call.arguments.args.first() else { + return; + }; + if !checker .semantic() .resolve_qualified_name(call.func.as_ref()) @@ -49,27 +53,26 @@ pub(crate) fn decimal_from_float_literal_syntax(checker: &mut Checker, call: &as { return; } - if let Some(arg) = call.arguments.args.first() { + + if let ast::Expr::NumberLiteral(ast::ExprNumberLiteral { + value: ast::Number::Float(_), + .. + }) = arg + { + let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( + fix_float_literal(arg.range(), &checker.generator().expr(arg)), + ); + checker.diagnostics.push(diagnostic); + } else if let ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) = arg { if let ast::Expr::NumberLiteral(ast::ExprNumberLiteral { value: ast::Number::Float(_), .. - }) = arg + }) = operand.as_ref() { let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( fix_float_literal(arg.range(), &checker.generator().expr(arg)), ); checker.diagnostics.push(diagnostic); - } else if let ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) = arg { - if let ast::Expr::NumberLiteral(ast::ExprNumberLiteral { - value: ast::Number::Float(_), - .. - }) = operand.as_ref() - { - let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( - fix_float_literal(arg.range(), &checker.generator().expr(arg)), - ); - checker.diagnostics.push(diagnostic); - } } } } From bbf32f05ccca071f13fcf57597f19404ceeb64f4 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Fri, 16 Aug 2024 14:04:34 -0400 Subject: [PATCH 13/19] Fix docs parse error --- .../src/rules/ruff/rules/decimal_from_float_literal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 776e2bba14ebe..11212eac47940 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -21,11 +21,11 @@ use crate::checkers::ast::Checker; /// Use instead: /// ```python /// num = Decimal("1.2345") +/// ``` /// /// ## Fix Safety /// This rule's fix is marked as unsafe because it changes the underlying value of the `Decimal` instance that is /// constructed. This can lead to unexpected behavior if your program relies on the previous imprecise value. -/// ``` #[violation] pub struct DecimalFromFloatLiteral; From aadb8652504dd5e997457d5888e19dc2d6df32d0 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Fri, 16 Aug 2024 14:17:47 -0400 Subject: [PATCH 14/19] Extract logic to identify float literal --- .../ruff/rules/decimal_from_float_literal.rs | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 11212eac47940..85a05b708ba2a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -54,26 +54,30 @@ pub(crate) fn decimal_from_float_literal_syntax(checker: &mut Checker, call: &as return; } - if let ast::Expr::NumberLiteral(ast::ExprNumberLiteral { - value: ast::Number::Float(_), - .. - }) = arg - { + if is_arg_float_literal(arg) { let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( fix_float_literal(arg.range(), &checker.generator().expr(arg)), ); checker.diagnostics.push(diagnostic); - } else if let ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) = arg { - if let ast::Expr::NumberLiteral(ast::ExprNumberLiteral { + } +} + +fn is_arg_float_literal(arg: &ast::Expr) -> bool { + match arg { + ast::Expr::NumberLiteral(ast::ExprNumberLiteral { value: ast::Number::Float(_), .. - }) = operand.as_ref() - { - let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( - fix_float_literal(arg.range(), &checker.generator().expr(arg)), - ); - checker.diagnostics.push(diagnostic); + }) => true, + ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => { + matches!( + operand.as_ref(), + ast::Expr::NumberLiteral(ast::ExprNumberLiteral { + value: ast::Number::Float(_), + .. + }) + ) } + _ => false, } } From d2ca9ea5f3bd0fc9de1da91f26ad4266616dbb5c Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Fri, 16 Aug 2024 14:29:30 -0400 Subject: [PATCH 15/19] Do faster checks first (again) --- .../src/rules/ruff/rules/decimal_from_float_literal.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 85a05b708ba2a..30d5d26f29749 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -46,15 +46,15 @@ pub(crate) fn decimal_from_float_literal_syntax(checker: &mut Checker, call: &as return; }; - if !checker + if !is_arg_float_literal(arg) { + return; + } + + if checker .semantic() .resolve_qualified_name(call.func.as_ref()) .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["decimal", "Decimal"])) { - return; - } - - if is_arg_float_literal(arg) { let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( fix_float_literal(arg.range(), &checker.generator().expr(arg)), ); From 5ddc822c5efa9f3b36e90335d2ed2e5423320e47 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Fri, 16 Aug 2024 14:31:59 -0400 Subject: [PATCH 16/19] Improve rule docs Co-authored-by: Alex Waygood --- .../src/rules/ruff/rules/decimal_from_float_literal.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 30d5d26f29749..dac54d6fb598c 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -25,7 +25,8 @@ use crate::checkers::ast::Checker; /// /// ## Fix Safety /// This rule's fix is marked as unsafe because it changes the underlying value of the `Decimal` instance that is -/// constructed. This can lead to unexpected behavior if your program relies on the previous imprecise value. +/// constructed. This can lead to unexpected behavior if your program relies on the previous value +/// (whether deliberately or not). #[violation] pub struct DecimalFromFloatLiteral; From d12efda7f45b82f9edebdef1a54af065190804f3 Mon Sep 17 00:00:00 2001 From: Ken Baskett Date: Fri, 16 Aug 2024 14:48:53 -0400 Subject: [PATCH 17/19] Simplify --- .../src/rules/ruff/rules/decimal_from_float_literal.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index dac54d6fb598c..0e2f4398bd2da 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -70,13 +70,7 @@ fn is_arg_float_literal(arg: &ast::Expr) -> bool { .. }) => true, ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => { - matches!( - operand.as_ref(), - ast::Expr::NumberLiteral(ast::ExprNumberLiteral { - value: ast::Number::Float(_), - .. - }) - ) + is_arg_float_literal(operand.as_ref()) } _ => false, } From 401ba8e610ed92490d93980e60a70c58a0d30a05 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 19 Aug 2024 11:09:30 +0200 Subject: [PATCH 18/19] Use quotes from sylist --- .../rules/ruff/rules/decimal_from_float_literal.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 0e2f4398bd2da..55c0d5d60c3cb 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -1,6 +1,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast}; +use ruff_python_codegen::Stylist; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -56,9 +57,12 @@ pub(crate) fn decimal_from_float_literal_syntax(checker: &mut Checker, call: &as .resolve_qualified_name(call.func.as_ref()) .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["decimal", "Decimal"])) { - let diagnostic = Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix( - fix_float_literal(arg.range(), &checker.generator().expr(arg)), - ); + let diagnostic = + Diagnostic::new(DecimalFromFloatLiteral, arg.range()).with_fix(fix_float_literal( + arg.range(), + &checker.generator().expr(arg), + checker.stylist(), + )); checker.diagnostics.push(diagnostic); } } @@ -76,7 +80,7 @@ fn is_arg_float_literal(arg: &ast::Expr) -> bool { } } -fn fix_float_literal(range: TextRange, float_literal: &str) -> Fix { - let content = format!("\"{float_literal}\""); +fn fix_float_literal(range: TextRange, float_literal: &str, stylist: &Stylist) -> Fix { + let content = format!("{quote}{float_literal}{quote}", quote = stylist.quote()); Fix::unsafe_edit(Edit::range_replacement(content, range)) } From 9a8fd5479e26c56c91db9747f4b558bc655f92c7 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 19 Aug 2024 10:16:07 +0100 Subject: [PATCH 19/19] style nitpicks --- .../ruff/rules/decimal_from_float_literal.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs index 55c0d5d60c3cb..3f7120d92359a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/decimal_from_float_literal.rs @@ -10,8 +10,9 @@ use crate::checkers::ast::Checker; /// Checks for `Decimal` calls passing a float literal. /// /// ## Why is this bad? -/// Float literals have limited precision that can lead to unexpected results. The `Decimal` class is designed to handle -/// numbers with fixed-point precision, so a string literal should be used instead. +/// Float literals have limited precision that can lead to unexpected results. +/// The `Decimal` class is designed to handle numbers with fixed-point precision, +/// so a string literal should be used instead. /// /// ## Example /// @@ -25,20 +26,20 @@ use crate::checkers::ast::Checker; /// ``` /// /// ## Fix Safety -/// This rule's fix is marked as unsafe because it changes the underlying value of the `Decimal` instance that is -/// constructed. This can lead to unexpected behavior if your program relies on the previous value -/// (whether deliberately or not). +/// This rule's fix is marked as unsafe because it changes the underlying value +/// of the `Decimal` instance that is constructed. This can lead to unexpected +/// behavior if your program relies on the previous value (whether deliberately or not). #[violation] pub struct DecimalFromFloatLiteral; impl AlwaysFixableViolation for DecimalFromFloatLiteral { #[derive_message_formats] fn message(&self) -> String { - format!(r#"`Decimal()` called with float literal argument"#) + format!("`Decimal()` called with float literal argument") } fn fix_title(&self) -> String { - "Use a string literal instead".into() + "Use a string literal instead".to_string() } } @@ -73,9 +74,7 @@ fn is_arg_float_literal(arg: &ast::Expr) -> bool { value: ast::Number::Float(_), .. }) => true, - ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => { - is_arg_float_literal(operand.as_ref()) - } + ast::Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => is_arg_float_literal(operand), _ => false, } }