cli: warn if -R/--repository could be involved in command alias expan…

…sion jj-vcs#2414
yuja · Dec 23, 2023 · 94a4dcd · 94a4dcd
1 parent cb8180a
commit 94a4dcd
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 3 deletions.
diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
@@ -2974,7 +2974,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,
@@ -2990,7 +2990,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 {
@@ -3000,6 +2999,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
@@ -276,14 +276,24 @@ 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) =
         test_env.jj_cmd_ok(&repo1_path, &["l", "-R", repo2_path.to_str().unwrap()]);
     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