From 49edbf47041c057c58cfca488d1d5ab547f589b9 Mon Sep 17 00:00:00 2001
From: Bryce Berger <bryce.z.berger@gmail.com>
Date: Sun, 24 Nov 2024 02:39:19 -0500
Subject: [PATCH 1/4] fileset: add alias parser

Additionally, adds alias handling to cli_util.

The implementation is mostly copied from the similar parts in
cli/template_parser.
---
 cli/src/cli_util.rs               |  50 ++++++++++++--
 cli/src/command_error.rs          |   3 +
 cli/src/commands/debug/fileset.rs |   4 +-
 cli/src/commands/fix.rs           |   1 +
 cli/src/commit_templater.rs       |   5 +-
 lib/src/fileset.pest              |   2 +
 lib/src/fileset.rs                |  56 +++++++++++++--
 lib/src/fileset_parser.rs         | 109 ++++++++++++++++++++++++++++++
 lib/src/revset.rs                 |  26 +++++--
 9 files changed, 239 insertions(+), 17 deletions(-)

diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index d1d8a523f8..21292b57b0 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -62,8 +62,11 @@ use jj_lib::config::ConfigGetResultExt as _;
 use jj_lib::config::ConfigNamePathBuf;
 use jj_lib::config::StackedConfig;
 use jj_lib::conflicts::ConflictMarkerStyle;
+use jj_lib::dsl_util::AliasDeclarationParser;
+use jj_lib::dsl_util::AliasesMap;
 use jj_lib::file_util;
 use jj_lib::fileset;
+use jj_lib::fileset::FilesetAliasesMap;
 use jj_lib::fileset::FilesetDiagnostics;
 use jj_lib::fileset::FilesetExpression;
 use jj_lib::git;
