templater: add list.map(|x| ...) operation

This involves a little hack to insert a lambda parameter 'x' to be used at
keyword position. If the template language were dynamically typed (and were
interpreted), .map() implementation would be simpler. I considered that, but
interpreter version has its own warts (late error reporting, uneasy to cache
static object, etc.), and I don't think the current template engine is
complex enough to rewrite from scratch.

.map() returns template, which can't be join()-ed. This will be fixed later.

docs/templates.md

@@ -92,6 +92,8 @@ The following methods are defined.
 
 * `.join(separator: Template) -> Template`: Concatenate elements with
   the given `separator`.
+* `.map(|item| expression) -> Template`: Apply template `expression`
+  to each element. Example: `parent_commit_ids.map(|id| id.short())`
 
 ### OperationId type
 

src/commit_templater.rs

@@ -65,7 +65,9 @@ impl<'repo> TemplateLanguage<'repo> for CommitTemplateLanguage<'repo, '_> {
                 build_commit_or_change_id_method(self, build_ctx, property, function)
             }
             CommitTemplatePropertyKind::CommitOrChangeIdList(property) => {
-                template_builder::build_list_method(self, build_ctx, property, function)
+                template_builder::build_list_method(self, build_ctx, property, function, |item| {
+                    self.wrap_commit_or_change_id(item)
+                })
             }
             CommitTemplatePropertyKind::ShortestIdPrefix(property) => {
                 build_shortest_id_prefix_method(self, build_ctx, property, function)

src/template_builder.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::collections::HashMap;
+
 use itertools::Itertools as _;
 use jujutsu_lib::backend::{Signature, Timestamp};
 
@@ -21,8 +23,8 @@ use crate::template_parser::{
 };
 use crate::templater::{
     ConcatTemplate, ConditionalTemplate, IntoTemplate, LabelTemplate, ListPropertyTemplate,
-    Literal, PlainTextFormattedProperty, ReformatTemplate, SeparateTemplate, Template,
-    TemplateFunction, TemplateProperty, TimestampRange,
+    Literal, PlainTextFormattedProperty, PropertyPlaceholder, ReformatTemplate, SeparateTemplate,
+    Template, TemplateFunction, TemplateProperty, TimestampRange,
 };
 use crate::{text_util, time_util};
 
@@ -237,7 +239,8 @@ impl<'a, C: 'a, P: IntoTemplate<'a, C>> IntoTemplate<'a, C> for Expression<P> {
 }
 
 pub struct BuildContext<'i, P> {
-    _phantom: std::marker::PhantomData<&'i P>, // TODO
+    /// Map of functions to create `L::Property`.
+    local_variables: HashMap<&'i str, &'i (dyn Fn() -> P)>,
 }
 
 fn build_method_call<'a, L: TemplateLanguage<'a>>(
@@ -263,7 +266,9 @@ pub fn build_core_method<'a, L: TemplateLanguage<'a>>(
             build_string_method(language, build_ctx, property, function)
         }
         CoreTemplatePropertyKind::StringList(property) => {
-            build_list_method(language, build_ctx, property, function)
+            build_list_method(language, build_ctx, property, function, |item| {
+                language.wrap_string(item)
+            })
         }
         CoreTemplatePropertyKind::Boolean(property) => {
             build_boolean_method(language, build_ctx, property, function)
@@ -450,12 +455,20 @@ fn build_timestamp_range_method<'a, L: TemplateLanguage<'a>>(
     Ok(property)
 }
 
-pub fn build_list_method<'a, L: TemplateLanguage<'a>, P: Template<()> + 'a>(
+/// Builds method call expression for printable list property.
+pub fn build_list_method<'a, L, O>(
     language: &L,
     build_ctx: &BuildContext<L::Property>,
-    self_property: impl TemplateProperty<L::Context, Output = Vec<P>> + 'a,
+    self_property: impl TemplateProperty<L::Context, Output = Vec<O>> + 'a,
     function: &FunctionCallNode,
-) -> TemplateParseResult<L::Property> {
+    // TODO: Generic L: WrapProperty<L::Context, O> trait might be needed to support more
+    // list operations such as first()/slice(). For .map(), a simple callback works.
+    wrap_item: impl Fn(PropertyPlaceholder<O>) -> L::Property,
+) -> TemplateParseResult<L::Property>
+where
+    L: TemplateLanguage<'a>,
+    O: Template<()> + Clone + 'a,
+{
     let property = match function.name {
         "join" => {
             let [separator_node] = template_parser::expect_exact_arguments(function)?;
@@ -466,12 +479,61 @@ pub fn build_list_method<'a, L: TemplateLanguage<'a>, P: Template<()> + 'a>(
                 });
             language.wrap_template(template)
         }
-        // TODO: .map()
+        "map" => build_map_operation(language, build_ctx, self_property, function, wrap_item)?,
         _ => return Err(TemplateParseError::no_such_method("List", function)),
     };
     Ok(property)
 }
 
