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

[ruff] Implement check for Decimal called with a float literal (RUF032) #12909

Merged
merged 20 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,11 +21,15 @@ 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.
kbaskett248 marked this conversation as resolved.
Show resolved Hide resolved
/// ```
#[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"#)
Expand All @@ -37,35 +41,38 @@ 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) {
let Some(arg) = call.arguments.args.first() else {
return;
};

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.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(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);
} 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);
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Loading