@@ -704,6 +707,7 @@ pub struct WorkspaceCommandEnvironment {
     command: CommandHelper,
     revset_aliases_map: RevsetAliasesMap,
     template_aliases_map: TemplateAliasesMap,
+    fileset_aliases_map: FilesetAliasesMap,
     path_converter: RepoPathUiConverter,
     workspace_id: WorkspaceId,
     immutable_heads_expression: Rc<UserRevsetExpression>,
@@ -716,6 +720,7 @@ impl WorkspaceCommandEnvironment {
     fn new(ui: &Ui, command: &CommandHelper, workspace: &Workspace) -> Result<Self, CommandError> {
         let revset_aliases_map = revset_util::load_revset_aliases(ui, command.settings().config())?;
         let template_aliases_map = command.load_template_aliases(ui)?;
+        let fileset_aliases_map = load_fileset_aliases(ui, command.settings().config())?;
         let path_converter = RepoPathUiConverter::Fs {
             cwd: command.cwd().to_owned(),
             base: workspace.workspace_root().to_owned(),
@@ -724,6 +729,7 @@ impl WorkspaceCommandEnvironment {
             command: command.clone(),
             revset_aliases_map,
             template_aliases_map,
+            fileset_aliases_map,
             path_converter,
             workspace_id: workspace.workspace_id().to_owned(),
             immutable_heads_expression: RevsetExpression::root(),
@@ -1263,7 +1269,14 @@ to the current parents may contain changes from multiple commits.
         let mut diagnostics = FilesetDiagnostics::new();
         let expressions: Vec<_> = file_args
             .iter()
-            .map(|arg| fileset::parse_maybe_bare(&mut diagnostics, arg, self.path_converter()))
+            .map(|arg| {
+                fileset::parse_maybe_bare(
+                    &mut diagnostics,
+                    arg,
+                    self.path_converter(),
+                    self.fileset_aliases_map(),
+                )
+            })
             .try_collect()?;
         print_parse_diagnostics(ui, "In fileset expression", &diagnostics)?;
         Ok(FilesetExpression::union_all(expressions))
@@ -1279,6 +1292,7 @@ to the current parents may contain changes from multiple commits.
                 cwd: "".into(),
                 base: "".into(),
             },
+            self.fileset_aliases_map(),
         )?;
         print_parse_diagnostics(ui, "In `snapshot.auto-track`", &diagnostics)?;
         Ok(expression.to_matcher())
@@ -1554,6 +1568,10 @@ to the current parents may contain changes from multiple commits.
         &self.env.template_aliases_map
     }
 
+    pub fn fileset_aliases_map(&self) -> &FilesetAliasesMap {
+        &self.env.fileset_aliases_map
+    }
+
     /// Parses template of the given language into evaluation tree.
     ///
     /// `wrap_self` specifies the type of the top-level property, which should
@@ -2684,11 +2702,31 @@ fn load_template_aliases(
     stacked_config: &StackedConfig,
 ) -> Result<TemplateAliasesMap, CommandError> {
     let table_name = ConfigNamePathBuf::from_iter(["template-aliases"]);
-    let mut aliases_map = TemplateAliasesMap::new();
+    load_aliases(ui, stacked_config, &table_name)
+}
+
+fn load_fileset_aliases(
+    ui: &Ui,
+    stacked_config: &StackedConfig,
+) -> Result<FilesetAliasesMap, CommandError> {
+    let table_name = ConfigNamePathBuf::from_iter(["fileset-aliases"]);
+    load_aliases(ui, stacked_config, &table_name)
+}
+
+fn load_aliases<P>(
+    ui: &Ui,
+    stacked_config: &StackedConfig,
+    table_name: &ConfigNamePathBuf,
+) -> Result<AliasesMap<P, String>, CommandError>
+where
+    P: Default + AliasDeclarationParser,
+    P::Error: ToString,
+{
+    let mut aliases_map = AliasesMap::new();
     // Load from all config layers in order. 'f(x)' in default layer should be
     // overridden by 'f(a)' in user.
     for layer in stacked_config.layers() {
-        let table = match layer.look_up_table(&table_name) {
+        let table = match layer.look_up_table(table_name) {
             Ok(Some(table)) => table,
             Ok(None) => continue,
             Err(item) => {
@@ -2708,7 +2746,11 @@ fn load_template_aliases(
                 .clone()
                 .into_string()
                 .map_err(|e| e.to_string())
-                .and_then(|v| aliases_map.insert(decl, v).map_err(|e| e.to_string()));
+                .and_then(|v| {
+                    aliases_map
+                        .insert(decl, v)
+                        .map_err(|e: P::Error| e.to_string())
+                });
             if let Err(s) = r {
                 writeln!(
                     ui.warning_default(),
diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs
index d98d9acf9b..cf1c22428e 100644
--- a/cli/src/command_error.rs
+++ b/cli/src/command_error.rs
@@ -661,6 +661,9 @@ fn fileset_parse_error_hint(err: &FilesetParseError) -> Option<String> {
         FilesetParseErrorKind::InvalidArguments { .. } | FilesetParseErrorKind::Expression(_) => {
             find_source_parse_error_hint(&err)
         }
+        FilesetParseErrorKind::InAliasExpansion(_)
+        | FilesetParseErrorKind::InParameterExpansion(_)
+        | FilesetParseErrorKind::RecursiveAlias(_) => None,
     }
 }
 
diff --git a/cli/src/commands/debug/fileset.rs b/cli/src/commands/debug/fileset.rs
index de4154375b..a2bfbde214 100644
--- a/cli/src/commands/debug/fileset.rs
+++ b/cli/src/commands/debug/fileset.rs
@@ -37,9 +37,11 @@ pub fn cmd_debug_fileset(
 ) -> Result<(), CommandError> {
     let workspace_command = command.workspace_helper(ui)?;
     let path_converter = workspace_command.path_converter();
+    let aliases_map = workspace_command.fileset_aliases_map();
 
     let mut diagnostics = FilesetDiagnostics::new();
-    let expression = fileset::parse_maybe_bare(&mut diagnostics, &args.path, path_converter)?;
+    let expression =
+        fileset::parse_maybe_bare(&mut diagnostics, &args.path, path_converter, aliases_map)?;
     print_parse_diagnostics(ui, "In fileset expression", &diagnostics)?;
     writeln!(ui.stdout(), "-- Parsed:")?;
     writeln!(ui.stdout(), "{expression:#?}")?;
diff --git a/cli/src/commands/fix.rs b/cli/src/commands/fix.rs
index 6a6508bfbe..f98b4305f9 100644
--- a/cli/src/commands/fix.rs
+++ b/cli/src/commands/fix.rs
@@ -492,6 +492,7 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
                                     cwd: "".into(),
                                     base: "".into(),
                                 },
+                                &Default::default(),
                             )
                         })
                         .try_collect()?,
diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs
index 7b6b542382..76345f22db 100644
--- a/cli/src/commit_templater.rs
+++ b/cli/src/commit_templater.rs
@@ -854,8 +854,9 @@ fn expect_fileset_literal(
 ) -> Result<FilesetExpression, TemplateParseError> {
     template_parser::expect_string_literal_with(node, |text, span| {
         let mut inner_diagnostics = FilesetDiagnostics::new();
-        let expression =
-            fileset::parse(&mut inner_diagnostics, text, path_converter).map_err(|err| {
+        let aliases_map = Default::default();
+        let expression = fileset::parse(&mut inner_diagnostics, text, path_converter, &aliases_map)
+            .map_err(|err| {
                 TemplateParseError::expression("In fileset expression", span).with_source(err)
             })?;
         diagnostics.extend_with(inner_diagnostics, |diag| {
diff --git a/lib/src/fileset.pest b/lib/src/fileset.pest
index 534198631b..8329e33b89 100644
--- a/lib/src/fileset.pest
+++ b/lib/src/fileset.pest
@@ -89,3 +89,5 @@ program_or_bare_string = _{
         | bare_string_pattern ~ EOI
         | bare_string ~ EOI )
 }
+
+alias_declaration = _{ SOI ~ identifier ~ EOI }
diff --git a/lib/src/fileset.rs b/lib/src/fileset.rs
index ce6fd22200..8941b704da 100644
--- a/lib/src/fileset.rs
+++ b/lib/src/fileset.rs
@@ -23,11 +23,14 @@ use itertools::Itertools as _;
 use once_cell::sync::Lazy;
 use thiserror::Error;
 
+use crate::dsl_util;
 use crate::dsl_util::collect_similar;
+use crate::dsl_util::AliasExpandError;
 use crate::fileset_parser;
 use crate::fileset_parser::BinaryOp;
 use crate::fileset_parser::ExpressionKind;
 use crate::fileset_parser::ExpressionNode;
+pub use crate::fileset_parser::FilesetAliasesMap;
 pub use crate::fileset_parser::FilesetDiagnostics;
 pub use crate::fileset_parser::FilesetParseError;
 pub use crate::fileset_parser::FilesetParseErrorKind;
@@ -465,6 +468,15 @@ fn resolve_expression(
         ExpressionKind::FunctionCall(function) => {
             resolve_function(diagnostics, path_converter, function)
         }
+        ExpressionKind::AliasExpanded(id, subst) => {
+            let mut inner_diagnostics = FilesetDiagnostics::new();
+            let expression = resolve_expression(&mut inner_diagnostics, path_converter, subst)
+                .map_err(|e| e.within_alias_expansion(*id, node.span))?;
+            diagnostics.extend_with(inner_diagnostics, |diag| {
+                diag.within_alias_expansion(*id, node.span)
+            });
+            Ok(expression)
+        }
     }
 }
 
@@ -473,9 +485,11 @@ pub fn parse(
     diagnostics: &mut FilesetDiagnostics,
     text: &str,
     path_converter: &RepoPathUiConverter,
+    aliases_map: &FilesetAliasesMap,
 ) -> FilesetParseResult<FilesetExpression> {
     let node = fileset_parser::parse_program(text)?;
     // TODO: add basic tree substitution pass to eliminate redundant expressions
+    let node = dsl_util::expand_aliases(node, aliases_map)?;
     resolve_expression(diagnostics, path_converter, &node)
 }
 
@@ -487,9 +501,11 @@ pub fn parse_maybe_bare(
     diagnostics: &mut FilesetDiagnostics,
     text: &str,
     path_converter: &RepoPathUiConverter,
+    aliases_map: &FilesetAliasesMap,
 ) -> FilesetParseResult<FilesetExpression> {
     let node = fileset_parser::parse_program_or_bare_string(text)?;
     // TODO: add basic tree substitution pass to eliminate redundant expressions
+    let node = dsl_util::expand_aliases(node, aliases_map)?;
     resolve_expression(diagnostics, path_converter, &node)
 }
 
@@ -529,7 +545,15 @@ mod tests {
             cwd: PathBuf::from("/ws/cur"),
             base: PathBuf::from("/ws"),
         };
-        let parse = |text| parse_maybe_bare(&mut FilesetDiagnostics::new(), text, &path_converter);
+        let aliases_map = Default::default();
+        let parse = |text| {
+            parse_maybe_bare(
+                &mut FilesetDiagnostics::new(),
+                text,
+                &path_converter,
+                &aliases_map,
+            )
+        };
 
         // cwd-relative patterns
         insta::assert_debug_snapshot!(
@@ -574,7 +598,15 @@ mod tests {
             cwd: PathBuf::from("/ws/cur*"),
             base: PathBuf::from("/ws"),
         };
-        let parse = |text| parse_maybe_bare(&mut FilesetDiagnostics::new(), text, &path_converter);
+        let aliases_map = Default::default();
+        let parse = |text| {
+            parse_maybe_bare(
+                &mut FilesetDiagnostics::new(),
+                text,
+                &path_converter,
+                &aliases_map,
+            )
+        };
 
         // cwd-relative, without meta characters
         insta::assert_debug_snapshot!(
@@ -747,7 +779,15 @@ mod tests {
             cwd: PathBuf::from("/ws/cur"),
             base: PathBuf::from("/ws"),
         };
-        let parse = |text| parse_maybe_bare(&mut FilesetDiagnostics::new(), text, &path_converter);
+        let aliases_map = Default::default();
+        let parse = |text| {
+            parse_maybe_bare(
+                &mut FilesetDiagnostics::new(),
+                text,
+                &path_converter,
+                &aliases_map,
+            )
+        };
 
         insta::assert_debug_snapshot!(parse("all()").unwrap(), @"All");
         insta::assert_debug_snapshot!(parse("none()").unwrap(), @"None");
@@ -775,7 +815,15 @@ mod tests {
             cwd: PathBuf::from("/ws/cur"),
             base: PathBuf::from("/ws"),
         };
-        let parse = |text| parse_maybe_bare(&mut FilesetDiagnostics::new(), text, &path_converter);
+        let aliases_map = Default::default();
+        let parse = |text| {
+            parse_maybe_bare(
+                &mut FilesetDiagnostics::new(),
+                text,
+                &path_converter,
+                &aliases_map,
+            )
+        };
 
         insta::assert_debug_snapshot!(parse("~x").unwrap(), @r###"
         Difference(
diff --git a/lib/src/fileset_parser.rs b/lib/src/fileset_parser.rs
index 2091fb96a5..569c32fa95 100644
--- a/lib/src/fileset_parser.rs
+++ b/lib/src/fileset_parser.rs
@@ -27,7 +27,16 @@ use pest_derive::Parser;
 use thiserror::Error;
 
 use crate::dsl_util;
+use crate::dsl_util::AliasDeclaration;
+use crate::dsl_util::AliasDeclarationParser;
+use crate::dsl_util::AliasDefinitionParser;
+use crate::dsl_util::AliasExpandError;
+use crate::dsl_util::AliasExpandableExpression;
+use crate::dsl_util::AliasId;
+use crate::dsl_util::AliasesMap;
 use crate::dsl_util::Diagnostics;
+use crate::dsl_util::ExpressionFolder;
+use crate::dsl_util::FoldableExpression;
 use crate::dsl_util::InvalidArguments;
 use crate::dsl_util::StringLiteralParser;
 
@@ -71,6 +80,7 @@ impl Rule {
             Rule::expression => None,
             Rule::program => None,
             Rule::program_or_bare_string => None,
+            Rule::alias_declaration => None,
         }
     }
 }
@@ -106,6 +116,12 @@ pub enum FilesetParseErrorKind {
     InvalidArguments { name: String, message: String },
     #[error("{0}")]
     Expression(String),
+    #[error(r#"In alias "{0}""#)]
+    InAliasExpansion(String),
+    #[error(r#"In function parameter "{0}""#)]
+    InParameterExpansion(String),
+    #[error(r#"Alias "{0}" expanded recursively"#)]
+    RecursiveAlias(String),
 }
 
 impl FilesetParseError {
@@ -141,6 +157,26 @@ impl FilesetParseError {
     }
 }
 
+impl AliasExpandError for FilesetParseError {
+    fn invalid_arguments(err: InvalidArguments<'_>) -> Self {
+        err.into()
+    }
+
+    fn recursive_expansion(id: AliasId<'_>, span: pest::Span<'_>) -> Self {
+        Self::new(FilesetParseErrorKind::RecursiveAlias(id.to_string()), span)
+    }
+
+    fn within_alias_expansion(self, id: AliasId<'_>, span: pest::Span<'_>) -> Self {
+        let kind = match id {
+            AliasId::Symbol(_) | AliasId::Function(..) => {
+                FilesetParseErrorKind::InAliasExpansion(id.to_string())
+            }
+            AliasId::Parameter(_) => FilesetParseErrorKind::InParameterExpansion(id.to_string()),
+        };
+        Self::new(kind, span).with_source(self)
+    }
+}
+
 impl From<pest::error::Error<Rule>> for FilesetParseError {
     fn from(err: pest::error::Error<Rule>) -> Self {
         FilesetParseError {
@@ -182,6 +218,51 @@ pub enum ExpressionKind<'i> {
     /// `x | y | ..`
     UnionAll(Vec<ExpressionNode<'i>>),
     FunctionCall(Box<FunctionCallNode<'i>>),
+    /// Identity node to preserve the span in the source template text.
+    AliasExpanded(AliasId<'i>, Box<ExpressionNode<'i>>),
+}
+
+impl<'i> FoldableExpression<'i> for ExpressionKind<'i> {
+    fn fold<F>(self, folder: &mut F, span: pest::Span<'i>) -> Result<Self, F::Error>
+    where
+        F: ExpressionFolder<'i, Self> + ?Sized,
+    {
+        match self {
+            ExpressionKind::Identifier(name) => folder.fold_identifier(name, span),
+            ExpressionKind::String(_) | ExpressionKind::StringPattern { .. } => Ok(self),
+            ExpressionKind::Unary(op, arg) => {
+                let arg = Box::new(folder.fold_expression(*arg)?);
+                Ok(ExpressionKind::Unary(op, arg))
+            }
+            ExpressionKind::Binary(op, lhs, rhs) => {
+                let lhs = Box::new(folder.fold_expression(*lhs)?);
+                let rhs = Box::new(folder.fold_expression(*rhs)?);
+                Ok(ExpressionKind::Binary(op, lhs, rhs))
+            }
+            ExpressionKind::UnionAll(nodes) => Ok(ExpressionKind::UnionAll(
+                dsl_util::fold_expression_nodes(folder, nodes)?,
+            )),
+            ExpressionKind::FunctionCall(function) => folder.fold_function_call(function, span),
+            ExpressionKind::AliasExpanded(id, subst) => {
+                let subst = Box::new(folder.fold_expression(*subst)?);
+                Ok(ExpressionKind::AliasExpanded(id, subst))
+            }
+        }
+    }
+}
+
+impl<'i> AliasExpandableExpression<'i> for ExpressionKind<'i> {
+    fn identifier(name: &'i str) -> Self {
+        ExpressionKind::Identifier(name)
+    }
+
+    fn function_call(function: Box<FunctionCallNode<'i>>) -> Self {
+        ExpressionKind::FunctionCall(function)
+    }
+
+    fn alias_expanded(id: AliasId<'i>, subst: Box<ExpressionNode<'i>>) -> Self {
+        ExpressionKind::AliasExpanded(id, subst)
+    }
 }
 
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
@@ -346,6 +427,33 @@ pub fn parse_program_or_bare_string(text: &str) -> FilesetParseResult<Expression
     Ok(ExpressionNode::new(expr, span))
 }
 
+pub type FilesetAliasesMap = AliasesMap<FilesetAliasParser, String>;
+
+#[derive(Clone, Debug, Default)]
+pub struct FilesetAliasParser;
+
+impl AliasDeclarationParser for FilesetAliasParser {
+    type Error = FilesetParseError;
+
+    fn parse_declaration(&self, source: &str) -> Result<AliasDeclaration, Self::Error> {
+        let mut pairs = FilesetParser::parse(Rule::alias_declaration, source)?;
+        let first = pairs.next().unwrap();
+        match first.as_rule() {
+            Rule::identifier => Ok(AliasDeclaration::Symbol(first.as_str().to_owned())),
+            r => panic!("unexpected alias declaration rule {r:?}"),
+        }
+    }
+}
+
+impl AliasDefinitionParser for FilesetAliasParser {
+    type Output<'i> = ExpressionKind<'i>;
+    type Error = FilesetParseError;
+
+    fn parse_definition<'i>(&self, source: &'i str) -> Result<ExpressionNode<'i>, Self::Error> {
+        parse_program_or_bare_string(source)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use assert_matches::assert_matches;
@@ -422,6 +530,7 @@ mod tests {
                 let function = Box::new(normalize_function_call(*function));
                 ExpressionKind::FunctionCall(function)
             }
+            ExpressionKind::AliasExpanded(_, subst) => normalize_tree(*subst).kind,
         };
         ExpressionNode {
             kind: normalized_kind,
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 5d1d8162ba..40c9dcc6b7 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -35,6 +35,7 @@ use crate::dsl_util;
 use crate::dsl_util::collect_similar;
 use crate::dsl_util::AliasExpandError as _;
 use crate::fileset;
+use crate::fileset::FilesetAliasesMap;
 use crate::fileset::FilesetDiagnostics;
 use crate::fileset::FilesetExpression;
 use crate::graph::GraphNode;
@@ -894,7 +895,9 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
             ));
         }
         let file_expressions = itertools::chain([arg], args)
-            .map(|arg| expect_fileset_expression(diagnostics, arg, ctx.path_converter))
+            .map(|arg| {
+                expect_fileset_expression(diagnostics, arg, ctx.path_converter, &Default::default())
+            })
             .try_collect()?;
         let expr = FilesetExpression::union_all(file_expressions);
         Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr)))
@@ -911,7 +914,12 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
                     files_arg.span,
                 )
             })?;
-            expect_fileset_expression(diagnostics, files_arg, ctx.path_converter)?
+            expect_fileset_expression(
+                diagnostics,
+                files_arg,
+                ctx.path_converter,
+                &Default::default(),
+            )?
         } else {
             // TODO: defaults to CLI path arguments?
             // https://github.com/martinvonz/jj/issues/2933#issuecomment-1925870731
@@ -968,16 +976,22 @@ pub fn expect_fileset_expression(
     diagnostics: &mut RevsetDiagnostics,
     node: &ExpressionNode,
     path_converter: &RepoPathUiConverter,
+    aliases_map: &FilesetAliasesMap,
 ) -> Result<FilesetExpression, RevsetParseError> {
     // Alias handling is a bit tricky. The outermost expression `alias` is
     // substituted, but inner expressions `x & alias` aren't. If this seemed
     // weird, we can either transform AST or turn off revset aliases completely.
     revset_parser::expect_expression_with(diagnostics, node, |diagnostics, node| {
         let mut inner_diagnostics = FilesetDiagnostics::new();
-        let expression = fileset::parse(&mut inner_diagnostics, node.span.as_str(), path_converter)
-            .map_err(|err| {
-                RevsetParseError::expression("In fileset expression", node.span).with_source(err)
-            })?;
+        let expression = fileset::parse(
+            &mut inner_diagnostics,
+            node.span.as_str(),
+            path_converter,
+            aliases_map,
+        )
+        .map_err(|err| {
+            RevsetParseError::expression("In fileset expression", node.span).with_source(err)
+        })?;
         diagnostics.extend_with(inner_diagnostics, |diag| {
             RevsetParseError::expression("In fileset expression", node.span).with_source(diag)
         });

From c7ae396907a07dad8d9e821902018b5dbfb01b2c Mon Sep 17 00:00:00 2001
From: Bryce Berger <bryce.z.berger@gmail.com>
Date: Tue, 26 Nov 2024 16:25:17 -0500
Subject: [PATCH 2/4] revset: add &FilesetAliasesMap to RevsetParseContext

This propogates `[fileset-aliases]` to everywhere they're used. Chiefly,
they need to be accessible inside of expressions like:

    jj log -T 'self.diff("...").summary()'
    jj log -r 'diff_contains("words", "...")'
---
 cli/src/cli_util.rs         |  1 +
 cli/src/commands/fix.rs     | 12 ++++++--
 cli/src/commit_templater.rs | 12 ++++++--
 lib/src/fileset.pest        |  2 +-
 lib/src/fileset_parser.rs   |  4 ++-
 lib/src/revset.rs           | 60 ++++++++++++++++++++++++++-----------
 lib/tests/test_revset.rs    | 18 +++++++++--
 7 files changed, 82 insertions(+), 27 deletions(-)

diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index 21292b57b0..86681ca836 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -771,6 +771,7 @@ impl WorkspaceCommandEnvironment {
             now.into(),
             self.command.revset_extensions(),
             Some(workspace_context),
+            &self.fileset_aliases_map,
         )
     }
 
diff --git a/cli/src/commands/fix.rs b/cli/src/commands/fix.rs
index f98b4305f9..5b697ab07e 100644
--- a/cli/src/commands/fix.rs
+++ b/cli/src/commands/fix.rs
@@ -48,6 +48,7 @@ use tracing::instrument;
 
 use crate::cli_util::CommandHelper;
 use crate::cli_util::RevisionArg;
+use crate::cli_util::WorkspaceCommandHelper;
 use crate::command_error::config_error;
 use crate::command_error::print_parse_diagnostics;
 use crate::command_error::CommandError;
@@ -145,7 +146,7 @@ pub(crate) fn cmd_fix(
     args: &FixArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
-    let tools_config = get_tools_config(ui, command.settings())?;
+    let tools_config = get_tools_config(ui, &workspace_command, command.settings())?;
     let root_commits: Vec<CommitId> = if args.source.is_empty() {
         let revs = command.settings().get_string("revsets.fix")?;
         workspace_command.parse_revset(ui, &RevisionArg::from(revs))?
@@ -446,7 +447,11 @@ struct RawToolConfig {
 /// Fails if any of the commands or patterns are obviously unusable, but does
 /// not check for issues that might still occur later like missing executables.
 /// This is a place where we could fail earlier in some cases, though.
-fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig, CommandError> {
+fn get_tools_config(
+    ui: &mut Ui,
+    workspace_command: &WorkspaceCommandHelper,
+    settings: &UserSettings,
+) -> Result<ToolsConfig, CommandError> {
     let mut tools_config = ToolsConfig { tools: Vec::new() };
     // TODO: Remove this block of code and associated documentation after at least
     // one release where the feature is marked deprecated.
@@ -475,6 +480,7 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
     }
     if let Ok(tools_table) = settings.get_table("fix.tools") {
         // Convert the map into a sorted vector early so errors are deterministic.
+        let fileset_aliases_map = workspace_command.fileset_aliases_map();
         let mut tools: Vec<ToolConfig> = tools_table
             .into_iter()
             .sorted_by(|a, b| a.0.cmp(&b.0))
@@ -492,7 +498,7 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
                                     cwd: "".into(),
                                     base: "".into(),
                                 },
-                                &Default::default(),
+                                fileset_aliases_map,
                             )
                         })
                         .try_collect()?,
diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs
index 76345f22db..3c3bc22176 100644
--- a/cli/src/commit_templater.rs
+++ b/cli/src/commit_templater.rs
@@ -29,6 +29,7 @@ use jj_lib::copies::CopiesTreeDiffEntry;
 use jj_lib::copies::CopyRecords;
 use jj_lib::extensions_map::ExtensionsMap;
 use jj_lib::fileset;
+use jj_lib::fileset::FilesetAliasesMap;
 use jj_lib::fileset::FilesetDiagnostics;
 use jj_lib::fileset::FilesetExpression;
 use jj_lib::id_prefix::IdPrefixContext;
@@ -806,7 +807,12 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
         |language, diagnostics, _build_ctx, self_property, function| {
             let ([], [files_node]) = function.expect_arguments()?;
             let files = if let Some(node) = files_node {
-                expect_fileset_literal(diagnostics, node, language.path_converter)?
+                expect_fileset_literal(
+                    diagnostics,
+                    node,
+                    language.path_converter,
+                    language.revset_parse_context.fileset_aliases_map(),
+                )?
             } else {
                 // TODO: defaults to CLI path arguments?
                 // https://github.com/martinvonz/jj/issues/2933#issuecomment-1925870731
@@ -851,11 +857,11 @@ fn expect_fileset_literal(
     diagnostics: &mut TemplateDiagnostics,
     node: &ExpressionNode,
     path_converter: &RepoPathUiConverter,
+    aliases_map: &FilesetAliasesMap,
 ) -> Result<FilesetExpression, TemplateParseError> {
     template_parser::expect_string_literal_with(node, |text, span| {
         let mut inner_diagnostics = FilesetDiagnostics::new();
-        let aliases_map = Default::default();
-        let expression = fileset::parse(&mut inner_diagnostics, text, path_converter, &aliases_map)
+        let expression = fileset::parse(&mut inner_diagnostics, text, path_converter, aliases_map)
             .map_err(|err| {
                 TemplateParseError::expression("In fileset expression", span).with_source(err)
             })?;
diff --git a/lib/src/fileset.pest b/lib/src/fileset.pest
index 8329e33b89..0ef3d15f13 100644
--- a/lib/src/fileset.pest
+++ b/lib/src/fileset.pest
@@ -90,4 +90,4 @@ program_or_bare_string = _{
         | bare_string ~ EOI )
 }
 
-alias_declaration = _{ SOI ~ identifier ~ EOI }
+alias_declaration = _{ SOI ~ strict_identifier ~ EOI }
diff --git a/lib/src/fileset_parser.rs b/lib/src/fileset_parser.rs
index 569c32fa95..5a119d6825 100644
--- a/lib/src/fileset_parser.rs
+++ b/lib/src/fileset_parser.rs
@@ -427,6 +427,8 @@ pub fn parse_program_or_bare_string(text: &str) -> FilesetParseResult<Expression
     Ok(ExpressionNode::new(expr, span))
 }
 
+/// User fileset aliases. When using the CLI, read from the "fileset-aliases"
+/// table.
 pub type FilesetAliasesMap = AliasesMap<FilesetAliasParser, String>;
 
 #[derive(Clone, Debug, Default)]
@@ -439,7 +441,7 @@ impl AliasDeclarationParser for FilesetAliasParser {
         let mut pairs = FilesetParser::parse(Rule::alias_declaration, source)?;
         let first = pairs.next().unwrap();
         match first.as_rule() {
-            Rule::identifier => Ok(AliasDeclaration::Symbol(first.as_str().to_owned())),
+            Rule::strict_identifier => Ok(AliasDeclaration::Symbol(first.as_str().to_owned())),
             r => panic!("unexpected alias declaration rule {r:?}"),
         }
     }
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 40c9dcc6b7..4c02f272e5 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -896,7 +896,12 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
         }
         let file_expressions = itertools::chain([arg], args)
             .map(|arg| {
-                expect_fileset_expression(diagnostics, arg, ctx.path_converter, &Default::default())
+                expect_fileset_expression(
+                    diagnostics,
+                    arg,
+                    ctx.path_converter,
+                    context.fileset_aliases_map,
+                )
             })
             .try_collect()?;
         let expr = FilesetExpression::union_all(file_expressions);
@@ -918,7 +923,7 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
                 diagnostics,
                 files_arg,
                 ctx.path_converter,
-                &Default::default(),
+                context.fileset_aliases_map,
             )?
         } else {
             // TODO: defaults to CLI path arguments?
@@ -2647,6 +2652,7 @@ pub struct RevsetParseContext<'a> {
     date_pattern_context: DatePatternContext,
     extensions: &'a RevsetExtensions,
     workspace: Option<RevsetWorkspaceContext<'a>>,
+    fileset_aliases_map: &'a FilesetAliasesMap,
 }
 
 impl<'a> RevsetParseContext<'a> {
@@ -2656,6 +2662,7 @@ impl<'a> RevsetParseContext<'a> {
         date_pattern_context: DatePatternContext,
         extensions: &'a RevsetExtensions,
         workspace: Option<RevsetWorkspaceContext<'a>>,
+        fileset_aliases_map: &'a FilesetAliasesMap,
     ) -> Self {
         Self {
             aliases_map,
@@ -2663,6 +2670,7 @@ impl<'a> RevsetParseContext<'a> {
             date_pattern_context,
             extensions,
             workspace,
+            fileset_aliases_map,
         }
     }
 
@@ -2670,6 +2678,10 @@ impl<'a> RevsetParseContext<'a> {
         self.aliases_map
     }
 
+    pub fn fileset_aliases_map(&self) -> &'a FilesetAliasesMap {
+        self.fileset_aliases_map
+    }
+
     pub fn user_email(&self) -> &str {
         &self.user_email
     }
@@ -2695,28 +2707,43 @@ mod tests {
     use std::path::PathBuf;
 
     use assert_matches::assert_matches;
+    use dsl_util::AliasesMap;
 
     use super::*;
 
+    const EMPTY: [(&str, &str); 0] = [];
+
+    fn get_alias_map<T>(
+        aliases: impl IntoIterator<Item = (impl AsRef<str>, impl Into<String>)>,
+    ) -> AliasesMap<T, String>
+    where
+        T: dsl_util::AliasDeclarationParser + Default,
+        T::Error: std::fmt::Debug,
+    {
+        let mut aliases_map = AliasesMap::new();
+        for (decl, defn) in aliases {
+            aliases_map.insert(decl, defn).unwrap();
+        }
+        aliases_map
+    }
+
     fn parse(revset_str: &str) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
-        parse_with_aliases(revset_str, [] as [(&str, &str); 0])
+        parse_with_aliases(revset_str, EMPTY)
     }
 
     fn parse_with_workspace(
         revset_str: &str,
         workspace_id: &WorkspaceId,
     ) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
-        parse_with_aliases_and_workspace(revset_str, [] as [(&str, &str); 0], workspace_id)
+        parse_with_aliases_and_workspace(revset_str, EMPTY, workspace_id)
     }
 
     fn parse_with_aliases(
         revset_str: &str,
         aliases: impl IntoIterator<Item = (impl AsRef<str>, impl Into<String>)>,
     ) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
-        let mut aliases_map = RevsetAliasesMap::new();
-        for (decl, defn) in aliases {
-            aliases_map.insert(decl, defn).unwrap();
-        }
+        let aliases_map = get_alias_map(aliases);
+        let fileset_aliases_map = get_alias_map(EMPTY);
         let extensions = RevsetExtensions::default();
         let context = RevsetParseContext::new(
             &aliases_map,
@@ -2724,6 +2751,7 @@ mod tests {
             chrono::Utc::now().fixed_offset().into(),
             &extensions,
             None,
+            &fileset_aliases_map,
         );
         super::parse(&mut RevsetDiagnostics::new(), revset_str, &context)
     }
@@ -2742,10 +2770,8 @@ mod tests {
             path_converter: &path_converter,
             workspace_id,
         };
-        let mut aliases_map = RevsetAliasesMap::new();
-        for (decl, defn) in aliases {
-            aliases_map.insert(decl, defn).unwrap();
-        }
+        let aliases_map = get_alias_map(aliases);
+        let fileset_aliases_map = get_alias_map(EMPTY);
         let extensions = RevsetExtensions::default();
         let context = RevsetParseContext::new(
             &aliases_map,
@@ -2753,6 +2779,7 @@ mod tests {
             chrono::Utc::now().fixed_offset().into(),
             &extensions,
             Some(workspace_ctx),
+            &fileset_aliases_map,
         );
         super::parse(&mut RevsetDiagnostics::new(), revset_str, &context)
     }
@@ -2760,17 +2787,15 @@ mod tests {
     fn parse_with_modifier(
         revset_str: &str,
     ) -> Result<(Rc<UserRevsetExpression>, Option<RevsetModifier>), RevsetParseError> {
-        parse_with_aliases_and_modifier(revset_str, [] as [(&str, &str); 0])
+        parse_with_aliases_and_modifier(revset_str, EMPTY)
     }
 
     fn parse_with_aliases_and_modifier(
         revset_str: &str,
         aliases: impl IntoIterator<Item = (impl AsRef<str>, impl Into<String>)>,
     ) -> Result<(Rc<UserRevsetExpression>, Option<RevsetModifier>), RevsetParseError> {
-        let mut aliases_map = RevsetAliasesMap::new();
-        for (decl, defn) in aliases {
-            aliases_map.insert(decl, defn).unwrap();
-        }
+        let aliases_map = get_alias_map(aliases);
+        let fileset_aliases_map = get_alias_map(EMPTY);
         let extensions = RevsetExtensions::default();
         let context = RevsetParseContext::new(
             &aliases_map,
@@ -2778,6 +2803,7 @@ mod tests {
             chrono::Utc::now().fixed_offset().into(),
             &extensions,
             None,
+            &fileset_aliases_map,
         );
         super::parse_with_modifier(&mut RevsetDiagnostics::new(), revset_str, &context)
     }
diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs
index 51ee358de4..eb31d2ec89 100644
--- a/lib/tests/test_revset.rs
+++ b/lib/tests/test_revset.rs
@@ -23,6 +23,7 @@ use jj_lib::backend::MillisSinceEpoch;
 use jj_lib::backend::Signature;
 use jj_lib::backend::Timestamp;
 use jj_lib::commit::Commit;
+use jj_lib::fileset::FilesetAliasesMap;
 use jj_lib::fileset::FilesetExpression;
 use jj_lib::git;
 use jj_lib::git_backend::GitBackend;
@@ -68,9 +69,16 @@ fn resolve_symbol_with_extensions(
     symbol: &str,
 ) -> Result<Vec<CommitId>, RevsetResolutionError> {
     let aliases_map = RevsetAliasesMap::default();
+    let fileset_aliases_map = FilesetAliasesMap::default();
     let now = chrono::Local::now();
-    let context =
-        RevsetParseContext::new(&aliases_map, String::new(), now.into(), extensions, None);
+    let context = RevsetParseContext::new(
+        &aliases_map,
+        String::new(),
+        now.into(),
+        extensions,
+        None,
+        &fileset_aliases_map,
+    );
     let expression = parse(&mut RevsetDiagnostics::new(), symbol, &context).unwrap();
     assert_matches!(*expression, RevsetExpression::CommitRef(_));
     let symbol_resolver = DefaultSymbolResolver::new(repo, extensions.symbol_resolvers());
@@ -206,6 +214,7 @@ fn test_resolve_symbol_commit_id() {
         &([] as [&Box<dyn SymbolResolverExtension>; 0]),
     );
     let aliases_map = RevsetAliasesMap::default();
+    let fileset_aliases_map = FilesetAliasesMap::default();
     let extensions = RevsetExtensions::default();
     let context = RevsetParseContext::new(
         &aliases_map,
@@ -213,6 +222,7 @@ fn test_resolve_symbol_commit_id() {
         chrono::Utc::now().fixed_offset().into(),
         &extensions,
         None,
+        &fileset_aliases_map,
     );
     assert_matches!(
         parse(&mut RevsetDiagnostics::new(), "present(04)", &context).unwrap()
@@ -921,6 +931,7 @@ fn try_resolve_commit_ids(
 ) -> Result<Vec<CommitId>, RevsetResolutionError> {
     let settings = testutils::user_settings();
     let aliases_map = RevsetAliasesMap::default();
+    let fileset_aliases_map = FilesetAliasesMap::default();
     let revset_extensions = RevsetExtensions::default();
     let context = RevsetParseContext::new(
         &aliases_map,
@@ -928,6 +939,7 @@ fn try_resolve_commit_ids(
         chrono::Utc::now().fixed_offset().into(),
         &revset_extensions,
         None,
+        &fileset_aliases_map,
     );
     let expression = parse(&mut RevsetDiagnostics::new(), revset_str, &context).unwrap();
     let symbol_resolver = DefaultSymbolResolver::new(repo, revset_extensions.symbol_resolvers());
@@ -956,6 +968,7 @@ fn resolve_commit_ids_in_workspace(
         workspace_id: workspace.workspace_id(),
     };
     let aliases_map = RevsetAliasesMap::default();
+    let fileset_aliases_map = FilesetAliasesMap::default();
     let extensions = RevsetExtensions::default();
     let context = RevsetParseContext::new(
         &aliases_map,
@@ -963,6 +976,7 @@ fn resolve_commit_ids_in_workspace(
         chrono::Utc::now().fixed_offset().into(),
         &extensions,
         Some(workspace_ctx),
+        &fileset_aliases_map,
     );
     let expression = parse(&mut RevsetDiagnostics::new(), revset_str, &context).unwrap();
     let symbol_resolver =

From 478047b72f8ff144f126fc0bac25f3fba432aacb Mon Sep 17 00:00:00 2001
From: Bryce Berger <bryce.z.berger@gmail.com>
Date: Thu, 28 Nov 2024 12:03:56 -0500
Subject: [PATCH 3/4] fileset: add test for alias expansion

---
 lib/src/fileset.rs        |   7 +--
 lib/src/fileset_parser.rs | 103 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/lib/src/fileset.rs b/lib/src/fileset.rs
index 8941b704da..8ad560d6a4 100644
--- a/lib/src/fileset.rs
+++ b/lib/src/fileset.rs
@@ -23,7 +23,6 @@ use itertools::Itertools as _;
 use once_cell::sync::Lazy;
 use thiserror::Error;
 
-use crate::dsl_util;
 use crate::dsl_util::collect_similar;
 use crate::dsl_util::AliasExpandError;
 use crate::fileset_parser;
@@ -487,9 +486,8 @@ pub fn parse(
     path_converter: &RepoPathUiConverter,
     aliases_map: &FilesetAliasesMap,
 ) -> FilesetParseResult<FilesetExpression> {
-    let node = fileset_parser::parse_program(text)?;
+    let node = fileset_parser::parse(text, aliases_map)?;
     // TODO: add basic tree substitution pass to eliminate redundant expressions
-    let node = dsl_util::expand_aliases(node, aliases_map)?;
     resolve_expression(diagnostics, path_converter, &node)
 }
 
@@ -503,9 +501,8 @@ pub fn parse_maybe_bare(
     path_converter: &RepoPathUiConverter,
     aliases_map: &FilesetAliasesMap,
 ) -> FilesetParseResult<FilesetExpression> {
-    let node = fileset_parser::parse_program_or_bare_string(text)?;
+    let node = fileset_parser::parse_maybe_bare(text, aliases_map)?;
     // TODO: add basic tree substitution pass to eliminate redundant expressions
-    let node = dsl_util::expand_aliases(node, aliases_map)?;
     resolve_expression(diagnostics, path_converter, &node)
 }
 
diff --git a/lib/src/fileset_parser.rs b/lib/src/fileset_parser.rs
index 5a119d6825..ba6593b3bb 100644
--- a/lib/src/fileset_parser.rs
+++ b/lib/src/fileset_parser.rs
@@ -394,19 +394,37 @@ fn parse_expression_node(pair: Pair<Rule>) -> FilesetParseResult<ExpressionNode>
         .parse(pair.into_inner())
 }
 
-/// Parses text into expression tree. No name resolution is made at this stage.
-pub fn parse_program(text: &str) -> FilesetParseResult<ExpressionNode> {
+/// Parses text into expression tree. No path resolution is made at this stage,
+/// see [`crate::fileset::parse()`].
+pub fn parse<'i>(
+    text: &'i str,
+    aliases_map: &'i FilesetAliasesMap,
+) -> FilesetParseResult<ExpressionNode<'i>> {
+    let node = parse_program(text)?;
+    dsl_util::expand_aliases(node, aliases_map)
+}
+
+fn parse_program(text: &str) -> FilesetParseResult<ExpressionNode> {
     let mut pairs = FilesetParser::parse(Rule::program, text)?;
     let first = pairs.next().unwrap();
     parse_expression_node(first)
 }
 
-/// Parses text into expression tree with bare string fallback. No name
-/// resolution is made at this stage.
+/// Parses text into expression tree with bare string fallback. No path
+/// resolution is made at this stage, see
+/// [`crate::fileset::parse_maybe_bare()`].
 ///
 /// If the text can't be parsed as a fileset expression, and if it doesn't
 /// contain any operator-like characters, it will be parsed as a file path.
-pub fn parse_program_or_bare_string(text: &str) -> FilesetParseResult<ExpressionNode> {
+pub fn parse_maybe_bare<'i>(
+    text: &'i str,
+    aliases_map: &'i FilesetAliasesMap,
+) -> FilesetParseResult<ExpressionNode<'i>> {
+    let node = parse_program_or_bare_string(text)?;
+    dsl_util::expand_aliases(node, aliases_map)
+}
+
+fn parse_program_or_bare_string(text: &str) -> FilesetParseResult<ExpressionNode> {
     let mut pairs = FilesetParser::parse(Rule::program_or_bare_string, text)?;
     let first = pairs.next().unwrap();
     let span = first.as_span();
@@ -463,6 +481,30 @@ mod tests {
     use super::*;
     use crate::dsl_util::KeywordArgument;
 
+    #[derive(Debug)]
+    struct WithFilesetAliasesMap(FilesetAliasesMap);
+
+    impl WithFilesetAliasesMap {
+        fn parse<'i>(&'i self, text: &'i str) -> Result<ExpressionNode<'i>, FilesetParseError> {
+            let node = parse_program(text)?;
+            dsl_util::expand_aliases(node, &self.0)
+        }
+
+        fn parse_normalized<'i>(&'i self, text: &'i str) -> ExpressionNode<'i> {
+            normalize_tree(self.parse(text).unwrap())
+        }
+    }
+
+    fn with_aliases(
+        aliases: impl IntoIterator<Item = (impl AsRef<str>, impl Into<String>)>,
+    ) -> WithFilesetAliasesMap {
+        let mut aliases_map = FilesetAliasesMap::new();
+        for (decl, defn) in aliases {
+            aliases_map.insert(decl, defn).unwrap();
+        }
+        WithFilesetAliasesMap(aliases_map)
+    }
+
     fn parse_into_kind(text: &str) -> Result<ExpressionKind, FilesetParseErrorKind> {
         parse_program(text)
             .map(|node| node.kind)
@@ -834,6 +876,57 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_expand_symbol_alias() {
+        assert_eq!(
+            with_aliases([("AB", "a&b")]).parse_normalized("AB|c"),
+            parse_normalized("(a&b)|c")
+        );
+
+        // Not string substitution 'a&b|c', but tree substitution.
+        assert_eq!(
+            with_aliases([("BC", "b|c")]).parse_normalized("a&BC"),
+            parse_normalized("a&(b|c)")
+        );
+
+        // String literal should not be substituted with alias.
+        assert_eq!(
+            with_aliases([("A", "a")]).parse_normalized(r#"A|"A"|'A'"#),
+            parse_normalized("a|'A'|'A'")
+        );
+
+        // Part of string pattern cannot be substituted.
+        assert_eq!(
+            with_aliases([("A", "a")]).parse_normalized("cwd:A"),
+            parse_normalized("cwd:A")
+        );
+
+        // Multi-level substitution.
+        assert_eq!(
+            with_aliases([("A", "BC"), ("BC", "b|C"), ("C", "c")]).parse_normalized("A"),
+            parse_normalized("b|c")
+        );
+
+        // Infinite recursion, where the top-level error isn't of RecursiveAlias kind.
+        assert_eq!(
+            with_aliases([("A", "A")]).parse("A").unwrap_err().kind,
+            FilesetParseErrorKind::InAliasExpansion("A".to_owned())
+        );
+        assert_eq!(
+            with_aliases([("A", "B"), ("B", "b|C"), ("C", "c|B")])
+                .parse("A")
+                .unwrap_err()
+                .kind,
+            FilesetParseErrorKind::InAliasExpansion("A".to_owned())
+        );
+
+        // Error in alias definition.
+        assert_eq!(
+            with_aliases([("A", "a(")]).parse("A").unwrap_err().kind,
+            FilesetParseErrorKind::InAliasExpansion("A".to_owned())
+        );
+    }
+
     #[test]
     fn test_parse_error() {
         insta::assert_snapshot!(parse_program("foo|").unwrap_err().to_string(), @r###"

From 3eebae12aa9d03f86f3208a87d87eb9ee7ac4f9c Mon Sep 17 00:00:00 2001
From: Bryce Berger <bryce.z.berger@gmail.com>
Date: Thu, 28 Nov 2024 12:09:43 -0500
Subject: [PATCH 4/4] fileset: add docs for aliases

---
 docs/filesets.md | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/docs/filesets.md b/docs/filesets.md
index ec46a7c838..86999acd5a 100644
--- a/docs/filesets.md
+++ b/docs/filesets.md
@@ -61,6 +61,20 @@ You can also specify patterns by using functions.
 * `all()`: Matches everything.
 * `none()`: Matches nothing.
 
+## Aliases
+
+New symbols can be defined in the config file, using predefined symbols/
+functions and other aliases.
+
+Unlike revsets, aliases currently cannot be functions.
+
+For example:
+```toml
+[fileset-aliases]
+LIB = "root-glob:lib/**"
+NO_LOCK = "~root-glob:**/*.lock"
+```
+
 ## Examples
 
 Show diff excluding `Cargo.lock`.