Skip to content

Commit

Permalink
templater: add relational operators (>=, >, <=, <)
Browse files Browse the repository at this point in the history
Closes #5062.
  • Loading branch information
bnjmnt4n committed Dec 12, 2024
1 parent 48233a1 commit da5b790
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 9 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
`snapshot.max-new-file-size` config option. It will print a warning and large
files will be left untracked.

* Templates now support the `>=`, `>`, `<=`, and `<` relational operators for
`Integer` types.

### Fixed bugs

* The `$NO_COLOR` environment variable must now be non-empty to be respected.
Expand Down
15 changes: 14 additions & 1 deletion cli/src/template.pest
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,23 @@ logical_or_op = { "||" }
logical_and_op = { "&&" }
logical_eq_op = { "==" }
logical_ne_op = { "!=" }
ge_op = { ">=" }
gt_op = { ">" }
le_op = { "<=" }
lt_op = { "<" }
logical_not_op = { "!" }
negate_op = { "-" }
prefix_ops = _{ logical_not_op | negate_op }
infix_ops = _{ logical_or_op | logical_and_op | logical_eq_op | logical_ne_op }
infix_ops = _{
logical_or_op
| logical_and_op
| logical_eq_op
| logical_ne_op
| ge_op
| gt_op
| le_op
| lt_op
}

function = { identifier ~ "(" ~ whitespace* ~ function_arguments ~ whitespace* ~ ")" }
keyword_argument = { identifier ~ whitespace* ~ "=" ~ whitespace* ~ template }
Expand Down
45 changes: 42 additions & 3 deletions cli/src/template_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,23 @@ fn build_binary_operation<'a, L: TemplateLanguage<'a> + ?Sized>(
_ => unreachable!(),
}
}
BinaryOp::Ge | BinaryOp::Gt | BinaryOp::Le | BinaryOp::Lt => {
let lhs = build_expression(language, diagnostics, build_ctx, lhs_node)?;
let rhs = build_expression(language, diagnostics, build_ctx, rhs_node)?;
let lty = lhs.type_name();
let rty = rhs.type_name();
let out = lhs.try_into_cmp(rhs).ok_or_else(|| {
let message = format!(r#"Cannot compare expressions of type "{lty}" and "{rty}""#);
TemplateParseError::expression(message, span)
})?;
match op {
BinaryOp::Ge => Ok(L::wrap_boolean(out.map(|ordering| ordering.is_ge()))),
BinaryOp::Gt => Ok(L::wrap_boolean(out.map(|ordering| ordering.is_gt()))),
BinaryOp::Le => Ok(L::wrap_boolean(out.map(|ordering| ordering.is_le()))),
BinaryOp::Lt => Ok(L::wrap_boolean(out.map(|ordering| ordering.is_lt()))),
_ => unreachable!(),
}
}
}
}

