From 7dfe04134dcb20f2b077b1b0b82b1d84d1626d7b Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 14 Mar 2024 11:59:34 +0900
Subject: [PATCH] cli: check invalid declaration of immutable_heads() alias
 earlier

I just wanted to remove CommandError from parse_immutable_expression(), which
will be called from the templater, but the new error message looks also better.
---
 cli/src/revset_util.rs              | 30 +++++++++++++++++++++--------
 cli/tests/test_immutable_commits.rs | 12 ++----------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs
index a5ce91767..217c87843 100644
--- a/cli/src/revset_util.rs
+++ b/cli/src/revset_util.rs
@@ -30,6 +30,8 @@ use crate::command_error::{user_error, CommandError};
 use crate::config::LayeredConfigs;
 use crate::ui::Ui;
 
+const BUILTIN_IMMUTABLE_HEADS: &str = "immutable_heads";
+
 pub fn load_revset_aliases(
     ui: &Ui,
     layered_configs: &LayeredConfigs,
@@ -54,6 +56,17 @@ pub fn load_revset_aliases(
             }
         }
     }
+
+    // TODO: If we add support for function overloading (#2966), this check can
+    // be removed.
+    let (params, _) = aliases_map.get_function(BUILTIN_IMMUTABLE_HEADS).unwrap();
+    if !params.is_empty() {
+        return Err(user_error(format!(
+            "The `revset-aliases.{name}()` function must be declared without arguments",
+            name = BUILTIN_IMMUTABLE_HEADS
+        )));
+    }
+
     Ok(aliases_map)
 }
 
@@ -93,14 +106,15 @@ pub fn default_symbol_resolver<'a>(
 pub fn parse_immutable_expression(
     repo: &dyn Repo,
     context: &RevsetParseContext,
-) -> Result<Rc<RevsetExpression>, CommandError> {
-    let (params, immutable_heads_str) =
-        context.aliases_map.get_function("immutable_heads").unwrap();
-    if !params.is_empty() {
-        return Err(user_error(
-            r#"The `revset-aliases.immutable_heads()` function must be declared without arguments."#,
-        ));
-    }
+) -> Result<Rc<RevsetExpression>, RevsetParseError> {
+    let (params, immutable_heads_str) = context
+        .aliases_map
+        .get_function(BUILTIN_IMMUTABLE_HEADS)
+        .unwrap();
+    assert!(
+        params.is_empty(),
+        "invalid declaration should have been rejected by load_revset_aliases()"
+    );
     let immutable_heads_revset = parse(immutable_heads_str, context)?;
     Ok(immutable_heads_revset
         .ancestors()
diff --git a/cli/tests/test_immutable_commits.rs b/cli/tests/test_immutable_commits.rs
index 610e65011..082bb74b3 100644
--- a/cli/tests/test_immutable_commits.rs
+++ b/cli/tests/test_immutable_commits.rs
@@ -57,24 +57,16 @@ fn test_rewrite_immutable_generic() {
     Error: The root commit 000000000000 is immutable
     "###);
     // Error if we redefine immutable_heads() with an argument
-    // TODO: This error comes from the built-in definition of
-    // `revsets.short-prefixes`. That's not clear to the user.
     test_env.add_config(r#"revset-aliases."immutable_heads(foo)" = "none()""#);
     let stderr = test_env.jj_cmd_failure(&repo_path, &["edit", "root()"]);
     insta::assert_snapshot!(stderr, @r###"
-    Config error: Invalid `revsets.short-prefixes`:  --> 1:31
-      |
-    1 | @ | ancestors(immutable_heads().., 2) | trunk()
-      |                               ^
-      |
-      = Invalid arguments to revset function "immutable_heads": Expected 1 arguments
-    For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md.
+    Error: The `revset-aliases.immutable_heads()` function must be declared without arguments
     "###);
     // ... even if we also update the built-in call sites
     test_env.add_config(r#"revsets.short-prefixes = "immutable_heads(root())""#);
     let stderr = test_env.jj_cmd_failure(&repo_path, &["edit", "root()"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: The `revset-aliases.immutable_heads()` function must be declared without arguments.
+    Error: The `revset-aliases.immutable_heads()` function must be declared without arguments
     "###);
 }