Skip to content

Commit 37e8842

Browse files
youknowoneyouknowone
authored
Merge pull request #3532 from fanninpm/fix-fstring-format-spec
Fix f-string format spec
2 parents ff7716b + a936c36 commit 37e8842

File tree

5 files changed

+173
-59
lines changed

5 files changed

+173
-59
lines changed

‎extra_tests/snippets/fstrings.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@
4747
spec = "0>+#10x"
4848
assert f"{16:{spec}}{foo}" == '00000+0x10bar'
4949

50+
part_spec = ">+#10x"
51+
assert f"{16:0{part_spec}}{foo}" == '00000+0x10bar'
52+
53+
# TODO: RUSTPYTHON, delete the next block once `test_fstring.py` can successfully parse
54+
assert f'{10:#{1}0x}' == ' 0xa'
55+
assert f'{10:{"#"}1{0}{"x"}}' == ' 0xa'
56+
assert f'{-10:-{"#"}1{0}x}' == ' -0xa'
57+
assert f'{-10:{"-"}#{1}0{"x"}}' == ' -0xa'
58+
5059
# TODO:
5160
# spec = "bla"
5261
# assert_raises(ValueError, lambda: f"{16:{spec}}")

‎parser/src/fstring.rs‎

Lines changed: 60 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@ use self::FStringErrorType::*;
1111
struct FStringParser<'a> {
1212
chars: iter::Peekable<str::Chars<'a>>,
1313
str_location: Location,
14+
recurse_lvl: u8,
1415
}
1516

