Repository: prql/prql
Pull request #390 (2022-04-21 05:42:35)

Simplify string parsing a bit


Patch:
diff --git a/Taskfile.yml b/Taskfile.yml
index 04cc45fd..54935198 100644
--- a/Taskfile.yml
+++ b/Taskfile.yml
@@ -2,7 +2,7 @@
 # assuming they have `cargo` and `Taskfile`.
 
 # Generally we shouldn't be requiring running this to have a good dev loop —
-# rust tools are indpendently good, and adding an intermediate layer means we're
+# rust tools are independently good, and adding an intermediate layer means we're
 # reimplementing things or getting in the way. Instead, this can be used to
 # aggregate commands that's aren't yet integrated.
 
diff --git a/prql-compiler/src/parser.rs b/prql-compiler/src/parser.rs
index bb435a29..fde288bb 100644
--- a/prql-compiler/src/parser.rs
+++ b/prql-compiler/src/parser.rs
@@ -206,12 +206,11 @@ fn ast_of_parse_tree(pairs: Pairs<Rule>) -> Result<Vec<Node>> {
                     })
                 }
                 Rule::ident => Item::Ident(pair.as_str().to_string()),
-                Rule::string_literal => {
-                    ast_of_parse_tree(pair.clone().into_inner())?
-                        .into_only()?
-                        .item
-                }
-                Rule::string => Item::String(pair.as_str().to_string()),
+                Rule::string_literal => Item::String(
+                    // Put the string_inner (which doesn't have quotes) into the
+                    // String item.
+                    pair.into_inner().into_only()?.as_str().to_string(),
+                ),
                 Rule::s_string => Item::SString(ast_of_interpolate_items(pair)?),
                 Rule::f_string => Item::FString(ast_of_interpolate_items(pair)?),
                 Rule::pipeline => Item::Pipeline({
@@ -264,20 +263,17 @@ fn ast_of_parse_tree(pairs: Pairs<Rule>) -> Result<Vec<Node>> {
                     Item::Range(Range { start, end })
                 }
                 Rule::interval => {
-                    let parsed = ast_of_parse_tree(pair.into_inner())?;
-                    // unimplemented!();
-                    let [n, unit]: [Node; 2] = parsed
+                    let parsed: Vec<_> = pair.into_inner().into_iter().collect();
+                    let [n, unit]: [Pair<Rule>; 2] = parsed
                         .try_into()
                         .map_err(|e| anyhow!("Expected two items; {e:?}"))?;
 
                     Item::Interval(Interval {
-                        n: n.item.as_raw().unwrap().parse()?,
-                        unit: unit.item.as_raw().unwrap().clone(),
+                        n: n.as_str().parse()?,
+                        unit: unit.as_str().to_owned(),
                     })
                 }
-                Rule::operator | Rule::number | Rule::interval_kind => {
-                    Item::Raw(pair.as_str().to_owned())
-                }
+                Rule::operator | Rule::number => Item::Raw(pair.as_str().to_owned()),
                 _ => unreachable!(),
             };
 
@@ -512,7 +508,7 @@ mod test {
                 },
                 inner: [
                     Pair {
-                        rule: string,
+                        rule: string_inner,
                         span: Span {
                             str: " U S A ",
                             start: 1,
diff --git a/prql-compiler/src/prql.pest b/prql-compiler/src/prql.pest
index 0f0eafef..15439ff6 100644
--- a/prql-compiler/src/prql.pest
+++ b/prql-compiler/src/prql.pest
@@ -61,7 +61,6 @@ ident = @{ !operator ~ (ASCII_ALPHA | "$") ~ (ASCII_ALPHANUMERIC | "." | "_" | "
 // - 1
 // - 2
 
-// Adding the WHITESPACE is like making it a compound-atomic operator.
 expr = !{ term ~ (operator ~ term)* }
 expr_simple = !{ term_simple ~ (operator ~ term_simple)* }
 
@@ -80,7 +79,7 @@ parenthesized_expr = _{ "(" ~ expr ~ ")" }
 // We need to have a non-silent rule which contains the quotes
 // — `string_literal` in this case — because of
 // https://github.com/pest-parser/pest/issues/583. Then when converting to AST,
-// we only keep the `string` and discard the `string_literal` given it contains
+// we only keep the `string_inner` and discard the `string_literal` given it contains
 // the quotes.
 //
 // TODO: I'm still a bit unclear how preceeding and trailing spaces are working
@@ -90,8 +89,8 @@ parenthesized_expr = _{ "(" ~ expr ~ ")" }
 
 quote = _{ "\"" | "'" }
 // PEEK refers to the opening quote; either `"` or `'`.
-string = { ( !( PEEK ) ~ ANY )+ }
-string_literal = ${ PUSH(quote) ~ string ~ POP }
+string_inner = { ( !( PEEK ) ~ ANY )+ }
+string_literal = ${ PUSH(quote) ~ string_inner ~ POP }
 
 number = ${ ( ASCII_DIGIT )+ ~ ("." ~ ( ASCII_DIGIT )+)? }
 


Test patch:
diff --git a/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_into_parse_tree-4.snap b/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_into_parse_tree-4.snap
index 811e4aa8..2a4c4e4d 100644
--- a/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_into_parse_tree-4.snap
+++ b/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_into_parse_tree-4.snap
@@ -63,7 +63,7 @@ expression: "parse_tree_of_str(r#\"    filter country = \"USA\"\"#, Rule::transf
                                 },
                                 inner: [
                                     Pair {
-                                        rule: string,
+                                        rule: string_inner,
                                         span: Span {
                                             str: "USA",
                                             start: 22,
diff --git a/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_into_parse_tree.snap b/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_into_parse_tree.snap
index 5d67d038..fa5c248d 100644
--- a/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_into_parse_tree.snap
+++ b/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_into_parse_tree.snap
@@ -38,7 +38,7 @@ expression: "parse_tree_of_str(r#\"country = \"USA\"\"#, Rule::expr)?"
                 },
                 inner: [
                     Pair {
-                        rule: string,
+                        rule: string_inner,
                         span: Span {
                             str: "USA",
                             start: 11,
diff --git a/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_pipeline_parse_tree-2.snap b/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_pipeline_parse_tree-2.snap
index af2a6d9f..acf14be6 100644
--- a/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_pipeline_parse_tree-2.snap
+++ b/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_pipeline_parse_tree-2.snap
@@ -1,6 +1,6 @@
 ---
 source: prql-compiler/src/parser.rs
-expression: "parse_tree_of_str(r#\"\n    from employees\n    filter country = \"USA\"\n    \"#.trim(),\n                  Rule::pipeline)?"
+expression: "parse_tree_of_str(r#\"\n    from employees\n    filter country = \"USA\"\n    \"#.trim(),\n        Rule::pipeline)?"
 ---
 [
     Pair {
@@ -119,7 +119,7 @@ expression: "parse_tree_of_str(r#\"\n    from employees\n    filter country = \"
                                         },
                                         inner: [
                                             Pair {
-                                                rule: string,
+                                                rule: string_inner,
                                                 span: Span {
                                                     str: "USA",
                                                     start: 37,
diff --git a/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_pipeline_parse_tree-3.snap b/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_pipeline_parse_tree-3.snap
index 9ed0e2e8..3ebf20b6 100644
--- a/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_pipeline_parse_tree-3.snap
+++ b/prql-compiler/src/snapshots/prql_compiler__parser__test__parse_pipeline_parse_tree-3.snap
@@ -1,6 +1,6 @@
 ---
 source: prql-compiler/src/parser.rs
-expression: "parse_tree_of_str(r#\"\nfrom employees\nfilter country = \"USA\"                           # Each line transforms the previous result.\nderive [                                         # This adds columns / variables.\n  gross_salary: salary + payroll_tax,\n  gross_cost  : gross_salary + benefits_cost    # Variables can use other variables.\n]\nfilter gross_cost > 0\naggregate by:[title, country] [                  # `by` are the columns to group by.\n    average salary,                              # These are aggregation calcs run on each group.\n    sum     salary,\n    average gross_salary,\n    sum     gross_salary,\n    average gross_cost,\n    sum_gross_cost: sum gross_cost,\n    count: count,\n]\nsort sum_gross_cost\nfilter count > 200\ntake 20\n    \"#.trim(),\n                  Rule::pipeline)?"
+expression: "parse_tree_of_str(r#\"\nfrom employees\nfilter country = \"USA\"                           # Each line transforms the previous result.\nderive [                                         # This adds columns / variables.\n  gross_salary: salary + payroll_tax,\n  gross_cost  : gross_salary + benefits_cost    # Variables can use other variables.\n]\nfilter gross_cost > 0\naggregate by:[title, country] [                  # `by` are the columns to group by.\n    average salary,                              # These are aggregation calcs run on each group.\n    sum     salary,\n    average gross_salary,\n    sum     gross_salary,\n    average gross_cost,\n    sum_gross_cost: sum gross_cost,\n    count: count,\n]\nsort sum_gross_cost\nfilter count > 200\ntake 20\n    \"#.trim(),\n        Rule::pipeline)?"
 ---
 [
     Pair {
@@ -119,7 +119,7 @@ expression: "parse_tree_of_str(r#\"\nfrom employees\nfilter country = \"USA\"
                                         },
                                         inner: [
                                             Pair {
-                                                rule: string,
+                                                rule: string_inner,
                                                 span: Span {
                                                     str: "USA",
                                                     start: 33,