Fix precedence parsing of parens. Limit use (#1606)

crates/nu-cli/tests/commands/math.rs

@@ -131,3 +131,15 @@ fn compound_where() {
 
     assert_eq!(actual, r#"{"a":2,"b":1}"#);
 }
+
+#[test]
+fn compound_where_paren() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+            echo '[{"a": 1, "b": 1}, {"a": 2, "b": 1}, {"a": 2, "b": 2}]' | from-json | where (a == 2 && b == 1) || b == 2 | to-json
+        "#
+    ));
+
+    assert_eq!(actual, r#"[{"a":2,"b":1},{"a":2,"b":2}]"#);
+}

crates/nu-parser/src/parse.rs

@@ -395,7 +395,6 @@ fn parse_arg(
                 SyntaxShape::Unit,
                 SyntaxShape::Block,
                 SyntaxShape::Table,
-                SyntaxShape::Parenthesized,
                 SyntaxShape::String,
             ];
             for shape in shapes.iter() {
@@ -453,34 +452,6 @@ fn parse_arg(
                 ),
             }
         }
-        SyntaxShape::Parenthesized => {
-            let mut chars = lite_arg.item.chars();
-
-            match (chars.next(), chars.next_back()) {
-                (Some('('), Some(')')) => {
-                    // We have a literal row
-                    let string: String = chars.collect();
-
-                    // We haven't done much with the inner string, so let's go ahead and work with it
-                    let mut lite_pipeline = match lite_parse(&string, lite_arg.span.start() + 1) {
-                        Ok(lp) => lp,
-                        Err(e) => return (garbage(lite_arg.span), Some(e)),
-                    };
-
-                    let mut collection = vec![];
-                    for lite_cmd in lite_pipeline.commands.iter_mut() {
-                        collection.push(lite_cmd.name.clone());
-                        collection.append(&mut lite_cmd.args);
-                    }
-                    let (_, expr, err) = parse_math_expression(0, &collection[..], registry, false);
-                    (expr, err)
-                }
-                _ => (
-                    garbage(lite_arg.span),
-                    Some(ParseError::mismatch("table", lite_arg.clone())),
-                ),
-            }
-        }
         SyntaxShape::Block | SyntaxShape::Math => {
             // Blocks have one of two forms: the literal block and the implied block
             // To parse a literal block, we need to detect that what we have is itself a block
@@ -603,6 +574,57 @@ fn shorthand_reparse(
     }
 }
 
+fn parse_parenthesized_expression(
+    lite_arg: &Spanned<String>,
+    registry: &dyn SignatureRegistry,
+    shorthand_mode: bool,
+) -> (SpannedExpression, Option<ParseError>) {
+    let mut chars = lite_arg.item.chars();
+
+    match (chars.next(), chars.next_back()) {
+        (Some('('), Some(')')) => {
+            // We have a literal row
+            let string: String = chars.collect();
+
+            // We haven't done much with the inner string, so let's go ahead and work with it
+            let mut lite_pipeline = match lite_parse(&string, lite_arg.span.start() + 1) {
+                Ok(lp) => lp,
+                Err(e) => return (garbage(lite_arg.span), Some(e)),
+            };
+
+            let mut collection = vec![];
+            for lite_cmd in lite_pipeline.commands.iter_mut() {
+                collection.push(lite_cmd.name.clone());
+                collection.append(&mut lite_cmd.args);
+            }
+            let (_, expr, err) =
+                parse_math_expression(0, &collection[..], registry, shorthand_mode);
+            (expr, err)
+        }
+        _ => (
+            garbage(lite_arg.span),
+            Some(ParseError::mismatch("table", lite_arg.clone())),
+        ),
+    }
+}
+
+fn parse_possibly_parenthesized(
+    lite_arg: &Spanned<String>,
+    registry: &dyn SignatureRegistry,
+    shorthand_mode: bool,
+) -> (
+    (Option<Spanned<String>>, SpannedExpression),
+    Option<ParseError>,
+) {
+    if lite_arg.item.starts_with('(') {
+        let (lhs, err) = parse_parenthesized_expression(lite_arg, registry, shorthand_mode);
+        ((None, lhs), err)
+    } else {
+        let (lhs, err) = parse_arg(SyntaxShape::Any, registry, lite_arg);
+        ((Some(lite_arg.clone()), lhs), err)
+    }
+}
+
 /// Handle parsing math expressions, complete with working with the precedence of the operators
 fn parse_math_expression(
     incoming_idx: usize,
@@ -622,12 +644,14 @@ fn parse_math_expression(
     let mut working_exprs = vec![];
     let mut prec = vec![];
 
-    let (lhs, err) = parse_arg(SyntaxShape::Any, registry, &lite_args[idx]);
+    let (lhs_working_expr, err) =
+        parse_possibly_parenthesized(&lite_args[idx], registry, shorthand_mode);
 
     if error.is_none() {
         error = err;
     }
-    working_exprs.push((Some(lite_args[idx].clone()), lhs));
+    working_exprs.push(lhs_working_expr);
+
     idx += 1;
 
     prec.push(0);
@@ -646,7 +670,10 @@ fn parse_math_expression(
                 working_exprs,
                 prec
             );
-            let (rhs, err) = parse_arg(SyntaxShape::Any, registry, &lite_args[idx]);
+
+            let (rhs_working_expr, err) =
+                parse_possibly_parenthesized(&lite_args[idx], registry, shorthand_mode);
+
             if error.is_none() {
                 error = err;
             }
@@ -656,7 +683,7 @@ fn parse_math_expression(
             if !prec.is_empty() && next_prec > *prec.last().expect("this shouldn't happen") {
                 prec.push(next_prec);
                 working_exprs.push((None, op));
-                working_exprs.push((Some(lite_args[idx].clone()), rhs));
+                working_exprs.push(rhs_working_expr);
             } else {
                 while !prec.is_empty()
                     && *prec.last().expect("This shouldn't happen") >= next_prec
@@ -692,7 +719,7 @@ fn parse_math_expression(
                     prec.pop();
                 }
                 working_exprs.push((None, op));
-                working_exprs.push((Some(lite_args[idx].clone()), rhs));
+                working_exprs.push(rhs_working_expr);
             }
 
             idx += 1;

crates/nu-protocol/src/syntax_shape.rs

@@ -30,8 +30,6 @@ pub enum SyntaxShape {
     Unit,
     /// An operator
     Operator,
-    /// A parenthesized math expression, eg `(1 + 3)`
-    Parenthesized,
     /// A math expression, eg `foo > 1`
     Math,
 }
@@ -53,7 +51,6 @@ impl PrettyDebug for SyntaxShape {
             SyntaxShape::Table => "table",
             SyntaxShape::Unit => "unit",
             SyntaxShape::Operator => "operator",
-            SyntaxShape::Parenthesized => "math with parentheses",
             SyntaxShape::Math => "condition",
         })
     }