1617
impl<'a> FStringParser<'a> {
17-
fn new(source: &'a str, str_location: Location) -> Self {
18+
fn new(source: &'a str, str_location: Location, recurse_lvl: u8) -> Self {
1819
Self {
1920
chars: source.chars().peekable(),
2021
str_location,
22+
recurse_lvl,
2123
}
2224
}
2325

@@ -91,52 +93,69 @@ impl<'a> FStringParser<'a> {
9193
}
9294

9395
':' if delims.is_empty() => {
94-
let mut nested = false;
9596
let mut in_nested = false;
96-
let mut spec_expression = String::new();
97+
let mut spec_constructor = Vec::new();
98+
let mut constant_piece = String::new();
99+
let mut formatted_value_piece = String::new();
100+
let mut spec_delims = Vec::new();
97101
while let Some(&next) = self.chars.peek() {
98102
match next {
99-
'{' => {
100-
if in_nested {
101-
return Err(ExpressionNestedTooDeeply);
102-
}
103-
in_nested = true;
104-
nested = true;
105-
self.chars.next();
106-
continue;
103+
'{' if in_nested => {
104+
spec_delims.push(next);
105+
formatted_value_piece.push(next);
107106
}
108-
'}' => {
109-
if in_nested {
107+
'}' if in_nested => {
108+
if spec_delims.is_empty() {
110109
in_nested = false;
111-
self.chars.next();
110+
spec_constructor.push(
111+
self.expr(ExprKind::FormattedValue {
112+
value: Box::new(
113+
FStringParser::new(
114+
&format!("{{{}}}", formatted_value_piece),
115+
Location::default(),
116+
&self.recurse_lvl + 1,
117+
)
118+
.parse()?,
119+
),
120+
conversion: None,
121+
format_spec: None,
122+
}),
123+
);
124+
formatted_value_piece.clear();
125+
} else {
126+
spec_delims.pop();
127+
formatted_value_piece.push(next);
112128
}
113-
break;
114129
}
115-
_ => (),
130+
_ if in_nested => {
131+
formatted_value_piece.push(next);
132+
}
133+
'{' => {
134+
in_nested = true;
135+
spec_constructor.push(self.expr(ExprKind::Constant {
136+
value: constant_piece.to_owned().into(),
137+
kind: None,
138+
}));
139+
constant_piece.clear();
140+
}
141+
'}' => break,
142+
_ => {
143+
constant_piece.push(next);
144+
}
116145
}
117-
spec_expression.push(next);
118146
self.chars.next();
119147
}
148+
spec_constructor.push(self.expr(ExprKind::Constant {
149+
value: constant_piece.to_owned().into(),
150+
kind: None,
151+
}));
152+
constant_piece.clear();
120153
if in_nested {
121154
return Err(UnclosedLbrace);
122155
}
123-
spec = Some(if nested {
124-
Box::new(
125-
self.expr(ExprKind::FormattedValue {
126-
value: Box::new(
127-
parse_fstring_expr(&spec_expression)
128-
.map_err(|e| InvalidExpression(Box::new(e.error)))?,
129-
),
130-
conversion: None,
131-
format_spec: None,
132-
}),
133-
)
134-
} else {
135-
Box::new(self.expr(ExprKind::Constant {
136-
value: spec_expression.to_owned().into(),
137-
kind: None,
138-
}))
139-
})
156+
spec = Some(Box::new(self.expr(ExprKind::JoinedStr {
157+
values: spec_constructor,
158+
})))
140159
}
141160
'(' | '{' | '[' => {
142161
expression.push(ch);
@@ -216,6 +235,10 @@ impl<'a> FStringParser<'a> {
216235
}
217236

218237
fn parse(mut self) -> Result<Expr, FStringErrorType> {
238+
if self.recurse_lvl >= 2 {
239+
return Err(ExpressionNestedTooDeeply);
240+
}
241+
219242
let mut content = String::new();
220243
let mut values = vec![];
221244

@@ -269,7 +292,7 @@ fn parse_fstring_expr(source: &str) -> Result<Expr, ParseError> {
269292
/// Parse an fstring from a string, located at a certain position in the sourcecode.
270293
/// In case of errors, we will get the location and the error returned.
271294
pub fn parse_located_fstring(source: &str, location: Location) -> Result<Expr, FStringError> {
272-
FStringParser::new(source, location)
295+
FStringParser::new(source, location, 0)
273296
.parse()
274297
.map_err(|error| FStringError { error, location })
275298
}
@@ -279,7 +302,7 @@ mod tests {
279302
use super::*;
280303

281304
fn parse_fstring(source: &str) -> Result<Expr, FStringErrorType> {
282-
FStringParser::new(source, Location::default()).parse()
305+
FStringParser::new(source, Location::default(), 0).parse()
283306
}
284307

285308
#[test]
@@ -347,7 +370,7 @@ mod tests {
347370
assert_eq!(parse_fstring("{5!}"), Err(InvalidConversionFlag));
348371
assert_eq!(parse_fstring("{5!x}"), Err(InvalidConversionFlag));
349372

350-
assert_eq!(parse_fstring("{a:{a:{b}}"), Err(ExpressionNestedTooDeeply));
373+
assert_eq!(parse_fstring("{a:{a:{b}}}"), Err(ExpressionNestedTooDeeply));
351374

352375
assert_eq!(parse_fstring("{a:b}}"), Err(UnopenedRbrace));
353376
assert_eq!(parse_fstring("}"), Err(UnopenedRbrace));

‎parser/src/snapshots/rustpython_parser__fstring__tests__fstring_parse_selfdocumenting_format.snap‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
---
22
source: parser/src/fstring.rs
33
expression: parse_ast
4+
45
---
56
Located {
67
location: Location {
@@ -62,11 +63,22 @@ Located {
6263
column: 0,
6364
},
6465
custom: (),
65-
node: Constant {
66-
value: Str(
67-
">10",
68-
),
69-
kind: None,
66+
node: JoinedStr {
67+
values: [
68+
Located {
69+
location: Location {
70+
row: 0,
71+
column: 0,
72+
},
73+
custom: (),
74+
node: Constant {
75+
value: Str(
76+
">10",
77+
),
78+
kind: None,
79+
},
80+
},
81+
],
7082
},
7183
},
7284
),

‎parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_nested_spec.snap‎

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,79 @@ Located {
3737
column: 0,
3838
},
3939
custom: (),
40-
node: FormattedValue {
41-
value: Located {
42-
location: Location {
43-
row: 1,
44-
column: 2,
40+
node: JoinedStr {
41+
values: [
42+
Located {
43+
location: Location {
44+
row: 0,
45+
column: 0,
46+
},
47+
custom: (),
48+
node: Constant {
49+
value: Str(
50+
"",
51+
),
52+
kind: None,
53+
},
4554
},
46-
custom: (),
47-
node: Name {
48-
id: "spec",
49-
ctx: Load,
55+
Located {
56+
location: Location {
57+
row: 0,
58+
column: 0,
59+
},
60+
custom: (),
61+
node: FormattedValue {
62+
value: Located {
63+
location: Location {
64+
row: 0,
65+
column: 0,
66+
},
67+
custom: (),
68+
node: JoinedStr {
69+
values: [
70+
Located {
71+
location: Location {
72+
row: 0,
73+
column: 0,
74+
},
75+
custom: (),
76+
node: FormattedValue {
77+
value: Located {
78+
location: Location {
79+
row: 1,
80+
column: 2,
81+
},
82+
custom: (),
83+
node: Name {
84+
id: "spec",
85+
ctx: Load,
86+
},
87+
},
88+
conversion: None,
89+
format_spec: None,
90+
},
91+
},
92+
],
93+
},
94+
},
95+
conversion: None,
96+
format_spec: None,
97+
},
5098
},
51-
},
52-
conversion: None,
53-
format_spec: None,
99+
Located {
100+
location: Location {
101+
row: 0,
102+
column: 0,
103+
},
104+
custom: (),
105+
node: Constant {
106+
value: Str(
107+
"",
108+
),
109+
kind: None,
110+
},
111+
},
112+
],
54113
},
55114
},
56115
),

‎parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_not_nested_spec.snap‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,22 @@ Located {
3737
column: 0,
3838
},
3939
custom: (),
40-
node: Constant {
41-
value: Str(
42-
"spec",
43-
),
44-
kind: None,
40+
node: JoinedStr {
41+
values: [
42+
Located {
43+
location: Location {
44+
row: 0,
45+
column: 0,
46+
},
47+
custom: (),
48+
node: Constant {
49+
value: Str(
50+
"spec",
51+
),
52+
kind: None,
53+
},
54+
},
55+
],
4556
},
4657
},
4758
),

0 commit comments

Comments
 (0)