From 3c1f6d5b5d4b6dbba12cf52429201f598be86bd2 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Mon, 3 Jun 2024 16:13:34 +0900
Subject: [PATCH] revset: insert AST-level representation of program modifier

---
 lib/src/revset.rs        |  9 +++++++++
 lib/src/revset_parser.rs | 25 +++++++++++++------------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index b6e2a67bd..479092ac8 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -851,6 +851,15 @@ pub fn parse_with_modifier(
     context: &RevsetParseContext,
 ) -> Result<(Rc<RevsetExpression>, Option<RevsetModifier>), RevsetParseError> {
     let (node, modifier) = revset_parser::parse_program_with_modifier(revset_str)?;
+    let modifier = modifier
+        .map(|n| match n.name {
+            "all" => Ok(RevsetModifier::All),
+            name => Err(RevsetParseError::with_span(
+                RevsetParseErrorKind::NoSuchModifier(name.to_owned()),
+                n.name_span,
+            )),
+        })
+        .transpose()?;
     let node = dsl_util::expand_aliases(node, context.aliases_map)?;
     let expression = lower_expression(&node, context)
         .map_err(|err| err.extend_function_candidates(context.aliases_map.function_names()))?;
diff --git a/lib/src/revset_parser.rs b/lib/src/revset_parser.rs
index 41fa3938a..c80c1456c 100644
--- a/lib/src/revset_parser.rs
+++ b/lib/src/revset_parser.rs
@@ -31,8 +31,6 @@ use crate::dsl_util::{
     AliasExpandError, AliasExpandableExpression, AliasId, AliasesMap, ExpressionFolder,
     FoldableExpression, InvalidArguments, KeywordArgument, StringLiteralParser,
 };
-// TODO: remove reverse dependency on revset module
-use crate::revset::RevsetModifier;
 
 #[derive(Parser)]
 #[grammar = "revset.pest"]
@@ -401,6 +399,14 @@ pub enum BinaryOp {
 pub type ExpressionNode<'i> = dsl_util::ExpressionNode<'i, ExpressionKind<'i>>;
 pub type FunctionCallNode<'i> = dsl_util::FunctionCallNode<'i, ExpressionKind<'i>>;
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct ModifierNode<'i> {
+    /// Modifier name.
+    pub name: &'i str,
+    /// Span of the modifier name.
+    pub name_span: pest::Span<'i>,
+}
+
 pub(super) fn parse_program(revset_str: &str) -> Result<ExpressionNode, RevsetParseError> {
     let mut pairs = RevsetParser::parse(Rule::program, revset_str)?;
     let first = pairs.next().unwrap();
@@ -409,7 +415,7 @@ pub(super) fn parse_program(revset_str: &str) -> Result<ExpressionNode, RevsetPa
 
 pub(super) fn parse_program_with_modifier(
     revset_str: &str,
-) -> Result<(ExpressionNode, Option<RevsetModifier>), RevsetParseError> {
+) -> Result<(ExpressionNode, Option<ModifierNode>), RevsetParseError> {
     let mut pairs = RevsetParser::parse(Rule::program_with_modifier, revset_str)?;
     let first = pairs.next().unwrap();
     match first.as_rule() {
@@ -423,17 +429,12 @@ pub(super) fn parse_program_with_modifier(
             assert_eq!(lhs.as_rule(), Rule::identifier);
             assert_eq!(op.as_rule(), Rule::pattern_kind_op);
             assert_eq!(rhs.as_rule(), Rule::expression);
-            let modififer = match lhs.as_str() {
-                "all" => RevsetModifier::All,
-                name => {
-                    return Err(RevsetParseError::with_span(
-                        RevsetParseErrorKind::NoSuchModifier(name.to_owned()),
-                        lhs.as_span(),
-                    ));
-                }
+            let modifier = ModifierNode {
+                name: lhs.as_str(),
+                name_span: lhs.as_span(),
             };
             let node = parse_expression_node(rhs.into_inner())?;
-            Ok((node, Some(modififer)))
+            Ok((node, Some(modifier)))
         }
         r => panic!("unexpected revset parse rule: {r:?}"),
     }