From 33d6c630b9bb72e4c74bd9adff70956a600afc4d Mon Sep 17 00:00:00 2001
From: Matt Stark <msta@google.com>
Date: Fri, 4 Oct 2024 11:13:06 +1000
Subject: [PATCH] Fix: Disallow revset function names starting with a number.

---
 CHANGELOG.md              | 2 ++
 lib/src/fileset.pest      | 2 +-
 lib/src/fileset_parser.rs | 8 ++++++++
 lib/src/revset.pest       | 2 +-
 lib/src/revset_parser.rs  | 1 +
 5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 052f4ed84a..ee16d34c9a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Breaking changes
 
+* Revset function names can no longer start with a number.
+
 ### Deprecations
 
 ### New features
diff --git a/lib/src/fileset.pest b/lib/src/fileset.pest
index bc84aa4e2f..41fb377b77 100644
--- a/lib/src/fileset.pest
+++ b/lib/src/fileset.pest
@@ -52,7 +52,7 @@ prefix_ops = _{ negate_op }
 infix_ops = _{ union_op | intersection_op | difference_op }
 
 function = { function_name ~ "(" ~ whitespace* ~ function_arguments ~ whitespace* ~ ")" }
-function_name = @{ (ASCII_ALPHANUMERIC | "_")+ }
+function_name = @{ (ASCII_ALPHA | "_") ~ (ASCII_ALPHANUMERIC | "_")* }
 function_arguments = {
   expression ~ (whitespace* ~ "," ~ whitespace* ~ expression)* ~ (whitespace* ~ ",")?
   | ""
diff --git a/lib/src/fileset_parser.rs b/lib/src/fileset_parser.rs
index 211be4ce01..09f722c589 100644
--- a/lib/src/fileset_parser.rs
+++ b/lib/src/fileset_parser.rs
@@ -438,6 +438,14 @@ mod tests {
         assert_ne!(parse_normalized(r#" foo "#), parse_normalized(r#" "foo" "#));
     }
 
+    #[test]
+    fn test_parse_invalid_function_name() {
+        assert_eq!(
+            parse_into_kind("5foo(x)"),
+            Err(FilesetParseErrorKind::SyntaxError)
+        );
+    }
+
     #[test]
     fn test_parse_whitespace() {
         let ascii_whitespaces: String = ('\x00'..='\x7f')
diff --git a/lib/src/revset.pest b/lib/src/revset.pest
index 7fd6aa09e1..2705b512d2 100644
--- a/lib/src/revset.pest
+++ b/lib/src/revset.pest
@@ -64,7 +64,7 @@ compat_sub_op = { "-" }
 infix_op = _{ union_op | intersection_op | difference_op | compat_add_op | compat_sub_op }
 
 function = { function_name ~ "(" ~ whitespace* ~ function_arguments ~ whitespace* ~ ")" }
-function_name = @{ (ASCII_ALPHANUMERIC | "_")+ }
+function_name = @{ (ASCII_ALPHA | "_") ~ (ASCII_ALPHANUMERIC | "_")* }
 keyword_argument = { identifier ~ whitespace* ~ "=" ~ whitespace* ~ expression }
 argument = _{ keyword_argument | expression }
 function_arguments = {
diff --git a/lib/src/revset_parser.rs b/lib/src/revset_parser.rs
index ea3ecea45c..9956522daf 100644
--- a/lib/src/revset_parser.rs
+++ b/lib/src/revset_parser.rs
@@ -1337,6 +1337,7 @@ mod tests {
     #[test]
     fn test_parse_revset_alias_func_decl() {
         let mut aliases_map = RevsetAliasesMap::new();
+        assert!(aliases_map.insert("5func()", r#""is function 0""#).is_err());
         aliases_map.insert("func()", r#""is function 0""#).unwrap();
         aliases_map
             .insert("func(a, b)", r#""is function 2""#)