Merge pull request #4116 from charliermarsh/charlie/f-string

Avoid creating unused JoinedStr in FStringParser

parser/src/fstring.rs

@@ -4,6 +4,7 @@ use crate::{
     error::{FStringError, FStringErrorType, ParseError},
     parser::parse_expression,
 };
+use itertools::Itertools;
 use std::{iter, mem, str};
 
 struct FStringParser<'a> {
@@ -105,19 +106,16 @@ impl<'a> FStringParser<'a> {
                                 nested -= 1;
                                 if nested == 0 {
                                     formatted_value_piece.push(next);
-                                    spec_constructor.push(
+                                    let values = FStringParser::new(
-                                        self.expr(ExprKind::FormattedValue {
-                                            value: Box::new(
-                                                FStringParser::new(
                                         &formatted_value_piece,
                                         Location::default(),
                                         &self.recurse_lvl + 1,
                                     )
-                                                .parse()?,
+                                    .parse()?;
-                                            ),
+                                    spec_constructor.push(values
-                                            conversion: ConversionFlag::None as _,
+                                        .into_iter()
-                                            format_spec: None,
+                                        .exactly_one()
-                                        }),
+                                        .expect("Expected formatted value to produce exactly one expression.")
                                     );
                                     formatted_value_piece.clear();
                                 } else {
@@ -129,11 +127,13 @@ impl<'a> FStringParser<'a> {
                             }
                             '{' => {
                                 nested += 1;
+                                if !constant_piece.is_empty() {
                                     spec_constructor.push(self.expr(ExprKind::Constant {
                                         value: constant_piece.to_owned().into(),
                                         kind: None,
                                     }));
                                     constant_piece.clear();
+                                }
                                 formatted_value_piece.push(next);
                                 formatted_value_piece.push(' ');
                             }
@@ -144,11 +144,13 @@ impl<'a> FStringParser<'a> {
                         }
                         self.chars.next();
                     }
+                    if !constant_piece.is_empty() {
                         spec_constructor.push(self.expr(ExprKind::Constant {
                             value: constant_piece.to_owned().into(),
                             kind: None,
                         }));
                         constant_piece.clear();
+                    }
                     if nested > 0 {
                         return Err(UnclosedLbrace);
                     }
@@ -241,7 +243,7 @@ impl<'a> FStringParser<'a> {
         Err(UnclosedLbrace)
     }
 
-    fn parse(mut self) -> Result<Expr, FStringErrorType> {
+    fn parse(mut self) -> Result<Vec<Expr>, FStringErrorType> {
         if self.recurse_lvl >= 2 {
             return Err(ExpressionNestedTooDeeply);
         }
@@ -287,7 +289,7 @@ impl<'a> FStringParser<'a> {
             }))
         }
 
-        Ok(self.expr(ExprKind::JoinedStr { values }))
+        Ok(values)
     }
 }
 
