Skip to content

Commit d407165

Browse files
authored
Fix formatter panic with comment after parenthesized dict value (#5293)
## Summary This snippet used to panic because it expected to see a comma or something similar after the `2` but met the closing parentheses that is not part of the range and panicked ```python a = { 1: (2), # comment 3: True, } ``` Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109 This snippet is also the test plan.
1 parent f7e1cf4 commit d407165

File tree

3 files changed

+40
-8
lines changed

3 files changed

+40
-8
lines changed

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py

+8
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,11 @@
4848
C: 0.1 * (10.0 / 12),
4949
D: 0.1 * (10.0 / 12),
5050
}
51+
52+
# Regression test for formatter panic with comment after parenthesized dict value
53+
# Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109
54+
a = {
55+
1: (2),
56+
# comment
57+
3: True,
58+
}

crates/ruff_python_formatter/src/comments/placement.rs

+16-8
Original file line numberDiff line numberDiff line change
@@ -991,14 +991,22 @@ fn handle_dict_unpacking_comment<'a>(
991991
.skip_trivia();
992992

993993
// we start from the preceding node but we skip its token
994-
if let Some(first) = tokens.next() {
995-
debug_assert!(matches!(
996-
first,
997-
Token {
998-
kind: TokenKind::LBrace | TokenKind::Comma | TokenKind::Colon,
999-
..
1000-
}
1001-
));
994+
for token in tokens.by_ref() {
995+
// Skip closing parentheses that are not part of the node range
996+
if token.kind == TokenKind::RParen {
997+
continue;
998+
}
999+
debug_assert!(
1000+
matches!(
1001+
token,
1002+
Token {
1003+
kind: TokenKind::LBrace | TokenKind::Comma | TokenKind::Colon,
1004+
..
1005+
}
1006+
),
1007+
"{token:?}",
1008+
);
1009+
break;
10021010
}
10031011

10041012
// if the remaining tokens from the previous node is exactly `**`,

crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__dict_py.snap

+16
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@ mapping = {
5454
C: 0.1 * (10.0 / 12),
5555
D: 0.1 * (10.0 / 12),
5656
}
57+
58+
# Regression test for formatter panic with comment after parenthesized dict value
59+
# Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109
60+
a = {
61+
1: (2),
62+
# comment
63+
3: True,
64+
}
5765
```
5866

5967

@@ -112,6 +120,14 @@ mapping = {
112120
C: 0.1 * (10.0 / 12),
113121
D: 0.1 * (10.0 / 12),
114122
}
123+
124+
# Regression test for formatter panic with comment after parenthesized dict value
125+
# Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109
126+
a = {
127+
1: (2),
128+
# comment
129+
3: True,
130+
}
115131
```
116132

117133

0 commit comments

Comments
 (0)