revset: translate symbol rules in error message

Since we have overloaded operator symbols, we need to deduplicate them
upfront. Legacy and compat operators are also removed from the suggestion.

It's a bit ugly to mutate the error struct before calling Error::renamed_rule(),
but I think it's still better than reimplementing message formatting function.

cli/tests/test_revset_output.rs

@@ -29,7 +29,7 @@ fn test_syntax_error() {
     1 | x &
       |    ^---
       |
-      = expected dag_range_pre_op, dag_range_all_op, legacy_dag_range_pre_op, range_pre_op, range_all_op, negate_op, or primary
+      = expected `::`, `..`, `~`, or <primary>
     "###);
 
     let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "x - y"]);
@@ -192,7 +192,7 @@ fn test_bad_function_call() {
     1 | remote_branches(=foo)
       |                 ^---
       |
-      = expected identifier or expression
+      = expected <identifier> or <expression>
     "###);
 
     let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "remote_branches(remote=)"]);
@@ -202,7 +202,7 @@ fn test_bad_function_call() {
     1 | remote_branches(remote=)
       |                        ^---
       |
-      = expected expression
+      = expected <expression>
     "###);
 }
 
@@ -287,7 +287,7 @@ fn test_alias() {
     1 | whatever &
       |           ^---
       |
-      = expected dag_range_pre_op, dag_range_all_op, legacy_dag_range_pre_op, range_pre_op, range_all_op, negate_op, or primary
+      = expected `::`, `..`, `~`, or <primary>
     "###);
 
     let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "identity()"]);
@@ -401,7 +401,7 @@ fn test_bad_alias_decl() {
     1 | "bad"
       | ^---
       |
-      = expected identifier or function_name
+      = expected <identifier> or <function_name>
     Failed to load "revset-aliases.badfn(a, a)":  --> 1:7
       |
     1 | badfn(a, a)

lib/src/revset.rs

@@ -14,7 +14,7 @@
 
 #![allow(missing_docs)]
 
-use std::collections::{BTreeMap, HashMap};
+use std::collections::{BTreeMap, HashMap, HashSet};
 use std::convert::Infallible;
 use std::ops::Range;
 use std::path::Path;
@@ -76,6 +76,74 @@ pub enum RevsetEvaluationError {
 #[grammar = "revset.pest"]
 pub struct RevsetParser;
 
+impl Rule {
+    /// Whether this is a placeholder rule for compatibility with the other
+    /// systems.
+    fn is_compat(&self) -> bool {
+        matches!(
+            self,
+            Rule::compat_parents_op | Rule::compat_add_op | Rule::compat_sub_op
+        )
+    }
+
+    /// Whether this is a deprecated rule to be removed in later version.
+    fn is_legacy(&self) -> bool {
+        matches!(
+            self,
+            Rule::legacy_dag_range_op
+                | Rule::legacy_dag_range_pre_op
+                | Rule::legacy_dag_range_post_op
+        )
+    }
+
+    fn to_symbol(self) -> Option<&'static str> {
+        match self {
+            Rule::EOI => None,
+            Rule::identifier_part => None,
+            Rule::identifier => None,
+            Rule::symbol => None,
+            Rule::literal_string => None,
+            Rule::whitespace => None,
+            Rule::at_op => Some("@"),
+            Rule::parents_op => Some("-"),
+            Rule::children_op => Some("+"),
+            Rule::compat_parents_op => Some("^"),
+            Rule::dag_range_op
+            | Rule::dag_range_pre_op
+            | Rule::dag_range_post_op
+            | Rule::dag_range_all_op => Some("::"),
+            Rule::legacy_dag_range_op
+            | Rule::legacy_dag_range_pre_op
+            | Rule::legacy_dag_range_post_op => Some(":"),
+            Rule::range_op => Some(".."),
+            Rule::range_pre_op | Rule::range_post_op | Rule::range_all_op => Some(".."),
+            Rule::range_ops => None,
+            Rule::range_pre_ops => None,
+            Rule::range_post_ops => None,
+            Rule::range_all_ops => None,
+            Rule::negate_op => Some("~"),
+            Rule::union_op => Some("|"),
+            Rule::intersection_op => Some("&"),
+            Rule::difference_op => Some("~"),
+            Rule::compat_add_op => Some("+"),
+            Rule::compat_sub_op => Some("-"),
+            Rule::infix_op => None,
+            Rule::function_name => None,
+            Rule::keyword_argument => None,
+            Rule::argument => None,
+            Rule::function_arguments => None,
+            Rule::formal_parameters => None,
+            Rule::primary => None,
+            Rule::neighbors_expression => None,
+            Rule::range_expression => None,
+            Rule::expression => None,
+            Rule::program => None,
+            Rule::alias_declaration_part => None,
+            Rule::alias_declaration => None,
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct RevsetParseError {
     kind: RevsetParseErrorKind,
@@ -175,7 +243,7 @@ impl From<pest::error::Error<Rule>> for RevsetParseError {
     fn from(err: pest::error::Error<Rule>) -> Self {
         RevsetParseError {
             kind: RevsetParseErrorKind::SyntaxError,
-            pest_error: Some(Box::new(err)),
+            pest_error: Some(Box::new(rename_rules_in_pest_error(err))),
             origin: None,
         }
     }
@@ -207,6 +275,32 @@ impl error::Error for RevsetParseError {
     }
 }
 
+fn rename_rules_in_pest_error(mut err: pest::error::Error<Rule>) -> pest::error::Error<Rule> {
+    let pest::error::ErrorVariant::ParsingError {
+        positives,
+        negatives,
+    } = &mut err.variant
+    else {
+        return err;
+    };
+
+    // Remove duplicated symbols. Legacy or compat symbols are also removed from the
+    // (positive) suggestion.
+    let mut known_syms = HashSet::new();
+    positives.retain(|rule| {
+        !rule.is_compat()
+            && !rule.is_legacy()
+            && rule.to_symbol().map_or(true, |sym| known_syms.insert(sym))
+    });
+    let mut known_syms = HashSet::new();
+    negatives.retain(|rule| rule.to_symbol().map_or(true, |sym| known_syms.insert(sym)));
+    err.renamed_rules(|rule| {
+        rule.to_symbol()
+            .map(|sym| format!("`{sym}`"))
+            .unwrap_or_else(|| format!("<{rule:?}>"))
+    })
+}
+
 // assumes index has less than u64::MAX entries.
 pub const GENERATION_RANGE_FULL: Range<u64> = 0..u64::MAX;
 pub const GENERATION_RANGE_EMPTY: Range<u64> = 0..0;