cli: warn user if they try to jj bookmark set bookmark@remote

CHANGELOG.md

@@ -115,6 +115,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 * New `subject(pattern)` revset function that matches first line of commit
   descriptions.
+* Warn user if he tries to use `jj boookmark set branch@remote`
 
 ### Fixed bugs
 

cli/src/cli_util.rs

@@ -97,6 +97,7 @@ use jj_lib::repo_path::RepoPathBuf;
 use jj_lib::repo_path::RepoPathUiConverter;
 use jj_lib::repo_path::UiPathParseError;
 use jj_lib::revset;
+use jj_lib::revset::ExpressionNode;
 use jj_lib::revset::ResolvedRevsetExpression;
 use jj_lib::revset::RevsetAliasesMap;
 use jj_lib::revset::RevsetDiagnostics;
@@ -1603,6 +1604,19 @@ to the current parents may contain changes from multiple commits.
         Ok((self.attach_revset_evaluator(expression), modifier))
     }
 
+    pub fn parse_revset_expression<'a, 'b>(
+        &'b self,
+        revision_arg: &'a str,
+    ) -> Result<ExpressionNode<'a>, CommandError>
+    where
+        'b: 'a,
+    {
+        let context = self.revset_parse_context();
+        let expression = revset::parse_expr_with_expansion(revision_arg, &context)?;
+
+        Ok(expression)
+    }
+
     /// Parses the given revset expressions and concatenates them all.
     pub fn parse_union_revsets(
         &self,

cli/src/commands/bookmark/create.rs

@@ -55,6 +55,8 @@ pub fn cmd_bookmark_create(
         .resolve_single_rev(ui, args.revision.as_ref().unwrap_or(&RevisionArg::AT))?;
     let view = workspace_command.repo().view();
     let bookmark_names = &args.names;
+    super::validate_bookmark_names(command, ui, bookmark_names, "create")?;
+
     for name in bookmark_names {
         if view.get_local_bookmark(name).is_present() {
             return Err(user_error_with_hint(

cli/src/commands/bookmark/mod.rs

@@ -27,6 +27,7 @@ use jj_lib::backend::CommitId;
 use jj_lib::op_store::RefTarget;
 use jj_lib::op_store::RemoteRef;
 use jj_lib::repo::Repo;
+use jj_lib::revset::ExpressionKind;
 use jj_lib::str_util::StringPattern;
 use jj_lib::view::View;
 
@@ -51,6 +52,7 @@ use self::untrack::BookmarkUntrackArgs;
 use crate::cli_util::CommandHelper;
 use crate::cli_util::RemoteBookmarkName;
 use crate::cli_util::RemoteBookmarkNamePattern;
+use crate::cli_util::WorkspaceCommandHelper;
 use crate::command_error::user_error;
 use crate::command_error::CommandError;
 use crate::ui::Ui;
@@ -201,3 +203,93 @@ fn is_fast_forward(repo: &dyn Repo, old_target: &RefTarget, new_target_id: &Comm
         true
     }
 }
+
+/// Check if any bookmark names contain '@' and provide a warning if they do.
+fn validate_bookmark_names(
+    command: &CommandHelper,
+    ui: &mut Ui,
+    bookmark_names: &[String],
+    command_name: &str,
+) -> Result<(), CommandError> {
+    let helper = command.workspace_helper_no_snapshot(ui)?;
+
+    let mut remote_like = Vec::new();
+    let mut revset_like = Vec::new();
+
+    for name in bookmark_names {
+        match BookMarkKind::parse(name, &helper)? {
+            BookMarkKind::RemoteLike => remote_like.push(name.as_str()),
+            BookMarkKind::RevsetLike => revset_like.push(name.as_str()),
+            BookMarkKind::Normal => {}
+        }
+    }
+
+    if !remote_like.is_empty() {
+        let mut writer = ui.warning_with_heading("Bookmarks containing '@':");
+        for name in &remote_like {
+            writeln!(writer, "  {name}")?;
+        }
+        drop(writer);
+
+        let mut hint = ui.hint_default();
+        writeln!(
+            hint,
+            "To track remote bookmarks, use: jj bookmark --track {}",
+            remote_like.join(" ")
+        )?;
+    }
+
+    if !revset_like.is_empty() {
+        let mut writer = ui.warning_with_heading("Bookmarks resembling revsets:");
+        for name in &revset_like {
+            writeln!(writer, "  {name}")?;
+        }
+        drop(writer);
+
+        let mut warn = ui.warning_default();
+        writeln!(
+            warn,
+            "These names look like revset expressions and might cause confusion: {}",
+            revset_like.join(" ")
+        )?;
+        let mut hint = ui.hint_default();
+
+        if revset_like.len() == 1 {
+            writeln!(
+                hint,
+                "Maybe you meant to use `jj bookmark {command_name} -r {name}` instead?",
+                name = revset_like[0],
+            )?;
+        }
+    }
+
+    Ok(())
+}
+
+enum BookMarkKind {
+    RevsetLike,
+    RemoteLike,
+    Normal,
+}
+
+impl BookMarkKind {
+    fn parse(name: &str, helper: &WorkspaceCommandHelper) -> Result<Self, CommandError> {
+        let node = helper.parse_revset_expression(name)?;
+        Ok(match node.kind {
+            ExpressionKind::RemoteSymbol { .. } => Self::RemoteLike,
+            ExpressionKind::String(_)
+            | ExpressionKind::StringPattern { .. }
+            | ExpressionKind::AtWorkspace(_)
+            | ExpressionKind::AtCurrentWorkspace
+            | ExpressionKind::DagRangeAll
+            | ExpressionKind::RangeAll
+            | ExpressionKind::Unary(..)
+            | ExpressionKind::Binary(..)
+            | ExpressionKind::UnionAll(..)
+            | ExpressionKind::FunctionCall(..)
+            | ExpressionKind::Modifier(..)
+            | ExpressionKind::AliasExpanded(..) => Self::RevsetLike,
+            ExpressionKind::Identifier(_) => Self::Normal,
+        })
+    }
+}

cli/src/commands/bookmark/set.rs

@@ -61,6 +61,9 @@ pub fn cmd_bookmark_set(
         .resolve_single_rev(ui, args.revision.as_ref().unwrap_or(&RevisionArg::AT))?;
     let repo = workspace_command.repo().as_ref();
     let bookmark_names = &args.names;
+
+    super::validate_bookmark_names(command, ui, bookmark_names, "set")?;
+
     let mut new_bookmark_count = 0;
     let mut moved_bookmark_count = 0;
     for name in bookmark_names {

cli/tests/test_bookmark_command.rs

@@ -1898,6 +1898,57 @@ fn test_bookmark_list_conflicted() {
     "###);
 }
 
+#[test]
+fn test_bookmark_warns_multiple() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    // Test multiple invalid bookmarks at once
+    let (out, error) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["bookmark", "set", "main@origin", "dev@remote", "@-"],
+    );
+    insta::assert_snapshot!(out, @"");
+    insta::assert_snapshot!(error, @r"
+    Bookmarks containing '@':  main@origin
+      dev@remote
+    Hint: To track remote bookmarks, use: jj bookmark --track main@origin dev@remote
+    Bookmarks resembling revsets:  @-
+    Warning: These names look like revset expressions and might cause confusion: @-
+    Hint: Maybe you meant to use `jj bookmark set -r @-` instead?
+    Created 3 bookmarks pointing to qpvuntsm 230dd059 @- dev@remote main@origin | (empty) (no description set)
+    Hint: Use -r to specify the target revision.
+    Hint: Consider using `jj bookmark move` if your intention was to move existing bookmarks.
+    ");
+}
+
+#[test]
+fn test_bookmark_warns_single() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    // Test single invalid bookmark
+    let (out, error) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "set", "feature@remote"]);
+    insta::assert_snapshot!(out, @"");
+    insta::assert_snapshot!(error, @r"
+    Bookmarks containing '@':  feature@remote
+    Hint: To track remote bookmarks, use: jj bookmark --track feature@remote
+    Created 1 bookmarks pointing to qpvuntsm 230dd059 feature@remote | (empty) (no description set)
+    Hint: Consider using `jj bookmark move` if your intention was to move existing bookmarks.
+    ");
+
+    let (out, error) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "a..b"]);
+    insta::assert_snapshot!(out, @"");
+    insta::assert_snapshot!(error, @r"
+    Bookmarks resembling revsets:  a..b
+    Warning: These names look like revset expressions and might cause confusion: a..b
+    Hint: Maybe you meant to use `jj bookmark create -r a..b` instead?
+    Created 1 bookmarks pointing to qpvuntsm 230dd059 a..b feature@remote | (empty) (no description set)
+    ");
+}
+
 fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
     let template = r#"bookmarks ++ " " ++ commit_id.short()"#;
     test_env.jj_cmd_success(cwd, &["log", "-T", template])

lib/src/revset.rs

@@ -1194,8 +1194,7 @@ pub fn parse(
     revset_str: &str,
     context: &RevsetParseContext,
 ) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
-    let node = revset_parser::parse_program(revset_str)?;
-    let node = dsl_util::expand_aliases(node, context.aliases_map)?;
+    let node = parse_expr_with_expansion(revset_str, context)?;
     lower_expression(diagnostics, &node, context)
         .map_err(|err| err.extend_function_candidates(context.aliases_map.function_names()))
 }
@@ -1205,8 +1204,7 @@ pub fn parse_with_modifier(
     revset_str: &str,
     context: &RevsetParseContext,
 ) -> Result<(Rc<UserRevsetExpression>, Option<RevsetModifier>), RevsetParseError> {
-    let node = revset_parser::parse_program(revset_str)?;
-    let node = dsl_util::expand_aliases(node, context.aliases_map)?;
+    let node = parse_expr_with_expansion(revset_str, context)?;
     revset_parser::expect_program_with(
         diagnostics,
         &node,
@@ -1222,6 +1220,18 @@ pub fn parse_with_modifier(
     .map_err(|err| err.extend_function_candidates(context.aliases_map.function_names()))
 }
 
+pub fn parse_expr_with_expansion<'a, 'b>(
+    revset_str: &'a str,
+    context: &RevsetParseContext<'b>,
+) -> Result<ExpressionNode<'a>, RevsetParseError>
+where
+    'b: 'a,
+{
+    let node = revset_parser::parse_program(revset_str)?;
+    let node = dsl_util::expand_aliases(node, context.aliases_map)?;
+    Ok(node)
+}
+
 /// `Some` for rewritten expression, or `None` to reuse the original expression.
 type TransformedExpression<St> = Option<Rc<RevsetExpression<St>>>;