Expand Down Expand Up @@ -1766,14 +1783,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 @@ -1873,6 +1890,14 @@ mod tests {
|
= Cannot compare expressions of type "String" and "Template"
"#);
insta::assert_snapshot!(env.parse_err(r#"'a' > 1"#), @r#"
--> 1:1
|
1 | 'a' > 1
| ^-----^
|
= Cannot compare expressions of type "String" and "Integer"
"#);

insta::assert_snapshot!(env.parse_err(r#"description.first_line().foo()"#), @r###"
--> 1:26
Expand Down Expand Up @@ -2083,6 +2108,20 @@ mod tests {
@"<Error: Attempt to negate with overflow>");
}

#[test]
fn test_relational_operation() {
let env = TestTemplateEnv::new();

insta::assert_snapshot!(env.render_ok(r#"1 >= 1"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"0 >= 1"#), @"false");
insta::assert_snapshot!(env.render_ok(r#"2 > 1"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"1 > 1"#), @"false");
insta::assert_snapshot!(env.render_ok(r#"1 <= 1"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"2 <= 1"#), @"false");
insta::assert_snapshot!(env.render_ok(r#"0 < 1"#), @"true");
insta::assert_snapshot!(env.render_ok(r#"1 < 1"#), @"false");
}

#[test]
fn test_logical_operation() {
let mut env = TestTemplateEnv::new();
Expand Down
30 changes: 28 additions & 2 deletions cli/src/template_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ impl Rule {
Rule::logical_and_op => Some("&&"),
Rule::logical_eq_op => Some("=="),
Rule::logical_ne_op => Some("!="),
Rule::ge_op => Some(">="),
Rule::gt_op => Some(">"),
Rule::le_op => Some("<="),
Rule::lt_op => Some("<"),
Rule::logical_not_op => Some("!"),
Rule::negate_op => Some("-"),
Rule::prefix_ops => None,
Expand Down Expand Up @@ -380,6 +384,14 @@ pub enum BinaryOp {
LogicalEq,
/// `!=`
LogicalNe,
/// `>=`
Ge,
/// `>`
Gt,
/// `<=`
Le,
/// `<`
Lt,
}

pub type ExpressionNode<'i> = dsl_util::ExpressionNode<'i, ExpressionKind<'i>>;
Expand Down Expand Up @@ -512,6 +524,10 @@ fn parse_expression_node(pair: Pair<Rule>) -> TemplateParseResult<ExpressionNode
.op(Op::infix(Rule::logical_and_op, Assoc::Left))
.op(Op::infix(Rule::logical_eq_op, Assoc::Left)
| Op::infix(Rule::logical_ne_op, Assoc::Left))
.op(Op::infix(Rule::ge_op, Assoc::Left)
| Op::infix(Rule::gt_op, Assoc::Left)
| Op::infix(Rule::le_op, Assoc::Left)
| Op::infix(Rule::lt_op, Assoc::Left))
.op(Op::prefix(Rule::logical_not_op) | Op::prefix(Rule::negate_op))
});
PRATT
Expand All @@ -533,6 +549,10 @@ fn parse_expression_node(pair: Pair<Rule>) -> TemplateParseResult<ExpressionNode
Rule::logical_and_op => BinaryOp::LogicalAnd,
Rule::logical_eq_op => BinaryOp::LogicalEq,
Rule::logical_ne_op => BinaryOp::LogicalNe,
Rule::ge_op => BinaryOp::Ge,
Rule::gt_op => BinaryOp::Gt,
Rule::le_op => BinaryOp::Le,
Rule::lt_op => BinaryOp::Lt,
r => panic!("unexpected infix operator rule {r:?}"),
};
let lhs = Box::new(lhs?);
Expand Down Expand Up @@ -861,8 +881,14 @@ mod tests {
parse_normalized("(!(x.f())) || (!(g()))"),
);
assert_eq!(
parse_normalized("!x.f() == !x.f() || !g() != !g()"),
parse_normalized("((!(x.f())) == (!(x.f()))) || ((!(g())) != (!(g())))"),
parse_normalized("!x.f() <= !x.f()"),
parse_normalized("((!(x.f())) <= (!(x.f())))"),
);
assert_eq!(
parse_normalized("!x.f() < !x.f() == !x.f() >= !x.f() || !g() != !g()"),
parse_normalized(
"((!(x.f()) < (!(x.f()))) == ((!(x.f())) >= (!(x.f())))) || ((!(g())) != (!(g())))"
),
);
assert_eq!(
parse_normalized("x.f() || y == y || z"),
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`, `x > y`, `x <= y`, `x < y`: Greater than or equal/greater than/
lesser than or equal/lesser than. Operands must be `Integer`s.
* `x == y`, `x != y`: Logical equal/not equal. Operands must be either
`Boolean`, `Integer`, or `String`.
* `x && y`: Logical and, short-circuiting.
Expand Down

0 comments on commit da5b790

Please sign in to comment.