From 1836a105bbdbe47750e4ace44ff10172bb5e9e3c Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Fri, 22 Dec 2023 12:57:24 +0900
Subject: [PATCH] cli: warn if -R/--repository could be involved in command
 alias expansion

#2414
---
 cli/src/cli_util.rs     | 16 ++++++++++++++--
 cli/tests/test_alias.rs | 12 +++++++++++-
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index 8ad79f69a..d39c9d2f1 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -2971,7 +2971,7 @@ impl CliRunner {
         let config = layered_configs.merge();
         ui.reset(&config)?;
 
-        let string_args = expand_args(ui, &self.app, std::env::args_os(), &config)?;
+        let string_args = expand_args(ui, &self.app, env::args_os(), &config)?;
         let (matches, args) = parse_args(
             ui,
             &self.app,
@@ -2987,7 +2987,6 @@ impl CliRunner {
             // Invalid -R path is an error. No need to proceed.
             let loader = WorkspaceLoader::init(&cwd.join(path))
                 .map_err(|err| map_workspace_load_error(err, Some(path)))?;
-            // TODO: maybe show error/warning if command aliases expanded differently
             layered_configs.read_repo_config(loader.repo_path())?;
             Ok(loader)
         } else {
@@ -2997,6 +2996,19 @@ impl CliRunner {
         // Apply workspace configs and --config-toml arguments.
         let config = layered_configs.merge();
         ui.reset(&config)?;
+
+        // If -R is specified, check if the expanded arguments differ. Aliases
+        // can also be injected by --config-toml, but that's obviously wrong.
+        if args.global_args.repository.is_some() {
+            let new_string_args = expand_args(ui, &self.app, env::args_os(), &config).ok();
+            if new_string_args.as_ref() != Some(&string_args) {
+                writeln!(
+                    ui.warning(),
+                    "Command aliases cannot be loaded from -R/--repository path"
+                )?;
+            }
+        }
+
         let settings = UserSettings::from_config(config);
         let working_copy_factories = self
             .working_copy_factories
diff --git a/cli/tests/test_alias.rs b/cli/tests/test_alias.rs
index f9599bba3..146739c39 100644
--- a/cli/tests/test_alias.rs
+++ b/cli/tests/test_alias.rs
@@ -276,7 +276,9 @@ fn test_alias_in_repo_config() {
     insta::assert_snapshot!(stdout, @r###"
     user alias
     "###);
-    insta::assert_snapshot!(stderr, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Command aliases cannot be loaded from -R/--repository path
+    "###);
 
     // Aliases are loaded from the cwd-relative workspace even with -R.
     let (stdout, stderr) =
@@ -284,6 +286,14 @@ fn test_alias_in_repo_config() {
     insta::assert_snapshot!(stdout, @r###"
     repo1 alias
     "###);
+    insta::assert_snapshot!(stderr, @r###"
+    Command aliases cannot be loaded from -R/--repository path
+    "###);
+
+    // No warning if the expanded command is identical.
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&repo1_path, &["files", "-R", repo2_path.to_str().unwrap()]);
+    insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @"");
 
     // Config loaded from the cwd-relative workspace shouldn't persist. It's