[refurb] Implement unnecessary-from-float (FURB164)

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

@@ -0,0 +1,34 @@
+from decimal import Decimal
+from fractions import Fraction
+import decimal
+import fractions
+
+# Errors
+_ = Fraction.from_float(0.1)
+_ = Fraction.from_float(-0.5)
+_ = Fraction.from_float(5.0)
+_ = fractions.Fraction.from_float(4.2)
+_ = Fraction.from_decimal(Decimal("4.2"))
+_ = Fraction.from_decimal(Decimal("-4.2"))
+_ = Fraction.from_decimal(Decimal.from_float(4.2))
+_ = Decimal.from_float(0.1)
+_ = Decimal.from_float(-0.5)
+_ = Decimal.from_float(5.0)
+_ = decimal.Decimal.from_float(4.2)
+_ = Decimal.from_float(float("inf"))
+_ = Decimal.from_float(float("-inf"))
+_ = Decimal.from_float(float("Infinity"))
+_ = Decimal.from_float(float("-Infinity"))
+_ = Decimal.from_float(float("nan"))
+
+# OK
+_ = Fraction(0.1)
+_ = Fraction(-0.5)
+_ = Fraction(5.0)
+_ = fractions.Fraction(4.2)
+_ = Fraction(Decimal("4.2"))
+_ = Fraction(Decimal("-4.2"))
+_ = Decimal(0.1)
+_ = Decimal(-0.5)
+_ = Decimal(5.0)
+_ = decimal.Decimal(4.2)

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

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

crates/ruff_linter/src/codes.rs

@@ -1051,6 +1051,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (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, "164") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryFromFloat),
         (Refurb, "167") => (RuleGroup::Preview, rules::refurb::rules::RegexFlagAlias),
         (Refurb, "168") => (RuleGroup::Preview, rules::refurb::rules::IsinstanceTypeNone),
         (Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison),

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

