From 3ceb2b7c12c2f37c71fe317b612df4012519816e Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Sat, 29 Apr 2023 16:15:38 -0700
Subject: [PATCH] cli: make `jj squash nonexistent` warn

This adds a check similar to that for `jj log`, so e.g. `jj squash @`
warns that the `@` argument is interpreted as path.
---
 src/commands/mod.rs          | 20 ++++++++++++++++++--
 tests/test_squash_command.rs | 15 ++++++++++++++-
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/commands/mod.rs b/src/commands/mod.rs
index b02b0c124..5b1b9691a 100644
--- a/src/commands/mod.rs
+++ b/src/commands/mod.rs
@@ -26,6 +26,7 @@ use std::sync::Arc;
 use std::{fs, io};
 
 use clap::builder::NonEmptyStringValueParser;
+use clap::parser::ValueSource;
 use clap::{ArgGroup, Command, CommandFactory, FromArgMatches, Subcommand};
 use indexmap::{IndexMap, IndexSet};
 use itertools::Itertools;
@@ -2303,8 +2304,23 @@ from the source will be moved into the parent.
         args.interactive,
         matcher.as_ref(),
     )?;
-    if args.interactive && &new_parent_tree_id == parent.tree_id() {
-        return Err(user_error("No changes selected"));
+    if &new_parent_tree_id == parent.tree_id() {
+        if args.interactive {
+            return Err(user_error("No changes selected"));
+        }
+
+        if let [only_path] = &args.paths[..] {
+            let (_, matches) = command.matches().subcommand().unwrap();
+            if matches.value_source("revision").unwrap() == ValueSource::DefaultValue
+                && revset::parse(only_path, &RevsetAliasesMap::new(), None).is_ok()
+            {
+                writeln!(
+                    ui.warning(),
+                    "warning: The argument {only_path:?} is being interpreted as a path. To \
+                     specify a revset, pass -r {only_path:?} instead."
+                )?;
+            }
+        }
     }
     // Abandon the child if the parent now has all the content from the child
     // (always the case in the non-interactive case).
diff --git a/tests/test_squash_command.rs b/tests/test_squash_command.rs
index 9756a14b4..9dba9d584 100644
--- a/tests/test_squash_command.rs
+++ b/tests/test_squash_command.rs
@@ -14,7 +14,7 @@
 
 use std::path::Path;
 
-use crate::common::TestEnvironment;
+use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
 
 pub mod common;
 
@@ -239,6 +239,19 @@ fn test_squash_partial() {
     Rebased 1 descendant commits
     Working copy now at: 5e297967f76c (no description set)
     "###);
+
+    // We get a warning if we pass a positional argument that looks like a revset
+    test_env.jj_cmd_success(&repo_path, &["undo"]);
+    let assert = test_env
+        .jj_cmd(&repo_path, &["squash", "b"])
+        .assert()
+        .success();
+    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
+    warning: The argument "b" is being interpreted as a path. To specify a revset, pass -r "b" instead.
+    "###);
+    insta::assert_snapshot!(get_stdout_string(&assert), @r###"
+    Working copy now at: 1c4e5596a511 (no description set)
+    "###);
 }
 
 fn get_log_output(test_env: &TestEnvironment, repo_path: &Path) -> String {