[`refurb`] Preserve digit separators in `Decimal` constructor (`FURB157`) (#20588)

## Summary

Fixes #20572

---------

Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
Dan Parizher 2025-10-28 17:47:52 -04:00 committed by GitHub
parent d0aebaa253
commit 349061117c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 232 additions and 14 deletions

View File

@ -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")

View File

@ -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

View File

@ -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)