Skip to content

Commit 47e41ac

Browse files
authored
[refurb] Fix false negative for underscores before sign in Decimal constructor (FURB157) (#21190)
## Summary Fixes FURB157 false negative where `Decimal("_-1")` was not flagged as verbose when underscores precede the sign character. This fixes #21186. ## Problem Analysis The `verbose-decimal-constructor` (FURB157) rule failed to detect verbose `Decimal` constructors when the sign character (`+` or `-`) was preceded by underscores. For example, `Decimal("_-1")` was not flagged, even though it can be simplified to `Decimal(-1)`. The bug occurred because the rule checked for the sign character at the start of the string before stripping leading underscores. According to Python's `Decimal` parser behavior (as documented in CPython's `_pydecimal.py`), underscores are removed before parsing the sign. The rule's logic didn't match this behavior, causing a false negative for cases like `"_-1"` where the underscore came before the sign. This was a regression introduced in version 0.14.3, as these cases were correctly flagged in version 0.14.2. ## Approach The fix updates the sign extraction logic to: 1. Strip leading underscores first (matching Python's Decimal parser behavior) 2. Extract the sign from the underscore-stripped string 3. Preserve the string after the sign for normalization purposes This ensures that cases like `Decimal("_-1")`, `Decimal("_+1")`, and `Decimal("_-1_000")` are correctly detected and flagged. The normalization logic was also updated to use the string after the sign (without underscores) to avoid double signs in the replacement output.
1 parent 2e7ab00 commit 47e41ac

File tree

3 files changed

+76
-6
lines changed

3 files changed

+76
-6
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,9 @@
8585
Decimal("0001_2345")
8686
Decimal("000_1_2345")
8787
Decimal("000_000")
88+
89+
# Test cases for underscores before sign
90+
# https://github.com/astral-sh/ruff/issues/21186
91+
Decimal("_-1") # Should flag as verbose
92+
Decimal("_+1") # Should flag as verbose
93+
Decimal("_-1_000") # Should flag as verbose

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,16 +93,21 @@ pub(crate) fn verbose_decimal_constructor(checker: &Checker, call: &ast::ExprCal
9393
// https://github.com/python/cpython/blob/ac556a2ad1213b8bb81372fe6fb762f5fcb076de/Lib/_pydecimal.py#L6060-L6077
9494
// _after_ trimming whitespace from the string and removing all occurrences of "_".
9595
let original_str = str_literal.to_str().trim_whitespace();
96+
// Strip leading underscores before extracting the sign, as Python's Decimal parser
97+
// removes underscores before parsing the sign.
98+
let sign_check_str = original_str.trim_start_matches('_');
9699
// Extract the unary sign, if any.
97-
let (unary, original_str) = if let Some(trimmed) = original_str.strip_prefix('+') {
100+
let (unary, sign_check_str) = if let Some(trimmed) = sign_check_str.strip_prefix('+') {
98101
("+", trimmed)
99-
} else if let Some(trimmed) = original_str.strip_prefix('-') {
102+
} else if let Some(trimmed) = sign_check_str.strip_prefix('-') {
100103
("-", trimmed)
101104
} else {
102-
("", original_str)
105+
("", sign_check_str)
103106
};
104-
let mut rest = Cow::from(original_str);
105-
let has_digit_separators = memchr::memchr(b'_', rest.as_bytes()).is_some();
107+
// Save the string after the sign for normalization (before removing underscores)
108+
let str_after_sign_for_normalization = sign_check_str;
109+
let mut rest = Cow::from(sign_check_str);
110+
let has_digit_separators = memchr::memchr(b'_', original_str.as_bytes()).is_some();
106111
if has_digit_separators {
107112
rest = Cow::from(rest.replace('_', ""));
108113
}
@@ -123,7 +128,7 @@ pub(crate) fn verbose_decimal_constructor(checker: &Checker, call: &ast::ExprCal
123128

124129
// If the original string had digit separators, normalize them
125130
let rest = if has_digit_separators {
126-
Cow::from(normalize_digit_separators(original_str))
131+
Cow::from(normalize_digit_separators(str_after_sign_for_normalization))
127132
} else {
128133
Cow::from(rest)
129134
};

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,7 @@ help: Replace with `1_2345`
669669
85 + Decimal(1_2345)
670670
86 | Decimal("000_1_2345")
671671
87 | Decimal("000_000")
672+
88 |
672673

673674
FURB157 [*] Verbose expression in `Decimal` constructor
674675
--> FURB157.py:86:9
@@ -686,6 +687,8 @@ help: Replace with `1_2345`
686687
- Decimal("000_1_2345")
687688
86 + Decimal(1_2345)
688689
87 | Decimal("000_000")
690+
88 |
691+
89 | # Test cases for underscores before sign
689692

690693
FURB157 [*] Verbose expression in `Decimal` constructor
691694
--> FURB157.py:87:9
@@ -694,10 +697,66 @@ FURB157 [*] Verbose expression in `Decimal` constructor
694697
86 | Decimal("000_1_2345")
695698
87 | Decimal("000_000")
696699
| ^^^^^^^^^
700+
88 |
701+
89 | # Test cases for underscores before sign
697702
|
698703
help: Replace with `0`
699704
84 | # Separators _and_ leading zeros
700705
85 | Decimal("0001_2345")
701706
86 | Decimal("000_1_2345")
702707
- Decimal("000_000")
703708
87 + Decimal(0)
709+
88 |
710+
89 | # Test cases for underscores before sign
711+
90 | # https://github.com/astral-sh/ruff/issues/21186
712+
713+
FURB157 [*] Verbose expression in `Decimal` constructor
714+
--> FURB157.py:91:9
715+
|
716+
89 | # Test cases for underscores before sign
717+
90 | # https://github.com/astral-sh/ruff/issues/21186
718+
91 | Decimal("_-1") # Should flag as verbose
719+
| ^^^^^
720+
92 | Decimal("_+1") # Should flag as verbose
721+
93 | Decimal("_-1_000") # Should flag as verbose
722+
|
723+
help: Replace with `-1`
724+
88 |
725+
89 | # Test cases for underscores before sign
726+
90 | # https://github.com/astral-sh/ruff/issues/21186
727+
- Decimal("_-1") # Should flag as verbose
728+
91 + Decimal(-1) # Should flag as verbose
729+
92 | Decimal("_+1") # Should flag as verbose
730+
93 | Decimal("_-1_000") # Should flag as verbose
731+
732+
FURB157 [*] Verbose expression in `Decimal` constructor
733+
--> FURB157.py:92:9
734+
|
735+
90 | # https://github.com/astral-sh/ruff/issues/21186
736+
91 | Decimal("_-1") # Should flag as verbose
737+
92 | Decimal("_+1") # Should flag as verbose
738+
| ^^^^^
739+
93 | Decimal("_-1_000") # Should flag as verbose
740+
|
741+
help: Replace with `+1`
742+
89 | # Test cases for underscores before sign
743+
90 | # https://github.com/astral-sh/ruff/issues/21186
744+
91 | Decimal("_-1") # Should flag as verbose
745+
- Decimal("_+1") # Should flag as verbose
746+
92 + Decimal(+1) # Should flag as verbose
747+
93 | Decimal("_-1_000") # Should flag as verbose
748+
749+
FURB157 [*] Verbose expression in `Decimal` constructor
750+
--> FURB157.py:93:9
751+
|
752+
91 | Decimal("_-1") # Should flag as verbose
753+
92 | Decimal("_+1") # Should flag as verbose
754+
93 | Decimal("_-1_000") # Should flag as verbose
755+
| ^^^^^^^^^
756+
|
757+
help: Replace with `-1_000`
758+
90 | # https://github.com/astral-sh/ruff/issues/21186
759+
91 | Decimal("_-1") # Should flag as verbose
760+
92 | Decimal("_+1") # Should flag as verbose
761+
- Decimal("_-1_000") # Should flag as verbose
762+
93 + Decimal(-1_000) # Should flag as verbose

0 commit comments

Comments
 (0)