@@ -298,7 +300,7 @@ fn parse_fstring_expr(source: &str) -> Result<Expr, ParseError> {
 
 /// Parse an fstring from a string, located at a certain position in the sourcecode.
 /// In case of errors, we will get the location and the error returned.
-pub fn parse_located_fstring(source: &str, location: Location) -> Result<Expr, FStringError> {
+pub fn parse_located_fstring(source: &str, location: Location) -> Result<Vec<Expr>, FStringError> {
     FStringParser::new(source, location, 0)
         .parse()
         .map_err(|error| FStringError { error, location })
@@ -308,7 +310,7 @@ pub fn parse_located_fstring(source: &str, location: Location) -> Result<Expr, F
 mod tests {
     use super::*;
 
-    fn parse_fstring(source: &str) -> Result<Expr, FStringErrorType> {
+    fn parse_fstring(source: &str) -> Result<Vec<Expr>, FStringErrorType> {
         FStringParser::new(source, Location::default(), 0).parse()
     }
 

parser/src/snapshots/rustpython_parser__fstring__tests__fstring_parse_selfdocumenting_base.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -58,6 +51,4 @@ Located {
             format_spec: None,
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__fstring_parse_selfdocumenting_base_more.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -132,6 +125,4 @@ Located {
             format_spec: None,
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__fstring_parse_selfdocumenting_format.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -83,6 +76,4 @@ Located {
             ),
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_empty_fstring.snap

@@ -1,15 +1,5 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: "parse_fstring(\"\").unwrap()"
-
 ---
-Located {
+[]
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [],
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -67,6 +60,4 @@ Located {
             kind: None,
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_equals.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -61,6 +54,4 @@ Located {
             format_spec: None,
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_nested_spec.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -31,34 +24,6 @@ Located {
             conversion: 0,
             format_spec: Some(
                 Located {
-                            location: Location {
-                                row: 0,
-                                column: 0,
-                            },
-                            custom: (),
-                            node: JoinedStr {
-                                values: [
-                                    Located {
-                                        location: Location {
-                                            row: 0,
-                                            column: 0,
-                                        },
-                                        custom: (),
-                                        node: Constant {
-                                            value: Str(
-                                                "",
-                                            ),
-                                            kind: None,
-                                        },
-                                    },
-                                    Located {
-                                        location: Location {
-                                            row: 0,
-                                            column: 0,
-                                        },
-                                        custom: (),
-                                        node: FormattedValue {
-                                            value: Located {
                     location: Location {
                         row: 0,
                         column: 0,
@@ -91,29 +56,7 @@ Located {
                         ],
                     },
                 },
-                                            conversion: 0,
-                                            format_spec: None,
-                                        },
-                                    },
-                                    Located {
-                                        location: Location {
-                                            row: 0,
-                                            column: 0,
-                                        },
-                                        custom: (),
-                                        node: Constant {
-                                            value: Str(
-                                                "",
-                                            ),
-                                            kind: None,
-                                        },
-                                    },
-                                ],
-                            },
-                        },
             ),
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_not_equals.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -61,6 +54,4 @@ Located {
             format_spec: None,
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_not_nested_spec.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -57,6 +50,4 @@ Located {
             ),
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_selfdoc_prec_space.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -58,6 +51,4 @@ Located {
             format_spec: None,
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_selfdoc_trailing_space.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -58,6 +51,4 @@ Located {
             format_spec: None,
         },
     },
-        ],
+]
-    },
-}

parser/src/snapshots/rustpython_parser__fstring__tests__parse_fstring_yield_expr.snap

@@ -1,15 +1,8 @@
 ---
-source: parser/src/fstring.rs
+source: compiler/parser/src/fstring.rs
 expression: parse_ast
 ---
-Located {
+[
-    location: Location {
-        row: 0,
-        column: 0,
-    },
-    custom: (),
-    node: JoinedStr {
-        values: [
     Located {
         location: Location {
             row: 0,
@@ -31,6 +24,4 @@ Located {
             format_spec: None,
         },
     },
-        ],
+]
-    },
-}

parser/src/string.rs

@@ -34,19 +34,10 @@ pub fn parse_strings(values: Vec<(Location, (String, StringKind))>) -> Result<Ex
             StringKind::Normal | StringKind::U => current.push(string),
             StringKind::F => {
                 has_fstring = true;
-                let values = if let ExprKind::JoinedStr { values } =
+                for value in parse_located_fstring(&string, location).map_err(|e| LexicalError {
-                    parse_located_fstring(&string, location)
-                        .map_err(|e| LexicalError {
                     location,
                     error: LexicalErrorType::FStringError(e.error),
-                        })?
+                })? {
-                        .node
-                {
-                    values
-                } else {
-                    unreachable!("parse_located_fstring returned a non-JoinedStr.")
-                };
-                for value in values {
                     match value.node {
                         ExprKind::FormattedValue { .. } => {
                             if !current.is_empty() {