Repository: rcoh/angle-grinder
Pull request #153 (2021-08-29 20:54:09)

[lang] add support for logical and/or expressions

tstack: > * `&&` / `||` as aliases

added

> * tests / negative tests for precedence, eg. `a and b and c and d`

I added the above example.  

The original change had the following test for precedence, was there something else you were thinking of?

https://github.com/rcoh/angle-grinder/pull/153/files#diff-9184a440fdb6af807da5a90cdcd5865c62abef73df0a89c2890129ee9e9a011cR2075

And, there's a test for a dangling operator:

https://github.com/rcoh/angle-grinder/pull/153/files#diff-9184a440fdb6af807da5a90cdcd5865c62abef73df0a89c2890129ee9e9a011cR2194

> * What happens if there is a field named `and`? I assume it would need to be escaped? Might be nice to show that as a hint if we get `and`/ `or` in a place we don't expect them

I think one of the benefits of parser combinators is that things like this just kinda work since there's no tokenization step.  I've added this test that calls a function with an argument of `and`:

https://github.com/rcoh/angle-grinder/pull/153/files#diff-9184a440fdb6af807da5a90cdcd5865c62abef73df0a89c2890129ee9e9a011cR2163


Patch:
diff --git a/README.md b/README.md
index 47287da..6e63d94 100644
--- a/README.md
+++ b/README.md
@@ -249,6 +249,7 @@ made up of the following:
   between two date-times, but cannot add them together.
 * `==`, `!=` (or `<>`), `<=`, `>=`, `<`, `>`: Boolean operators work
   on most data types.
+* `and`, `&&`, `or`, `||`: Short-circuiting logical operators.
 * `<field>`: The name of a field in the current row.  If the row does not
   contain the given field, an error will be reported.
 * Parentheses to group operations
diff --git a/src/data.rs b/src/data.rs
index fc370c4..33abc0a 100644
--- a/src/data.rs
+++ b/src/data.rs
@@ -505,6 +505,12 @@ impl Record {
                     found: "arithmetic expr".to_string(),
                 })
             }
