diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB157.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB157.py index 236fe1b046..d795fd1941 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB157.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB157.py @@ -69,3 +69,19 @@ Decimal(float("\N{space}\N{hyPHen-MINus}nan")) Decimal(float("\x20\N{character tabulation}\N{hyphen-minus}nan")) Decimal(float(" -" "nan")) Decimal(float("-nAn")) + +# Test cases for digit separators (safe fixes) +# https://github.com/astral-sh/ruff/issues/20572 +Decimal("15_000_000") # Safe fix: normalizes separators, becomes Decimal(15_000_000) +Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) +Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) +Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) + +# Test cases for non-thousands separators +Decimal("12_34_56_78") # Safe fix: preserves non-thousands separators +Decimal("1234_5678") # Safe fix: preserves non-thousands separators + +# Separators _and_ leading zeros +Decimal("0001_2345") +Decimal("000_1_2345") +Decimal("000_000") diff --git a/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs b/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs index 04d169e3f6..50c98026d5 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs @@ -1,5 +1,6 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; +use itertools::Itertools; use ruff_python_ast::{self as ast, Expr}; use ruff_python_trivia::PythonWhitespace; use ruff_text_size::Ranged; @@ -91,18 +92,20 @@ pub(crate) fn verbose_decimal_constructor(checker: &Checker, call: &ast::ExprCal // using this regex: // https://github.com/python/cpython/blob/ac556a2ad1213b8bb81372fe6fb762f5fcb076de/Lib/_pydecimal.py#L6060-L6077 // _after_ trimming whitespace from the string and removing all occurrences of "_". - let mut trimmed = Cow::from(str_literal.to_str().trim_whitespace()); - if memchr::memchr(b'_', trimmed.as_bytes()).is_some() { - trimmed = Cow::from(trimmed.replace('_', "")); - } + let original_str = str_literal.to_str().trim_whitespace(); // Extract the unary sign, if any. - let (unary, rest) = if let Some(trimmed) = trimmed.strip_prefix('+') { - ("+", Cow::from(trimmed)) - } else if let Some(trimmed) = trimmed.strip_prefix('-') { - ("-", Cow::from(trimmed)) + let (unary, original_str) = if let Some(trimmed) = original_str.strip_prefix('+') { + ("+", trimmed) + } else if let Some(trimmed) = original_str.strip_prefix('-') { + ("-", trimmed) } else { - ("", trimmed) + ("", original_str) }; + let mut rest = Cow::from(original_str); + let has_digit_separators = memchr::memchr(b'_', rest.as_bytes()).is_some(); + if has_digit_separators { + rest = Cow::from(rest.replace('_', "")); + } // Early return if we now have an empty string // or a very long string: @@ -118,6 +121,13 @@ pub(crate) fn verbose_decimal_constructor(checker: &Checker, call: &ast::ExprCal return; } + // If the original string had digit separators, normalize them + let rest = if has_digit_separators { + Cow::from(normalize_digit_separators(original_str)) + } else { + Cow::from(rest) + }; + // If all the characters are zeros, then the value is zero. let rest = match (unary, rest.is_empty()) { // `Decimal("-0")` is not the same as `Decimal("0")` @@ -126,10 +136,11 @@ pub(crate) fn verbose_decimal_constructor(checker: &Checker, call: &ast::ExprCal return; } (_, true) => "0", - _ => rest, + _ => &rest, }; let replacement = format!("{unary}{rest}"); + let mut diagnostic = checker.report_diagnostic( VerboseDecimalConstructor { replacement: replacement.clone(), @@ -186,6 +197,22 @@ pub(crate) fn verbose_decimal_constructor(checker: &Checker, call: &ast::ExprCal } } +/// Normalizes digit separators in a numeric string by: +/// - Stripping leading and trailing underscores +/// - Collapsing medial underscore sequences to single underscores +fn normalize_digit_separators(original_str: &str) -> String { + // Strip leading and trailing underscores + let trimmed = original_str + .trim_start_matches(['_', '0']) + .trim_end_matches('_'); + + // Collapse medial underscore sequences to single underscores + trimmed + .chars() + .dedup_by(|a, b| *a == '_' && a == b) + .collect() +} + // ```console // $ python // >>> import sys diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB157_FURB157.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB157_FURB157.py.snap index 3bb10588ab..92e8057055 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB157_FURB157.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB157_FURB157.py.snap @@ -167,12 +167,12 @@ FURB157 [*] Verbose expression in `Decimal` constructor | ^^^^^^^ 24 | Decimal("__1____000") | -help: Replace with `1000` +help: Replace with `1_000` 20 | # See https://github.com/astral-sh/ruff/issues/13807 21 | 22 | # Errors - Decimal("1_000") -23 + Decimal(1000) +23 + Decimal(1_000) 24 | Decimal("__1____000") 25 | 26 | # Ok @@ -187,12 +187,12 @@ FURB157 [*] Verbose expression in `Decimal` constructor 25 | 26 | # Ok | -help: Replace with `1000` +help: Replace with `1_000` 21 | 22 | # Errors 23 | Decimal("1_000") - Decimal("__1____000") -24 + Decimal(1000) +24 + Decimal(1_000) 25 | 26 | # Ok 27 | Decimal("2e-4") @@ -494,6 +494,7 @@ help: Replace with `"nan"` 69 + Decimal("nan") 70 | Decimal(float(" -" "nan")) 71 | Decimal(float("-nAn")) +72 | FURB157 [*] Verbose expression in `Decimal` constructor --> FURB157.py:70:9 @@ -511,6 +512,8 @@ help: Replace with `"nan"` - Decimal(float(" -" "nan")) 70 + Decimal("nan") 71 | Decimal(float("-nAn")) +72 | +73 | # Test cases for digit separators (safe fixes) FURB157 [*] Verbose expression in `Decimal` constructor --> FURB157.py:71:9 @@ -519,6 +522,8 @@ FURB157 [*] Verbose expression in `Decimal` constructor 70 | Decimal(float(" -" "nan")) 71 | Decimal(float("-nAn")) | ^^^^^^^^^^^^^ +72 | +73 | # Test cases for digit separators (safe fixes) | help: Replace with `"nan"` 68 | Decimal(float("\N{space}\N{hyPHen-MINus}nan")) @@ -526,3 +531,173 @@ help: Replace with `"nan"` 70 | Decimal(float(" -" "nan")) - Decimal(float("-nAn")) 71 + Decimal("nan") +72 | +73 | # Test cases for digit separators (safe fixes) +74 | # https://github.com/astral-sh/ruff/issues/20572 + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:75:9 + | +73 | # Test cases for digit separators (safe fixes) +74 | # https://github.com/astral-sh/ruff/issues/20572 +75 | Decimal("15_000_000") # Safe fix: normalizes separators, becomes Decimal(15_000_000) + | ^^^^^^^^^^^^ +76 | Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) +77 | Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) + | +help: Replace with `15_000_000` +72 | +73 | # Test cases for digit separators (safe fixes) +74 | # https://github.com/astral-sh/ruff/issues/20572 + - Decimal("15_000_000") # Safe fix: normalizes separators, becomes Decimal(15_000_000) +75 + Decimal(15_000_000) # Safe fix: normalizes separators, becomes Decimal(15_000_000) +76 | Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) +77 | Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) +78 | Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:76:9 + | +74 | # https://github.com/astral-sh/ruff/issues/20572 +75 | Decimal("15_000_000") # Safe fix: normalizes separators, becomes Decimal(15_000_000) +76 | Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) + | ^^^^^^^^^^^ +77 | Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) +78 | Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) + | +help: Replace with `1_234_567` +73 | # Test cases for digit separators (safe fixes) +74 | # https://github.com/astral-sh/ruff/issues/20572 +75 | Decimal("15_000_000") # Safe fix: normalizes separators, becomes Decimal(15_000_000) + - Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) +76 + Decimal(1_234_567) # Safe fix: normalizes separators, becomes Decimal(1_234_567) +77 | Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) +78 | Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) +79 | + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:77:9 + | +75 | Decimal("15_000_000") # Safe fix: normalizes separators, becomes Decimal(15_000_000) +76 | Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) +77 | Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) + | ^^^^^^^^ +78 | Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) + | +help: Replace with `-5_000` +74 | # https://github.com/astral-sh/ruff/issues/20572 +75 | Decimal("15_000_000") # Safe fix: normalizes separators, becomes Decimal(15_000_000) +76 | Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) + - Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) +77 + Decimal(-5_000) # Safe fix: normalizes separators, becomes Decimal(-5_000) +78 | Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) +79 | +80 | # Test cases for non-thousands separators + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:78:9 + | +76 | Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) +77 | Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) +78 | Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) + | ^^^^^^^^ +79 | +80 | # Test cases for non-thousands separators + | +help: Replace with `+9_999` +75 | Decimal("15_000_000") # Safe fix: normalizes separators, becomes Decimal(15_000_000) +76 | Decimal("1_234_567") # Safe fix: normalizes separators, becomes Decimal(1_234_567) +77 | Decimal("-5_000") # Safe fix: normalizes separators, becomes Decimal(-5_000) + - Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) +78 + Decimal(+9_999) # Safe fix: normalizes separators, becomes Decimal(+9_999) +79 | +80 | # Test cases for non-thousands separators +81 | Decimal("12_34_56_78") # Safe fix: preserves non-thousands separators + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:81:9 + | +80 | # Test cases for non-thousands separators +81 | Decimal("12_34_56_78") # Safe fix: preserves non-thousands separators + | ^^^^^^^^^^^^^ +82 | Decimal("1234_5678") # Safe fix: preserves non-thousands separators + | +help: Replace with `12_34_56_78` +78 | Decimal("+9_999") # Safe fix: normalizes separators, becomes Decimal(+9_999) +79 | +80 | # Test cases for non-thousands separators + - Decimal("12_34_56_78") # Safe fix: preserves non-thousands separators +81 + Decimal(12_34_56_78) # Safe fix: preserves non-thousands separators +82 | Decimal("1234_5678") # Safe fix: preserves non-thousands separators +83 | +84 | # Separators _and_ leading zeros + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:82:9 + | +80 | # Test cases for non-thousands separators +81 | Decimal("12_34_56_78") # Safe fix: preserves non-thousands separators +82 | Decimal("1234_5678") # Safe fix: preserves non-thousands separators + | ^^^^^^^^^^^ +83 | +84 | # Separators _and_ leading zeros + | +help: Replace with `1234_5678` +79 | +80 | # Test cases for non-thousands separators +81 | Decimal("12_34_56_78") # Safe fix: preserves non-thousands separators + - Decimal("1234_5678") # Safe fix: preserves non-thousands separators +82 + Decimal(1234_5678) # Safe fix: preserves non-thousands separators +83 | +84 | # Separators _and_ leading zeros +85 | Decimal("0001_2345") + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:85:9 + | +84 | # Separators _and_ leading zeros +85 | Decimal("0001_2345") + | ^^^^^^^^^^^ +86 | Decimal("000_1_2345") +87 | Decimal("000_000") + | +help: Replace with `1_2345` +82 | Decimal("1234_5678") # Safe fix: preserves non-thousands separators +83 | +84 | # Separators _and_ leading zeros + - Decimal("0001_2345") +85 + Decimal(1_2345) +86 | Decimal("000_1_2345") +87 | Decimal("000_000") + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:86:9 + | +84 | # Separators _and_ leading zeros +85 | Decimal("0001_2345") +86 | Decimal("000_1_2345") + | ^^^^^^^^^^^^ +87 | Decimal("000_000") + | +help: Replace with `1_2345` +83 | +84 | # Separators _and_ leading zeros +85 | Decimal("0001_2345") + - Decimal("000_1_2345") +86 + Decimal(1_2345) +87 | Decimal("000_000") + +FURB157 [*] Verbose expression in `Decimal` constructor + --> FURB157.py:87:9 + | +85 | Decimal("0001_2345") +86 | Decimal("000_1_2345") +87 | Decimal("000_000") + | ^^^^^^^^^ + | +help: Replace with `0` +84 | # Separators _and_ leading zeros +85 | Decimal("0001_2345") +86 | Decimal("000_1_2345") + - Decimal("000_000") +87 + Decimal(0)