[refurb] Implement verbose-decimal-constructor (FURB157)

crates/ruff_linter/resources/test/fixtures/refurb/FURB157.py

@@ -0,0 +1,17 @@
+import decimal
+from decimal import Decimal
+
+# Errors
+Decimal("0")
+Decimal("-42")
+Decimal(float("Infinity"))
+Decimal(float("-Infinity"))
+Decimal(float("inf"))
+Decimal(float("-inf"))
+Decimal(float("nan"))
+decimal.Decimal("0")
+
+# OK
+Decimal(0)
+Decimal("Infinity")
+decimal.Decimal(0)

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -926,6 +926,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::RedundantLogBase) {
                 refurb::rules::redundant_log_base(checker, call);
             }
+            if checker.enabled(Rule::VerboseDecimalConstructor) {
+                refurb::rules::verbose_decimal_constructor(checker, call);
+            }
             if checker.enabled(Rule::QuadraticListSummation) {
                 ruff::rules::quadratic_list_summation(checker, call);
             }

crates/ruff_linter/src/codes.rs

@@ -1047,6 +1047,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (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, "157") => (RuleGroup::Preview, rules::refurb::rules::VerboseDecimalConstructor),
         (Refurb, "161") => (RuleGroup::Preview, rules::refurb::rules::BitCount),
         (Refurb, "163") => (RuleGroup::Preview, rules::refurb::rules::RedundantLogBase),
         (Refurb, "167") => (RuleGroup::Preview, rules::refurb::rules::RegexFlagAlias),

crates/ruff_linter/src/rules/refurb/mod.rs

@@ -25,6 +25,7 @@ mod tests {
     #[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::VerboseDecimalConstructor, Path::new("FURB157.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"))]

crates/ruff_linter/src/rules/refurb/rules/mod.rs

@@ -20,6 +20,7 @@ pub(crate) use single_item_membership_test::*;
 pub(crate) use slice_copy::*;
 pub(crate) use type_none_comparison::*;
 pub(crate) use unnecessary_enumerate::*;
+pub(crate) use verbose_decimal_constructor::*;
 
 mod bit_count;
 mod check_and_remove_from_set;
@@ -43,3 +44,4 @@ mod single_item_membership_test;
 mod slice_copy;
 mod type_none_comparison;
 mod unnecessary_enumerate;
+mod verbose_decimal_constructor;

crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs

@@ -0,0 +1,168 @@
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{self as ast, Expr, ExprCall};
+use ruff_python_trivia::PythonWhitespace;
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for unnecessary string literal or float casts in `Decimal`
+/// constructors.
+///
+/// ## Why is this bad?
+/// The `Decimal` constructor accepts a variety of arguments, including
+/// integers, floats, and strings. However, it's not necessary to cast
+/// integer literals to strings when passing them to the `Decimal`.
+///
+/// Similarly, `Decimal` accepts `inf`, `-inf`, and `nan` as string literals,
+/// so there's no need to wrap those values in a `float` call when passing
+/// them to the `Decimal` constructor.
+///
+/// Prefer the more concise form of argument passing for `Decimal`
+/// constructors, as it's more readable and idiomatic.
+///
+/// ## Example
+/// ```python
+/// Decimal("0")
+/// Decimal(float("Infinity"))
+/// ```
+///
+/// Use instead:
+/// ```python
+/// Decimal(0)
+/// Decimal("Infinity")
+/// ```
+///
+/// ## References
+/// - [Python documentation: `decimal`](https://docs.python.org/3/library/decimal.html)
+#[violation]
+pub struct VerboseDecimalConstructor {
+    replacement: String,
+}
+
+impl Violation for VerboseDecimalConstructor {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Always;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Verbose expression in `Decimal` constructor")
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        let VerboseDecimalConstructor { replacement } = self;
+        Some(format!("Replace with `{replacement}`"))
+    }
+}
+
+/// FURB157
+pub(crate) fn verbose_decimal_constructor(checker: &mut Checker, call: &ExprCall) {
+    if !checker
+        .semantic()
+        .resolve_qualified_name(&call.func)
+        .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["decimal", "Decimal"]))
+    {
+        return;
+    }
+
+    // Decimal accepts arguments of the form: `Decimal(value='0', context=None)`
+    let Some(value) = call.arguments.find_argument("value", 0) else {
+        return;
+    };
+
+    let diagnostic = match value {
+        Expr::StringLiteral(ast::ExprStringLiteral {
+            value: str_literal, ..
+        }) => {
+            // Parse the inner string as an integer.
+            let trimmed = str_literal.to_str().trim_whitespace();
+
+            // Extract the unary sign, if any.
+            let (unary, rest) = if let Some(trimmed) = trimmed.strip_prefix('+') {
+                ("+", trimmed)
+            } else if let Some(trimmed) = trimmed.strip_prefix('-') {
+                ("-", trimmed)
+            } else {
+                ("", trimmed)
+            };
+
+            // Skip leading zeros.
+            let rest = rest.trim_start_matches('0');
+
+            // Verify that the rest of the string is a valid integer.
+            if !rest.chars().all(|c| c.is_ascii_digit()) {
+                return;
+            };
+
+            // If all the characters are zeros, then the value is zero.
+            let rest = if rest.is_empty() { "0" } else { rest };
+
+            let replacement = format!("{unary}{rest}");
+            let mut diagnostic = Diagnostic::new(
+                VerboseDecimalConstructor {
+                    replacement: replacement.clone(),
+                },
+                value.range(),
+            );
+
+            diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                replacement,
+                value.range(),
+            )));
+
+            diagnostic
+        }
+        Expr::Call(ast::ExprCall {
+            func, arguments, ..
+        }) => {
+            // Must be a call to the `float` builtin.
+            let Some(func_name) = func.as_name_expr() else {
+                return;
+            };
+            if func_name.id != "float" {
+                return;
+            };
+
+            // Must have exactly one argument, which is a string literal.
+            if arguments.keywords.len() != 0 {
+                return;
+            };
+            let [float] = arguments.args.as_ref() else {
+                return;
+            };
+            let Some(float) = float.as_string_literal_expr() else {
+                return;
+            };
+            if !matches!(
+                float.value.to_str().to_lowercase().as_str(),
+                "inf" | "-inf" | "infinity" | "-infinity" | "nan"
+            ) {
+                return;
+            }
+
+            if !checker.semantic().is_builtin("float") {
+                return;
+            };
+
+            let replacement = checker.locator().slice(float).to_string();
+            let mut diagnostic = Diagnostic::new(
+                VerboseDecimalConstructor {
+                    replacement: replacement.clone(),
+                },
+                value.range(),
+            );
+
+            diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                replacement,
+                value.range(),
+            )));
+
+            diagnostic
+        }
+        _ => {
+            return;
+        }
+    };
+
+    checker.diagnostics.push(diagnostic);
+}

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB157_FURB157.py.snap