+            Expr::Logical(_) => {
+                return Err(EvalError::ExpectedXYZ {
+                    expected: "valid expr".to_string(),
+                    found: "logical expr".to_string(),
+                })
+            }
             Expr::FunctionCall { .. } => {
                 return Err(EvalError::ExpectedXYZ {
                     expected: "valid expr".to_string(),
diff --git a/src/lang.rs b/src/lang.rs
index dece754..27c177b 100644
--- a/src/lang.rs
+++ b/src/lang.rs
@@ -158,10 +158,17 @@ pub enum ArithmeticOp {
     Divide,
 }
 
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub enum LogicalOp {
+    And,
+    Or,
+}
+
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum BinaryOp {
     Comparison(ComparisonOp),
     Arithmetic(ArithmeticOp),
+    Logical(LogicalOp),
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -999,8 +1006,7 @@ fn arith_expr(input: Span) -> IResult<Span, Expr> {
     )(input)
 }
 
-/// Parses an expression, optionally, including a comparison operation
-fn opt_expr(input: Span) -> IResult<Span, Expr> {
+fn cmp_expr(input: Span) -> IResult<Span, Expr> {
     let cmp = map(
         arith_expr.and(opt(pair(
             delimited(multispace0, comp_op, multispace0),
@@ -1022,6 +1028,86 @@ fn opt_expr(input: Span) -> IResult<Span, Expr> {
     cmp.preceded_by(multispace0).parse(input)
 }
 
+fn logical_and(input: Span) -> IResult<Span, Expr> {
+    let (input, init) = cmp_expr(input)?;
+    let qc = &input.extra;
+
+    let retval = fold_many0(
+        alt((
+            pair(
+                multispace1.precedes(with_pos(tag("and"))),
+                opt(multispace1.precedes(cmp_expr)),
+            ),
+            pair(with_pos(tag("&&").delimited_by(multispace0)), opt(cmp_expr)),
+        ))
+        .map(|(pos_and, opt_right)| {
+            if let Some(right) = opt_right {
+                right
+            } else {
+                qc.report_error_for("expecting an operand for 'and' operator")
+                    .with_code_range(pos_and.range, "- dangling 'and'")
+                    .with_resolution("Add the operand or delete the 'and'")
+                    .send_report();
+
+                Expr::Error
+            }
+        }),
+        init,
+        |left, right| Expr::Binary {
+            op: BinaryOp::Logical(LogicalOp::And),
+            left: Box::new(left),
+            right: Box::new(right),
+        },
+    )
+    .parse(input);
+
+    retval
+}
+
+fn logical_or(input: Span) -> IResult<Span, Expr> {
+    let (input, init) = logical_and(input)?;
+    let qc = &input.extra;
+
+    let retval = fold_many0(
+        alt((
+            pair(
+                multispace1.precedes(with_pos(tag("or"))),
+                opt(multispace1.precedes(logical_and)),
+            ),
+            pair(
+                with_pos(tag("||").delimited_by(multispace0)),
+                opt(logical_and),
+            ),
+        ))
+        .map(|(pos_or, opt_right)| {
+            if let Some(right) = opt_right {
+                right
+            } else {
+                qc.report_error_for("expecting an operand for 'or' operator")
+                    .with_code_range(pos_or.range, "- dangling 'or'")
+                    .with_resolution("Add the operand or delete the 'or'")
+                    .send_report();
+
+                Expr::Error
+            }
+        }),
+        init,
+        |left, right| Expr::Binary {
+            op: BinaryOp::Logical(LogicalOp::Or),
+            left: Box::new(left),
+            right: Box::new(right),
+        },
+    )
+    .parse(input);
+
+    retval
+}
+
+/// Parses an expression, optionally, including a comparison operation
+fn opt_expr(input: Span) -> IResult<Span, Expr> {
+    logical_or.preceded_by(multispace0).parse(input)
+}
+
 /// Parses a required expression
 fn expr(input: Span) -> IResult<Span, Expr> {
     Ok(opt_expr(input).unwrap_or_else(|_| {
@@ -1867,6 +1953,259 @@ mod tests {
 
     #[test]
     fn parse_expr() {
+        check_query(
+            "* | k1&&k2&&k3 and k4 as value",
+            expect![[r#"
+            Query {
+                search: And(
+                    [],
+                ),
+                operators: [
+                    Inline(
+                        Positioned {
+                            range: 4..30,
+                            value: FieldExpression {
+                                value: Binary {
+                                    op: Logical(
+                                        And,
+                                    ),
+                                    left: Binary {
+                                        op: Logical(
+                                            And,
+                                        ),
+                                        left: Binary {
+                                            op: Logical(
+                                                And,
+                                            ),
+                                            left: Column {
+                                                head: Key(
+                                                    "k1",
+                                                ),
+                                                rest: [],
+                                            },
+                                            right: Column {
+                                                head: Key(
+                                                    "k2",
+                                                ),
+                                                rest: [],
+                                            },
+                                        },
+                                        right: Column {
+                                            head: Key(
+                                                "k3",
+                                            ),
+                                            rest: [],
+                                        },
+                                    },
+                                    right: Column {
+                                        head: Key(
+                                            "k4",
+                                        ),
+                                        rest: [],
+                                    },
+                                },
+                                name: "value",
+                            },
+                        },
+                    ),
+                ],
+            }
+        "#]],
+        );
+        check_query(
+            "* | k1||k2||k3 or k4 as value",
+            expect![[r#"
+            Query {
+                search: And(
+                    [],
+                ),
+                operators: [
+                    Inline(
+                        Positioned {
+                            range: 4..29,
+                            value: FieldExpression {
+                                value: Binary {
+                                    op: Logical(
+                                        Or,
+                                    ),
+                                    left: Binary {
+                                        op: Logical(
+                                            Or,
+                                        ),
+                                        left: Binary {
+                                            op: Logical(
+                                                Or,
+                                            ),
+                                            left: Column {
+                                                head: Key(
+                                                    "k1",
+                                                ),
+                                                rest: [],
+                                            },
+                                            right: Column {
+                                                head: Key(
+                                                    "k2",
+                                                ),
+                                                rest: [],
+                                            },
+                                        },
+                                        right: Column {
+                                            head: Key(
+                                                "k3",
+                                            ),
+                                            rest: [],
+                                        },
+                                    },
+                                    right: Column {
+                                        head: Key(
+                                            "k4",
+                                        ),
+                                        rest: [],
+                                    },
+                                },
+                                name: "value",
+                            },
+                        },
+                    ),
+                ],
+            }
+        "#]],
+        );
+        check_query(
+            "* | k1 + 0 > 2 or k3 > 4 and k5 < 6 as value",
+            expect![[r#"
+                Query {
+                    search: And(
+                        [],
+                    ),
+                    operators: [
+                        Inline(
+                            Positioned {
+                                range: 4..44,
+                                value: FieldExpression {
+                                    value: Binary {
+                                        op: Logical(
+                                            Or,
+                                        ),
+                                        left: Binary {
+                                            op: Comparison(
+                                                Gt,
+                                            ),
+                                            left: Binary {
+                                                op: Arithmetic(
+                                                    Add,
+                                                ),
+                                                left: Column {
+                                                    head: Key(
+                                                        "k1",
+                                                    ),
+                                                    rest: [],
+                                                },
+                                                right: Value(
+                                                    Int(
+                                                        0,
+                                                    ),
+                                                ),
+                                            },
+                                            right: Value(
+                                                Int(
+                                                    2,
+                                                ),
+                                            ),
+                                        },
+                                        right: Binary {
+                                            op: Logical(
+                                                And,
+                                            ),
+                                            left: Binary {
+                                                op: Comparison(
+                                                    Gt,
+                                                ),
+                                                left: Column {
+                                                    head: Key(
+                                                        "k3",
+                                                    ),
+                                                    rest: [],
+                                                },
+                                                right: Value(
+                                                    Int(
+                                                        4,
+                                                    ),
+                                                ),
+                                            },
+                                            right: Binary {
+                                                op: Comparison(
+                                                    Lt,
+                                                ),
+                                                left: Column {
+                                                    head: Key(
+                                                        "k5",
+                                                    ),
+                                                    rest: [],
+                                                },
+                                                right: Value(
+                                                    Int(
+                                                        6,
+                                                    ),
+                                                ),
+                                            },
+                                        },
+                                    },
+                                    name: "value",
+                                },
+                            },
+                        ),
+                    ],
+                }
+            "#]],
+        );
+        check_query(
+            "* | lowercase(and) as value",
+            expect![[r#"
+            Query {
+                search: And(
+                    [],
+                ),
+                operators: [
+                    Inline(
+                        Positioned {
+                            range: 4..27,
+                            value: FieldExpression {
+                                value: FunctionCall {
+                                    name: "lowercase",
+                                    args: [
+                                        Column {
+                                            head: Key(
+                                                "and",
+                                            ),
+                                            rest: [],
+                                        },
+                                    ],
+                                },
+                                name: "value",
+                            },
+                        },
+                    ),
+                ],
+            }
+        "#]],
+        );
+        check_query(
+            "* | if(abc and, abc, def) as value",
+            expect![[r#"
+            Query {
+                search: And(
+                    [],
+                ),
+                operators: [],
+            }
+            error: expecting an operand for 'and' operator
+              |
+            1 | * | if(abc and, abc, def) as value
+              |            ^^^ - dangling 'and'
+              |
+              = help: Add the operand or delete the 'and'"#]],
+        );
         check_query(
             "* | if() as i",
             expect![[r#"
diff --git a/src/operator.rs b/src/operator.rs
index cbfd271..f7a3036 100644
--- a/src/operator.rs
+++ b/src/operator.rs
@@ -232,6 +232,7 @@ pub enum Expr {
     BoolUnary(UnaryExpr<BoolUnaryExpr>),
     Comparison(BinaryExpr<BoolExpr>),
     Arithmetic(BinaryExpr<ArithmeticExpr>),
+    Logical(BinaryExpr<LogicalExpr>),
     FunctionCall {
         func: &'static funcs::FunctionContainer,
         args: Vec<Expr>,
@@ -289,6 +290,12 @@ pub enum ArithmeticExpr {
     Divide,
 }
 
+#[derive(Clone, Debug)]
+pub enum LogicalExpr {
+    And,
+    Or,
+}
+
 impl<T: Copy + Send + Sync> Evaluatable<T> for T {
     fn eval(&self, _record: &HashMap<String, data::Value>) -> Result<T, EvalError> {
         Ok(*self)
@@ -324,6 +331,28 @@ impl Evaluatable<data::Value> for BinaryExpr<ArithmeticExpr> {
     }
 }
 
+impl Evaluatable<data::Value> for BinaryExpr<LogicalExpr> {
+    fn eval(&self, record: &HashMap<String, data::Value>) -> Result<data::Value, EvalError> {
+        let l: bool = self.left.eval(record)?;
+        match self.operator {
+            LogicalExpr::And => {
+                if l {
+                    self.right.eval(record)
+                } else {
+                    Ok(data::Value::Bool(false))
+                }
+            }
+            LogicalExpr::Or => {
+                if l {
+                    Ok(data::Value::Bool(true))
+                } else {
+                    self.right.eval(record)
+                }
+            }
+        }
+    }
+}
+
 impl Evaluatable<bool> for UnaryExpr<BoolUnaryExpr> {
     fn eval(&self, record: &HashMap<String, data::Value>) -> Result<bool, EvalError> {
         let bool_res: data::Value = self.operand.eval(record)?;
@@ -407,6 +436,7 @@ impl Evaluatable<data::Value> for Expr {
                 Ok(data::Value::from_bool(bool_res))
             }
             Expr::Arithmetic(ref binary_expr) => binary_expr.eval(record),
+            Expr::Logical(ref logical_expr) => logical_expr.eval(record),
             Expr::FunctionCall { ref func, ref args } => {
                 let evaluated_args: Result<Vec<data::Value>, EvalError> =
                     args.into_iter().map(|expr| expr.eval(record)).collect();
diff --git a/src/typecheck.rs b/src/typecheck.rs
index f015d10..6b98f14 100644
--- a/src/typecheck.rs
+++ b/src/typecheck.rs
@@ -66,6 +66,18 @@ impl TypeCheck<operator::ArithmeticExpr> for lang::ArithmeticOp {
     }
 }
 
+impl TypeCheck<operator::LogicalExpr> for lang::LogicalOp {
+    fn type_check<E: ErrorBuilder>(
+        self,
+        _error_builder: &E,
+    ) -> Result<operator::LogicalExpr, TypeError> {
+        match self {
+            lang::LogicalOp::And => Ok(operator::LogicalExpr::And),
+            lang::LogicalOp::Or => Ok(operator::LogicalExpr::Or),
+        }
+    }
+}
+
 impl TypeCheck<operator::Expr> for lang::Expr {
     fn type_check<E: ErrorBuilder>(self, error_builder: &E) -> Result<operator::Expr, TypeError> {
         match self {
@@ -109,6 +121,15 @@ impl TypeCheck<operator::Expr> for lang::Expr {
                         operator: arith_op.type_check(error_builder)?,
                     }))
                 }
+                lang::BinaryOp::Logical(logical_op) => {
+                    Ok(operator::Expr::Logical(operator::BinaryExpr::<
+                        operator::LogicalExpr,
+                    > {
+                        left: Box::new((*left).type_check(error_builder)?),
+                        right: Box::new((*right).type_check(error_builder)?),
+                        operator: logical_op.type_check(error_builder)?,
+                    }))
+                }
             },
             lang::Expr::FunctionCall { name, args } => {
                 let converted_args: Result<Vec<operator::Expr>, TypeError> = args


Test patch:
diff --git a/tests/structured_tests/logical-expr-1.toml b/tests/structured_tests/logical-expr-1.toml
new file mode 100644
index 0000000..b5b533a
--- /dev/null
+++ b/tests/structured_tests/logical-expr-1.toml
@@ -0,0 +1,11 @@
+query = """* | json | if(!isNull(a), a, b) as value"""
+input = """
+{"a": null, "b": "goodbye"}
+{"a": "hello", "b": "abc"}
+{"a": "world", "b": "def"}
+"""
+output = """
+[a=None]        [b=goodbye]        [value=goodbye]
+[a=hello]       [b=abc]            [value=hello]
+[a=world]       [b=def]            [value=world]
+"""
diff --git a/tests/structured_tests/logical-expr-2.toml b/tests/structured_tests/logical-expr-2.toml
new file mode 100644
index 0000000..0bbae4a
--- /dev/null
+++ b/tests/structured_tests/logical-expr-2.toml
@@ -0,0 +1,9 @@
+query = """* | json | where a and b"""
+input = """
+{"a": false}
+{"a": true, "b": false}
+{"a": true, "b": true}
+"""
+output = """
+[a=true]        [b=true]
+"""