From 1372b3934105ae57a7bc2e34999f0a2aeb5d1733 Mon Sep 17 00:00:00 2001 From: Benjamin Tan Date: Wed, 6 Nov 2024 21:18:34 +0800 Subject: [PATCH] template: add support for logical equality operator --- CHANGELOG.md | 3 ++ cli/src/template.pest | 3 +- cli/src/template_builder.rs | 103 ++++++++++++++++++++++++++++++++++-- cli/src/template_parser.rs | 21 ++++++-- cli/tests/test_templater.rs | 6 +-- docs/templates.md | 2 + 6 files changed, 127 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 642eb46ba..ca5a9e48f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### New features +* Templates now support the `==` logical operator for `Boolean`, `Integer`, and + `String` types. + ### Fixed bugs ## [0.23.0] - 2024-11-06 diff --git a/cli/src/template.pest b/cli/src/template.pest index c69087b1e..83f8a6ed3 100644 --- a/cli/src/template.pest +++ b/cli/src/template.pest @@ -40,10 +40,11 @@ identifier = @{ (ASCII_ALPHA | "_") ~ (ASCII_ALPHANUMERIC | "_")* } concat_op = { "++" } logical_or_op = { "||" } logical_and_op = { "&&" } +logical_eq_op = { "==" } logical_not_op = { "!" } negate_op = { "-" } prefix_ops = _{ logical_not_op | negate_op } -infix_ops = _{ logical_or_op | logical_and_op } +infix_ops = _{ logical_or_op | logical_and_op | logical_eq_op } function = { identifier ~ "(" ~ whitespace* ~ function_arguments ~ whitespace* ~ ")" } keyword_argument = { identifier ~ whitespace* ~ "=" ~ whitespace* ~ template } diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs index bd7c2ea4f..3444cb438 100644 --- a/cli/src/template_builder.rs +++ b/cli/src/template_builder.rs @@ -605,6 +605,41 @@ fn build_binary_operation<'a, L: TemplateLanguage<'a> + ?Sized>( let out = lhs.and_then(move |l| Ok(l && rhs.extract()?)); Ok(L::wrap_boolean(out)) } + BinaryOp::LogicalEq => { + let lhs = build_expression(language, diagnostics, build_ctx, lhs_node)?; + let rhs = build_expression(language, diagnostics, build_ctx, rhs_node)?; + // TODO: Implement as a method on Property type + // e.g., lhs.try_eq(rhs) -> Option>> + match (lhs.type_name(), rhs.type_name()) { + ("Boolean", "Boolean") => { + let lhs = lhs.try_into_boolean().unwrap(); + let rhs = rhs.try_into_boolean().unwrap(); + let out = lhs.and_then(move |l| Ok(l == rhs.extract()?)); + Ok(L::wrap_boolean(out)) + } + ("Integer", "Integer") => { + let lhs = lhs.try_into_integer().unwrap(); + let rhs = rhs.try_into_integer().unwrap(); + let out = lhs.and_then(move |l| Ok(l == rhs.extract()?)); + Ok(L::wrap_boolean(out)) + } + ("String", "String") => { + let lhs = lhs.try_into_plain_text().unwrap(); + let rhs = rhs.try_into_plain_text().unwrap(); + let out = lhs.and_then(move |l| Ok(l == rhs.extract()?)); + Ok(L::wrap_boolean(out)) + } + (lhs_type @ ("Boolean" | "Integer" | "String"), rhs_type) => Err( + TemplateParseError::expected_type(lhs_type, rhs_type, rhs_node.span), + ), + (lhs_type, _) => { + let message = format!( + r#"Expected expression of type "Boolean", "Integer", or "String", but actual type is "{lhs_type}""# + ); + Err(TemplateParseError::expression(message, lhs_node.span)) + } + } + } } } @@ -1677,14 +1712,14 @@ mod tests { env.add_keyword("description", || L::wrap_string(Literal("".to_owned()))); env.add_keyword("empty", || L::wrap_boolean(Literal(true))); - insta::assert_snapshot!(env.parse_err(r#"description ()"#), @r###" + insta::assert_snapshot!(env.parse_err(r#"description ()"#), @r#" --> 1:13 | 1 | description () | ^--- | - = expected , `++`, `||`, or `&&` - "###); + = expected , `++`, `||`, `&&`, or `==` + "#); insta::assert_snapshot!(env.parse_err(r#"foo"#), @r###" --> 1:1 @@ -1728,6 +1763,62 @@ mod tests { | = Expected expression of type "Boolean", but actual type is "Integer" "###); + insta::assert_snapshot!(env.parse_err(r#"true == 1"#), @r#" + --> 1:9 + | + 1 | true == 1 + | ^ + | + = Expected expression of type "Boolean", but actual type is "Integer" + "#); + insta::assert_snapshot!(env.parse_err(r#"true == 'a'"#), @r#" + --> 1:9 + | + 1 | true == 'a' + | ^-^ + | + = Expected expression of type "Boolean", but actual type is "String" + "#); + insta::assert_snapshot!(env.parse_err(r#"1 == true"#), @r#" + --> 1:6 + | + 1 | 1 == true + | ^--^ + | + = Expected expression of type "Integer", but actual type is "Boolean" + "#); + insta::assert_snapshot!(env.parse_err(r#"1 == 'a'"#), @r#" + --> 1:6 + | + 1 | 1 == 'a' + | ^-^ + | + = Expected expression of type "Integer", but actual type is "String" + "#); + insta::assert_snapshot!(env.parse_err(r#"'a' == true"#), @r#" + --> 1:8 + | + 1 | 'a' == true + | ^--^ + | + = Expected expression of type "String", but actual type is "Boolean" + "#); + insta::assert_snapshot!(env.parse_err(r#"'a' == 1"#), @r#" + --> 1:8 + | + 1 | 'a' == 1 + | ^ + | + = Expected expression of type "String", but actual type is "Integer" + "#); + insta::assert_snapshot!(env.parse_err(r#"'a' == label("", "")"#), @r#" + --> 1:8 + | + 1 | 'a' == label("", "") + | ^-----------^ + | + = Expected expression of type "String", but actual type is "Template" + "#); insta::assert_snapshot!(env.parse_err(r#"description.first_line().foo()"#), @r###" --> 1:26 @@ -1945,6 +2036,12 @@ mod tests { insta::assert_snapshot!(env.render_ok(r#"!false"#), @"true"); insta::assert_snapshot!(env.render_ok(r#"false || !false"#), @"true"); insta::assert_snapshot!(env.render_ok(r#"false && true"#), @"false"); + insta::assert_snapshot!(env.render_ok(r#"true == true"#), @"true"); + insta::assert_snapshot!(env.render_ok(r#"true == false"#), @"false"); + insta::assert_snapshot!(env.render_ok(r#"1 == 1"#), @"true"); + insta::assert_snapshot!(env.render_ok(r#"1 == 2"#), @"false"); + insta::assert_snapshot!(env.render_ok(r#"'a' == 'a'"#), @"true"); + insta::assert_snapshot!(env.render_ok(r#"'a' == 'b'"#), @"false"); insta::assert_snapshot!(env.render_ok(r#" !"" "#), @"true"); insta::assert_snapshot!(env.render_ok(r#" "" || "a".lines() "#), @"true"); diff --git a/cli/src/template_parser.rs b/cli/src/template_parser.rs index 1c6382141..536df62ec 100644 --- a/cli/src/template_parser.rs +++ b/cli/src/template_parser.rs @@ -74,6 +74,7 @@ impl Rule { Rule::concat_op => Some("++"), Rule::logical_or_op => Some("||"), Rule::logical_and_op => Some("&&"), + Rule::logical_eq_op => Some("=="), Rule::logical_not_op => Some("!"), Rule::negate_op => Some("-"), Rule::prefix_ops => None, @@ -374,6 +375,8 @@ pub enum BinaryOp { LogicalOr, /// `&&` LogicalAnd, + /// `==` + LogicalEq, } pub type ExpressionNode<'i> = dsl_util::ExpressionNode<'i, ExpressionKind<'i>>; @@ -504,6 +507,7 @@ fn parse_expression_node(pair: Pair) -> TemplateParseResult) -> TemplateParseResult BinaryOp::LogicalOr, Rule::logical_and_op => BinaryOp::LogicalAnd, + Rule::logical_eq_op => BinaryOp::LogicalEq, r => panic!("unexpected infix operator rule {r:?}"), }; let lhs = Box::new(lhs?); @@ -851,12 +856,16 @@ mod tests { parse_normalized("(!(x.f())) || (!(g()))"), ); assert_eq!( - parse_normalized("x.f() || y || z"), - parse_normalized("((x.f()) || y) || z"), + parse_normalized("!x.f() == !x.f() || !g() == !g()"), + parse_normalized("((!(x.f())) == (!(x.f()))) || ((!(g())) == (!(g())))"), ); assert_eq!( - parse_normalized("x || y && z.h()"), - parse_normalized("x || (y && (z.h()))"), + parse_normalized("x.f() || y == y || z"), + parse_normalized("((x.f()) || (y == y)) || z"), + ); + assert_eq!( + parse_normalized("x || y == y && z.h() == z"), + parse_normalized("x || ((y == y) && ((z.h()) == z))"), ); // Logical operator bounds more tightly than concatenation. This might @@ -869,6 +878,10 @@ mod tests { parse_normalized(r"x ++ y || z"), parse_normalized(r"x ++ (y || z)"), ); + assert_eq!( + parse_normalized(r"x == y ++ z"), + parse_normalized(r"(x == y) ++ z"), + ); // Expression span assert_eq!(parse_template(" ! x ").unwrap().span.as_str(), "! x"); diff --git a/cli/tests/test_templater.rs b/cli/tests/test_templater.rs index 5e796ed04..7e454dda3 100644 --- a/cli/tests/test_templater.rs +++ b/cli/tests/test_templater.rs @@ -25,15 +25,15 @@ fn test_templater_parse_error() { let repo_path = test_env.env_root().join("repo"); let render_err = |template| test_env.jj_cmd_failure(&repo_path, &["log", "-T", template]); - insta::assert_snapshot!(render_err(r#"description ()"#), @r###" + insta::assert_snapshot!(render_err(r#"description ()"#), @r#" Error: Failed to parse template: Syntax error Caused by: --> 1:13 | 1 | description () | ^--- | - = expected , `++`, `||`, or `&&` - "###); + = expected , `++`, `||`, `&&`, or `==` + "#); // Typo test_env.add_config( diff --git a/docs/templates.md b/docs/templates.md index 76862c083..6fde385df 100644 --- a/docs/templates.md +++ b/docs/templates.md @@ -31,6 +31,8 @@ The following operators are supported. * `x.f()`: Method call. * `-x`: Negate integer value. * `!x`: Logical not. +* `x == y`: Logical equal. Operands must be either `Boolean`, `Integer`, or + `String`. * `x && y`: Logical and, short-circuiting. * `x || y`: Logical or, short-circuiting. * `x ++ y`: Concatenate `x` and `y` templates.