Skip to content

Commit

Permalink
template: add support for logical equality operator
Browse files Browse the repository at this point in the history
  • Loading branch information
bnjmnt4n committed Nov 7, 2024
1 parent aa04002 commit 1372b39
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 11 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion cli/src/template.pest
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
103 changes: 100 additions & 3 deletions cli/src/template_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Box<dyn TemplateProperty<Output = bool>>>
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))
}
}
}
}
}

Expand Down Expand Up @@ -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 <EOI>, `++`, `||`, or `&&`
"###);
= expected <EOI>, `++`, `||`, `&&`, or `==`
"#);

insta::assert_snapshot!(env.parse_err(r#"foo"#), @r###"
--> 1:1
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down
21 changes: 17 additions & 4 deletions cli/src/template_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -374,6 +375,8 @@ pub enum BinaryOp {
LogicalOr,
/// `&&`
LogicalAnd,
/// `==`
LogicalEq,
}

pub type ExpressionNode<'i> = dsl_util::ExpressionNode<'i, ExpressionKind<'i>>;
Expand Down Expand Up @@ -504,6 +507,7 @@ fn parse_expression_node(pair: Pair<Rule>) -> TemplateParseResult<ExpressionNode
PrattParser::new()
.op(Op::infix(Rule::logical_or_op, Assoc::Left))
.op(Op::infix(Rule::logical_and_op, Assoc::Left))
.op(Op::infix(Rule::logical_eq_op, Assoc::Left))
.op(Op::prefix(Rule::logical_not_op) | Op::prefix(Rule::negate_op))
});
PRATT
Expand All @@ -523,6 +527,7 @@ fn parse_expression_node(pair: Pair<Rule>) -> TemplateParseResult<ExpressionNode
let op_kind = match op.as_rule() {
Rule::logical_or_op => 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?);
Expand Down Expand Up @@ -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
Expand All @@ -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");
Expand Down
6 changes: 3 additions & 3 deletions cli/tests/test_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <EOI>, `++`, `||`, or `&&`
"###);
= expected <EOI>, `++`, `||`, `&&`, or `==`
"#);

// Typo
test_env.add_config(
Expand Down
2 changes: 2 additions & 0 deletions docs/templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 1372b39

Please sign in to comment.