+/// Builds expression that extracts iterable property and applies template to
+/// each item.
+///
+/// `wrap_item()` is the function to wrap a list item of type `O` as a property.
+fn build_map_operation<'a, L, O, P>(
+    language: &L,
+    build_ctx: &BuildContext<L::Property>,
+    self_property: P,
+    function: &FunctionCallNode,
+    wrap_item: impl Fn(PropertyPlaceholder<O>) -> L::Property,
+) -> TemplateParseResult<L::Property>
+where
+    L: TemplateLanguage<'a>,
+    P: TemplateProperty<L::Context> + 'a,
+    P::Output: IntoIterator<Item = O>,
+    O: Clone + 'a,
+{
+    // Build an item template with placeholder property, then evaluate it
+    // for each item.
+    //
+    // It would be nice if we could build a template of (L::Context, O)
+    // input, but doing that for a generic item type wouldn't be easy. It's
+    // also invalid to convert &C to &(C, _).
+    let [lambda_node] = template_parser::expect_exact_arguments(function)?;
+    let item_placeholder = PropertyPlaceholder::new();
+    let item_template = template_parser::expect_lambda_with(lambda_node, |lambda, _span| {
+        let item_fn = || wrap_item(item_placeholder.clone());
+        let mut local_variables = build_ctx.local_variables.clone();
+        if let [name] = lambda.params.as_slice() {
+            local_variables.insert(name, &item_fn);
+        } else {
+            return Err(TemplateParseError::unexpected_expression(
+                "Expected 1 lambda parameters",
+                lambda.params_span,
+            ));
+        }
+        let build_ctx = BuildContext { local_variables };
+        Ok(build_expression(language, &build_ctx, &lambda.body)?.into_template())
+    })?;
+    let list_template = ListPropertyTemplate::new(
+        self_property,
+        Literal(" "), // separator
+        move |context, formatter, item| {
+            item_placeholder.with_value(item, || item_template.format(context, formatter))
+        },
+    );
+    Ok(language.wrap_template(list_template))
+}
+
 fn build_global_function<'a, L: TemplateLanguage<'a>>(
     language: &L,
     build_ctx: &BuildContext<L::Property>,
@@ -558,8 +620,13 @@ pub fn build_expression<'a, L: TemplateLanguage<'a>>(
 ) -> TemplateParseResult<Expression<L::Property>> {
     match &node.kind {
         ExpressionKind::Identifier(name) => {
-            let property = language.build_keyword(name, node.span)?;
-            Ok(Expression::with_label(property, *name))
+            if let Some(make) = build_ctx.local_variables.get(name) {
+                // Don't label a local variable with its name
+                Ok(Expression::unlabeled(make()))
+            } else {
+                let property = language.build_keyword(name, node.span)?;
+                Ok(Expression::with_label(property, *name))
+            }
         }
         ExpressionKind::Integer(value) => {
             let property = language.wrap_integer(Literal(*value));
@@ -596,7 +663,7 @@ pub fn build<'a, L: TemplateLanguage<'a>>(
     node: &ExpressionNode,
 ) -> TemplateParseResult<Box<dyn Template<L::Context> + 'a>> {
     let build_ctx = BuildContext {
-        _phantom: std::marker::PhantomData,
+        local_variables: HashMap::new(),
     };
     let expression = build_expression(language, &build_ctx, node)?;
     Ok(expression.into_template())

src/template_parser.rs

@@ -689,6 +689,28 @@ pub fn expect_string_literal_with<'a, 'i, T>(
     }
 }
 
+/// Applies the given function if the `node` is a lambda.
+pub fn expect_lambda_with<'a, 'i, T>(
+    node: &'a ExpressionNode<'i>,
+    f: impl FnOnce(&'a LambdaNode<'i>, pest::Span<'i>) -> TemplateParseResult<T>,
+) -> TemplateParseResult<T> {
+    match &node.kind {
+        ExpressionKind::Lambda(lambda) => f(lambda, node.span),
+        ExpressionKind::String(_)
+        | ExpressionKind::Identifier(_)
+        | ExpressionKind::Integer(_)
+        | ExpressionKind::Concat(_)
+        | ExpressionKind::FunctionCall(_)
+        | ExpressionKind::MethodCall(_) => Err(TemplateParseError::unexpected_expression(
+            "Expected lambda expression",
+            node.span,
+        )),
+        ExpressionKind::AliasExpanded(id, subst) => {
+            expect_lambda_with(subst, f).map_err(|e| e.within_alias_expansion(*id, node.span))
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use assert_matches::assert_matches;

src/templater.rs

@@ -12,7 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::cell::RefCell;
 use std::io;
+use std::rc::Rc;
 
 use jujutsu_lib::backend::{Signature, Timestamp};
 
@@ -477,6 +479,53 @@ where
     }
 }
 
+/// Property which will be compiled into template once, and substituted later.
+#[derive(Clone, Debug)]
+pub struct PropertyPlaceholder<O> {
+    value: Rc<RefCell<Option<O>>>,
+}
+
+impl<O> PropertyPlaceholder<O> {
+    pub fn new() -> Self {
+        PropertyPlaceholder {
+            value: Rc::new(RefCell::new(None)),
+        }
+    }
+
+    pub fn set(&self, value: O) {
+        *self.value.borrow_mut() = Some(value);
+    }
+
+    pub fn take(&self) -> Option<O> {
+        self.value.borrow_mut().take()
+    }
+
+    pub fn with_value<R>(&self, value: O, f: impl FnOnce() -> R) -> R {
+        self.set(value);
+        let result = f();
+        self.take();
+        result
+    }
+}
+
+impl<O> Default for PropertyPlaceholder<O> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl<C, O: Clone> TemplateProperty<C> for PropertyPlaceholder<O> {
+    type Output = O;
+
+    fn extract(&self, _: &C) -> Self::Output {
+        self.value
+            .borrow()
+            .as_ref()
+            .expect("placeholder value must be set before evaluating template")
+            .clone()
+    }
+}
+
 pub fn format_joined<C, I, S>(
     context: &C,
     formatter: &mut dyn Formatter,

tests/test_commit_template.rs

@@ -39,6 +39,17 @@ fn test_log_parent_commit_ids() {
     ●  0000000000000000000000000000000000000000
        P:
     "###);
+
+    let template = r#"parent_commit_ids.map(|id| id.shortest(4))"#;
+    let stdout = test_env.jj_cmd_success(
+        &repo_path,
+        &["log", "-T", template, "-r@", "--color=always"],
+    );
+    insta::assert_snapshot!(stdout, @r###"
+    @  4db4 230d
+    │
+    ~
+    "###);
 }
 
 #[test]

tests/test_templater.rs

@@ -251,11 +251,86 @@ fn test_templater_list_method() {
     test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
     let repo_path = test_env.env_root().join("repo");
     let render = |template| get_template_output(&test_env, &repo_path, "@-", template);
+    let render_err = |template| test_env.jj_cmd_failure(&repo_path, &["log", "-T", template]);
+
+    test_env.add_config(
+        r###"
+    [template-aliases]
+    'identity' = '|x| x'
+    'too_many_params' = '|x, y| x'
+    "###,
+    );
 
     insta::assert_snapshot!(render(r#""".lines().join("|")"#), @"");
     insta::assert_snapshot!(render(r#""a\nb\nc".lines().join("|")"#), @"a|b|c");
     // Keyword as separator
     insta::assert_snapshot!(render(r#""a\nb\nc".lines().join(commit_id.short(2))"#), @"a00b00c");
+
+    insta::assert_snapshot!(render(r#""a\nb\nc".lines().map(|s| s ++ s)"#), @"aa bb cc");
+    // Global keyword in item template
+    insta::assert_snapshot!(
+        render(r#""a\nb\nc".lines().map(|s| s ++ empty)"#), @"atrue btrue ctrue");
+    // Override global keyword 'empty'
+    insta::assert_snapshot!(
+        render(r#""a\nb\nc".lines().map(|empty| empty)"#), @"a b c");
+    // Nested map operations
+    insta::assert_snapshot!(
+        render(r#""a\nb\nc".lines().map(|s| "x\ny".lines().map(|t| s ++ t))"#),
+        @"ax ay bx by cx cy");
+
+    // Lambda expression in alias
+    insta::assert_snapshot!(render(r#""a\nb\nc".lines().map(identity)"#), @"a b c");
+
+    // Not a lambda expression
+    insta::assert_snapshot!(render_err(r#""a".lines().map(empty)"#), @r###"
+    Error: Failed to parse template:  --> 1:17
+      |
+    1 | "a".lines().map(empty)
+      |                 ^---^
+      |
+      = Expected lambda expression
+    "###);
+    // Bad lambda parameter count
+    insta::assert_snapshot!(render_err(r#""a".lines().map(|| "")"#), @r###"
+    Error: Failed to parse template:  --> 1:18
+      |
+    1 | "a".lines().map(|| "")
+      |                  ^
+      |
+      = Expected 1 lambda parameters
+    "###);
+    insta::assert_snapshot!(render_err(r#""a".lines().map(|a, b| "")"#), @r###"
+    Error: Failed to parse template:  --> 1:18
+      |
+    1 | "a".lines().map(|a, b| "")
+      |                  ^--^
+      |
+      = Expected 1 lambda parameters
+    "###);
+    // Error in lambda expression
+    insta::assert_snapshot!(render_err(r#""a".lines().map(|s| s.unknown())"#), @r###"
+    Error: Failed to parse template:  --> 1:23
+      |
+    1 | "a".lines().map(|s| s.unknown())
+      |                       ^-----^
+      |
+      = Method "unknown" doesn't exist for type "String"
+    "###);
+    // Error in lambda alias
+    insta::assert_snapshot!(render_err(r#""a".lines().map(too_many_params)"#), @r###"
+    Error: Failed to parse template:  --> 1:17
+      |
+    1 | "a".lines().map(too_many_params)
+      |                 ^-------------^
+      |
+      = Alias "too_many_params" cannot be expanded
+     --> 1:2
+      |
+    1 | |x, y| x
+      |  ^--^
+      |
+      = Expected 1 lambda parameters
+    "###);
 }
 
 #[test]