From 96e0bc0bdd997152bbbcc356d52f99ce58bb5c65 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 21 Mar 2024 13:14:53 +0900
Subject: [PATCH] templater: turn logical && and || into short-circuiting
 operators

Since the context (or self) property is no longer passed by argument, it's easy
to implement short-circuiting behavior.
---
 cli/src/template_builder.rs | 21 ++++++++++++++++-----
 docs/templates.md           |  4 ++--
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs
index b991e80d0..6125a23b5 100644
--- a/cli/src/template_builder.rs
+++ b/cli/src/template_builder.rs
@@ -495,16 +495,16 @@ fn build_binary_operation<'a, L: TemplateLanguage<'a> + ?Sized>(
 ) -> TemplateParseResult<L::Property> {
     match op {
         BinaryOp::LogicalOr => {
-            // No short-circuiting supported
             let lhs = expect_boolean_expression(language, build_ctx, lhs_node)?;
             let rhs = expect_boolean_expression(language, build_ctx, rhs_node)?;
-            Ok(L::wrap_boolean((lhs, rhs).map(|(l, r)| l | r)))
+            let out = lhs.and_then(move |l| Ok(l || rhs.extract()?));
+            Ok(L::wrap_boolean(out))
         }
         BinaryOp::LogicalAnd => {
-            // No short-circuiting supported
             let lhs = expect_boolean_expression(language, build_ctx, lhs_node)?;
             let rhs = expect_boolean_expression(language, build_ctx, rhs_node)?;
-            Ok(L::wrap_boolean((lhs, rhs).map(|(l, r)| l & r)))
+            let out = lhs.and_then(move |l| Ok(l && rhs.extract()?));
+            Ok(L::wrap_boolean(out))
         }
     }
 }
@@ -1199,6 +1199,10 @@ mod tests {
         }
     }
 
+    fn new_error_property<O>(message: &str) -> impl TemplateProperty<Output = O> + '_ {
+        Literal(()).and_then(|()| Err(TemplatePropertyError(message.into())))
+    }
+
     fn new_signature(name: &str, email: &str) -> Signature {
         Signature {
             name: name.to_owned(),
@@ -1490,7 +1494,7 @@ mod tests {
 
     #[test]
     fn test_logical_operation() {
-        let env = TestTemplateEnv::new();
+        let mut env = TestTemplateEnv::new();
 
         insta::assert_snapshot!(env.render_ok(r#"!false"#), @"true");
         insta::assert_snapshot!(env.render_ok(r#"false || !false"#), @"true");
@@ -1498,6 +1502,13 @@ mod tests {
 
         insta::assert_snapshot!(env.render_ok(r#" !"" "#), @"true");
         insta::assert_snapshot!(env.render_ok(r#" "" || "a".lines() "#), @"true");
+
+        // Short-circuiting
+        env.add_keyword("bad_bool", || L::wrap_boolean(new_error_property("Bad")));
+        insta::assert_snapshot!(env.render_ok(r#"false && bad_bool"#), @"false");
+        insta::assert_snapshot!(env.render_ok(r#"true && bad_bool"#), @"<Error: Bad>");
+        insta::assert_snapshot!(env.render_ok(r#"false || bad_bool"#), @"<Error: Bad>");
+        insta::assert_snapshot!(env.render_ok(r#"true || bad_bool"#), @"true");
     }
 
     #[test]
diff --git a/docs/templates.md b/docs/templates.md
index fe4f1db41..bb5bace3c 100644
--- a/docs/templates.md
+++ b/docs/templates.md
@@ -31,8 +31,8 @@ The following operators are supported.
 * `x.f()`: Method call.
 * `-x`: Negate integer value.
 * `!x`: Logical not.
-* `x && y`: Logical and.
-* `x || y`: Logical or.
+* `x && y`: Logical and, short-circuiting.
+* `x || y`: Logical or, short-circuiting.
 * `x ++ y`: Concatenate `x` and `y` templates.
 
 ## Global functions