@@ -0,0 +1,168 @@
+---
+source: crates/ruff_linter/src/rules/refurb/mod.rs
+---
+FURB157.py:5:9: FURB157 [*] Verbose expression in `Decimal` constructor
+  |
+4 | # Errors
+5 | Decimal("0")
+  |         ^^^ FURB157
+6 | Decimal("-42")
+7 | Decimal(float("Infinity"))
+  |
+  = help: Replace with `0`
+
+ℹ Safe fix
+2 2 | from decimal import Decimal
+3 3 | 
+4 4 | # Errors
+5   |-Decimal("0")
+  5 |+Decimal(0)
+6 6 | Decimal("-42")
+7 7 | Decimal(float("Infinity"))
+8 8 | Decimal(float("-Infinity"))
+
+FURB157.py:6:9: FURB157 [*] Verbose expression in `Decimal` constructor
+  |
+4 | # Errors
+5 | Decimal("0")
+6 | Decimal("-42")
+  |         ^^^^^ FURB157
+7 | Decimal(float("Infinity"))
+8 | Decimal(float("-Infinity"))
+  |
+  = help: Replace with `-42`
+
+ℹ Safe fix
+3 3 | 
+4 4 | # Errors
+5 5 | Decimal("0")
+6   |-Decimal("-42")
+  6 |+Decimal(-42)
+7 7 | Decimal(float("Infinity"))
+8 8 | Decimal(float("-Infinity"))
+9 9 | Decimal(float("inf"))
+
+FURB157.py:7:9: FURB157 [*] Verbose expression in `Decimal` constructor
+  |
+5 | Decimal("0")
+6 | Decimal("-42")
+7 | Decimal(float("Infinity"))
+  |         ^^^^^^^^^^^^^^^^^ FURB157
+8 | Decimal(float("-Infinity"))
+9 | Decimal(float("inf"))
+  |
+  = help: Replace with `"Infinity"`
+
+ℹ Safe fix
+4 4 | # Errors
+5 5 | Decimal("0")
+6 6 | Decimal("-42")
+7   |-Decimal(float("Infinity"))
+  7 |+Decimal("Infinity")
+8 8 | Decimal(float("-Infinity"))
+9 9 | Decimal(float("inf"))
+10 10 | Decimal(float("-inf"))
+
+FURB157.py:8:9: FURB157 [*] Verbose expression in `Decimal` constructor
+   |
+ 6 | Decimal("-42")
+ 7 | Decimal(float("Infinity"))
+ 8 | Decimal(float("-Infinity"))
+   |         ^^^^^^^^^^^^^^^^^^ FURB157
+ 9 | Decimal(float("inf"))
+10 | Decimal(float("-inf"))
+   |
+   = help: Replace with `"-Infinity"`
+
+ℹ Safe fix
+5 5 | Decimal("0")
+6 6 | Decimal("-42")
+7 7 | Decimal(float("Infinity"))
+8   |-Decimal(float("-Infinity"))
+  8 |+Decimal("-Infinity")
+9 9 | Decimal(float("inf"))
+10 10 | Decimal(float("-inf"))
+11 11 | Decimal(float("nan"))
+
+FURB157.py:9:9: FURB157 [*] Verbose expression in `Decimal` constructor
+   |
+ 7 | Decimal(float("Infinity"))
+ 8 | Decimal(float("-Infinity"))
+ 9 | Decimal(float("inf"))
+   |         ^^^^^^^^^^^^ FURB157
+10 | Decimal(float("-inf"))
+11 | Decimal(float("nan"))
+   |
+   = help: Replace with `"inf"`
+
+ℹ Safe fix
+6  6  | Decimal("-42")
+7  7  | Decimal(float("Infinity"))
+8  8  | Decimal(float("-Infinity"))
+9     |-Decimal(float("inf"))
+   9  |+Decimal("inf")
+10 10 | Decimal(float("-inf"))
+11 11 | Decimal(float("nan"))
+12 12 | decimal.Decimal("0")
+
+FURB157.py:10:9: FURB157 [*] Verbose expression in `Decimal` constructor
+   |
+ 8 | Decimal(float("-Infinity"))
+ 9 | Decimal(float("inf"))
+10 | Decimal(float("-inf"))
+   |         ^^^^^^^^^^^^^ FURB157
+11 | Decimal(float("nan"))
+12 | decimal.Decimal("0")
+   |
+   = help: Replace with `"-inf"`
+
+ℹ Safe fix
+7  7  | Decimal(float("Infinity"))
+8  8  | Decimal(float("-Infinity"))
+9  9  | Decimal(float("inf"))
+10    |-Decimal(float("-inf"))
+   10 |+Decimal("-inf")
+11 11 | Decimal(float("nan"))
+12 12 | decimal.Decimal("0")
+13 13 | 
+
+FURB157.py:11:9: FURB157 [*] Verbose expression in `Decimal` constructor
+   |
+ 9 | Decimal(float("inf"))
+10 | Decimal(float("-inf"))
+11 | Decimal(float("nan"))
+   |         ^^^^^^^^^^^^ FURB157
+12 | decimal.Decimal("0")
+   |
+   = help: Replace with `"nan"`
+
+ℹ Safe fix
+8  8  | Decimal(float("-Infinity"))
+9  9  | Decimal(float("inf"))
+10 10 | Decimal(float("-inf"))
+11    |-Decimal(float("nan"))
+   11 |+Decimal("nan")
+12 12 | decimal.Decimal("0")
+13 13 | 
+14 14 | # OK
+
+FURB157.py:12:17: FURB157 [*] Verbose expression in `Decimal` constructor
+   |
+10 | Decimal(float("-inf"))
+11 | Decimal(float("nan"))
+12 | decimal.Decimal("0")
+   |                 ^^^ FURB157
+13 | 
+14 | # OK
+   |
+   = help: Replace with `0`
+
+ℹ Safe fix
+9  9  | Decimal(float("inf"))
+10 10 | Decimal(float("-inf"))
+11 11 | Decimal(float("nan"))
+12    |-decimal.Decimal("0")
+   12 |+decimal.Decimal(0)
+13 13 | 
+14 14 | # OK
+15 15 | Decimal(0)