@@ -27,6 +27,7 @@ mod tests {
     #[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::UnnecessaryFromFloat, Path::new("FURB164.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

@@ -21,6 +21,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 unnecessary_from_float::*;
 pub(crate) use verbose_decimal_constructor::*;
 
 mod bit_count;
@@ -46,4 +47,5 @@ mod single_item_membership_test;
 mod slice_copy;
 mod type_none_comparison;
 mod unnecessary_enumerate;
+mod unnecessary_from_float;
 mod verbose_decimal_constructor;

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

@@ -0,0 +1,205 @@
+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_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for unnecessary `from_float` and `from_decimal` usages to construct
+/// `Decimal` and `Fraction` instances.
+///
+/// ## Why is this bad?
+/// Since Python 3.2, the `Fraction` and `Decimal` classes can be constructed
+/// by passing float or decimal instances to the constructor directly. As such,
+/// the use of `from_float` and `from_decimal` methods is unnecessary, and
+/// should be avoided in favor of the more concise constructor syntax.
+///
+/// ## Examples
+/// ```python
+/// Decimal.from_float(4.2)
+/// Decimal.from_float(float("inf"))
+/// Fraction.from_float(4.2)
+/// Fraction.from_decimal(Decimal("4.2"))
+/// ```
+///
+/// Use instead:
+/// ```python
+/// Decimal(4.2)
+/// Decimal("inf")
+/// Fraction(4.2)
+/// Fraction(Decimal(4.2))
+/// ```
+///
+/// ## References
+/// - [Python documentation: `decimal`](https://docs.python.org/3/library/decimal.html)
+/// - [Python documentation: `fractions`](https://docs.python.org/3/library/fractions.html)
+#[violation]
+pub struct UnnecessaryFromFloat {
+    method_name: MethodName,
+    constructor: Constructor,
+}
+
+impl Violation for UnnecessaryFromFloat {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let UnnecessaryFromFloat {
+            method_name,
+            constructor,
+        } = self;
+        format!("Verbose method `{method_name}` in `{constructor}` construction",)
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        let UnnecessaryFromFloat { constructor, .. } = self;
+        Some(format!("Replace with `{constructor}` constructor"))
+    }
+}
+
+/// FURB164
+pub(crate) fn unnecessary_from_float(checker: &mut Checker, call: &ExprCall) {
+    let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = &*call.func else {
+        return;
+    };
+
+    // The method name must be either `from_float` or `from_decimal`.
+    let method_name = match attr.as_str() {
+        "from_float" => MethodName::FromFloat,
+        "from_decimal" => MethodName::FromDecimal,
+        _ => return,
+    };
+
+    // The value must be either `decimal.Decimal` or `fractions.Fraction`.
+    let Some(constructor) =
+        checker
+            .semantic()
+            .resolve_qualified_name(value)
+            .and_then(|qualified_name| match qualified_name.segments() {
+                ["decimal", "Decimal"] => Some(Constructor::Decimal),
+                ["fractions", "Fraction"] => Some(Constructor::Fraction),
+                _ => None,
+            })
+    else {
+        return;
+    };
+
+    // `Decimal.from_decimal` doesn't exist.
+    if matches!(
+        (method_name, constructor),
+        (MethodName::FromDecimal, Constructor::Decimal)
+    ) {
+        return;
+    }
+
+    let mut diagnostic = Diagnostic::new(
+        UnnecessaryFromFloat {
+            method_name,
+            constructor,
+        },
+        call.range(),
+    );
+
+    let edit = Edit::range_replacement(
+        checker.locator().slice(&**value).to_string(),
+        call.func.range(),
+    );
+
+    // Short-circuit case for special values, such as: `Decimal.from_float(float("inf"))` to `Decimal("inf")`.
+    'short_circuit: {
+        if !matches!(constructor, Constructor::Decimal) {
+            break 'short_circuit;
+        };
+        if !(method_name == MethodName::FromFloat) {
+            break 'short_circuit;
+        };
+
+        let Some(value) = (match method_name {
+            MethodName::FromFloat => call.arguments.find_argument("f", 0),
+            MethodName::FromDecimal => call.arguments.find_argument("dec", 0),
+        }) else {
+            return;
+        };
+
+        let Expr::Call(
+            call @ ast::ExprCall {
+                func, arguments, ..
+            },
+        ) = value
+        else {
+            break 'short_circuit;
+        };
+
+        // Must be a call to the `float` builtin.
+        let Some(func_name) = func.as_name_expr() else {
+            break 'short_circuit;
+        };
+        if func_name.id != "float" {
+            break 'short_circuit;
+        };
+
+        // Must have exactly one argument, which is a string literal.
+        if arguments.keywords.len() != 0 {
+            break 'short_circuit;
+        };
+        let [float] = arguments.args.as_ref() else {
+            break 'short_circuit;
+        };
+        let Some(float) = float.as_string_literal_expr() else {
+            break 'short_circuit;
+        };
+        if !matches!(
+            float.value.to_str().to_lowercase().as_str(),
+            "inf" | "-inf" | "infinity" | "-infinity" | "nan"
+        ) {
+            break 'short_circuit;
+        }
+
+        if !checker.semantic().is_builtin("float") {
+            break 'short_circuit;
+        };
+
+        let replacement = checker.locator().slice(float).to_string();
+        diagnostic.set_fix(Fix::safe_edits(
+            edit,
+            [Edit::range_replacement(replacement, call.range())],
+        ));
+        checker.diagnostics.push(diagnostic);
+
+        return;
+    }
+
+    diagnostic.set_fix(Fix::safe_edit(edit));
+    checker.diagnostics.push(diagnostic);
+}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+enum MethodName {
+    FromFloat,
+    FromDecimal,
+}
+
+impl std::fmt::Display for MethodName {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match self {
+            MethodName::FromFloat => fmt.write_str("from_float"),
+            MethodName::FromDecimal => fmt.write_str("from_decimal"),
+        }
+    }
+}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+enum Constructor {
+    Decimal,
+    Fraction,
+}
+
+impl std::fmt::Display for Constructor {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match self {
+            Constructor::Decimal => fmt.write_str("Decimal"),
+            Constructor::Fraction => fmt.write_str("Fraction"),
+        }
+    }
+}

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

@@ -1,6 +1,6 @@
 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_ast::{self as ast, Expr};
 use ruff_python_trivia::PythonWhitespace;
 use ruff_text_size::Ranged;
 
@@ -56,7 +56,7 @@ impl Violation for VerboseDecimalConstructor {
 }
 
 /// FURB157
-pub(crate) fn verbose_decimal_constructor(checker: &mut Checker, call: &ExprCall) {
+pub(crate) fn verbose_decimal_constructor(checker: &mut Checker, call: &ast::ExprCall) {
     if !checker
         .semantic()
         .resolve_qualified_name(&call.func)