From 283231b1e448b7d81af4a74fdd12e8b8ebf7bd54 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Wed, 7 Dec 2022 19:16:29 +0900
Subject: [PATCH] cli: load all aliases to map first to suppress cryptic error

Otherwise 'jj foo.' would fail with 'Config error: Char'.
---
 src/cli_util.rs     | 43 +++++++++++++++++++------------------------
 tests/test_alias.rs | 16 ++++++++++++++++
 2 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/src/cli_util.rs b/src/cli_util.rs
index b91eb0022..3bf8da546 100644
--- a/src/cli_util.rs
+++ b/src/cli_util.rs
@@ -1402,6 +1402,10 @@ fn resolve_aliases(
     app: &clap::Command,
     string_args: &[String],
 ) -> Result<Vec<String>, CommandError> {
+    let mut aliases_map = user_settings
+        .config()
+        .get_table("alias")
+        .unwrap_or_default();
     let mut resolved_aliases = HashSet::new();
     let mut string_args = string_args.to_vec();
     let mut real_commands = HashSet::new();
@@ -1427,31 +1431,22 @@ fn resolve_aliases(
                         r#"Recursive alias definition involving "{alias_name}""#
                     )));
                 }
-                match user_settings
-                    .config()
-                    .get::<config::Value>(&format!("alias.{}", alias_name))
-                {
-                    Ok(value) => {
-                        if let Some(alias_definition) = string_list_from_config(value) {
-                            assert!(string_args.ends_with(&alias_args));
-                            string_args.truncate(string_args.len() - 1 - alias_args.len());
-                            string_args.extend(alias_definition);
-                            string_args.extend_from_slice(&alias_args);
-                            resolved_aliases.insert(alias_name.clone());
-                            continue;
-                        } else {
-                            return Err(user_error(format!(
-                                r#"Alias definition for "{alias_name}" must be a string list"#
-                            )));
-                        }
-                    }
-                    Err(config::ConfigError::NotFound(_)) => {
-                        // Not a real command and not an alias, so return what we've resolved so far
-                        return Ok(string_args);
-                    }
-                    Err(err) => {
-                        return Err(CommandError::from(err));
+                if let Some(value) = aliases_map.remove(&alias_name) {
+                    if let Some(alias_definition) = string_list_from_config(value) {
+                        assert!(string_args.ends_with(&alias_args));
+                        string_args.truncate(string_args.len() - 1 - alias_args.len());
+                        string_args.extend(alias_definition);
+                        string_args.extend_from_slice(&alias_args);
+                        resolved_aliases.insert(alias_name.clone());
+                        continue;
+                    } else {
+                        return Err(user_error(format!(
+                            r#"Alias definition for "{alias_name}" must be a string list"#
+                        )));
                     }
+                } else {
+                    // Not a real command and not an alias, so return what we've resolved so far
+                    return Ok(string_args);
                 }
             }
         }
diff --git a/tests/test_alias.rs b/tests/test_alias.rs
index c7dcd0bd0..8babe3d6e 100644
--- a/tests/test_alias.rs
+++ b/tests/test_alias.rs
@@ -35,6 +35,22 @@ fn test_alias_basic() {
     "###);
 }
 
+#[test]
+fn test_alias_bad_name() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    let stderr = test_env.jj_cmd_cli_error(&repo_path, &["foo."]);
+    insta::assert_snapshot!(stderr, @r###"
+    error: The subcommand 'foo.' wasn't recognized
+
+    Usage: jj [OPTIONS] <COMMAND>
+
+    For more information try '--help'
+    "###);
+}
+
 #[test]
 fn test_alias_calls_unknown_command() {
     let test_env = TestEnvironment::default();