diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index d1d8a523f8..86681ca836 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, @@ -716,6 +720,7 @@ impl WorkspaceCommandEnvironment { fn new(ui: &Ui, command: &CommandHelper, workspace: &Workspace) -> Result { 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(), @@ -765,6 +771,7 @@ impl WorkspaceCommandEnvironment { now.into(), self.command.revset_extensions(), Some(workspace_context), + &self.fileset_aliases_map, ) } @@ -1263,7 +1270,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 +1293,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 +1569,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 +2703,31 @@ fn load_template_aliases( stacked_config: &StackedConfig, ) -> Result { 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 { + let table_name = ConfigNamePathBuf::from_iter(["fileset-aliases"]); + load_aliases(ui, stacked_config, &table_name) +} + +fn load_aliases

( + ui: &Ui, + stacked_config: &StackedConfig, + table_name: &ConfigNamePathBuf, +) -> Result, 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 +2747,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 { 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..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 = 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 { +fn get_tools_config( + ui: &mut Ui, + workspace_command: &WorkspaceCommandHelper, + settings: &UserSettings, +) -> Result { 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 = tools_table .into_iter() .sorted_by(|a, b| a.0.cmp(&b.0)) @@ -492,6 +498,7 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result() -> 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,12 @@ fn expect_fileset_literal( diagnostics: &mut TemplateDiagnostics, node: &ExpressionNode, path_converter: &RepoPathUiConverter, + aliases_map: &FilesetAliasesMap, ) -> Result { 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 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/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`. diff --git a/lib/src/fileset.pest b/lib/src/fileset.pest index 534198631b..0ef3d15f13 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 ~ strict_identifier ~ EOI } diff --git a/lib/src/fileset.rs b/lib/src/fileset.rs index ce6fd22200..8ad560d6a4 100644 --- a/lib/src/fileset.rs +++ b/lib/src/fileset.rs @@ -24,10 +24,12 @@ use once_cell::sync::Lazy; use thiserror::Error; 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 +467,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,8 +484,9 @@ pub fn parse( diagnostics: &mut FilesetDiagnostics, text: &str, path_converter: &RepoPathUiConverter, + aliases_map: &FilesetAliasesMap, ) -> FilesetParseResult { - 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 resolve_expression(diagnostics, path_converter, &node) } @@ -487,8 +499,9 @@ pub fn parse_maybe_bare( diagnostics: &mut FilesetDiagnostics, text: &str, path_converter: &RepoPathUiConverter, + aliases_map: &FilesetAliasesMap, ) -> FilesetParseResult { - 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 resolve_expression(diagnostics, path_converter, &node) } @@ -529,7 +542,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 +595,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 +776,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 +812,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..ba6593b3bb 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> for FilesetParseError { fn from(err: pest::error::Error) -> Self { FilesetParseError { @@ -182,6 +218,51 @@ pub enum ExpressionKind<'i> { /// `x | y | ..` UnionAll(Vec>), FunctionCall(Box>), + /// Identity node to preserve the span in the source template text. + AliasExpanded(AliasId<'i>, Box>), +} + +impl<'i> FoldableExpression<'i> for ExpressionKind<'i> { + fn fold(self, folder: &mut F, span: pest::Span<'i>) -> Result + 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>) -> Self { + ExpressionKind::FunctionCall(function) + } + + fn alias_expanded(id: AliasId<'i>, subst: Box>) -> Self { + ExpressionKind::AliasExpanded(id, subst) + } } #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] @@ -313,19 +394,37 @@ fn parse_expression_node(pair: Pair) -> FilesetParseResult .parse(pair.into_inner()) } -/// Parses text into expression tree. No name resolution is made at this stage. -pub fn parse_program(text: &str) -> FilesetParseResult { +/// 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> { + let node = parse_program(text)?; + dsl_util::expand_aliases(node, aliases_map) +} + +fn parse_program(text: &str) -> FilesetParseResult { 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 { +pub fn parse_maybe_bare<'i>( + text: &'i str, + aliases_map: &'i FilesetAliasesMap, +) -> FilesetParseResult> { + let node = parse_program_or_bare_string(text)?; + dsl_util::expand_aliases(node, aliases_map) +} + +fn parse_program_or_bare_string(text: &str) -> FilesetParseResult { let mut pairs = FilesetParser::parse(Rule::program_or_bare_string, text)?; let first = pairs.next().unwrap(); let span = first.as_span(); @@ -346,6 +445,35 @@ pub fn parse_program_or_bare_string(text: &str) -> FilesetParseResult; + +#[derive(Clone, Debug, Default)] +pub struct FilesetAliasParser; + +impl AliasDeclarationParser for FilesetAliasParser { + type Error = FilesetParseError; + + fn parse_declaration(&self, source: &str) -> Result { + let mut pairs = FilesetParser::parse(Rule::alias_declaration, source)?; + let first = pairs.next().unwrap(); + match first.as_rule() { + Rule::strict_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, Self::Error> { + parse_program_or_bare_string(source) + } +} + #[cfg(test)] mod tests { use assert_matches::assert_matches; @@ -353,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, 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, impl Into)>, + ) -> 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 { parse_program(text) .map(|node| node.kind) @@ -422,6 +574,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, @@ -723,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###" diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 5d1d8162ba..4c02f272e5 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,14 @@ static BUILTIN_FUNCTION_MAP: Lazy> = 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, + context.fileset_aliases_map, + ) + }) .try_collect()?; let expr = FilesetExpression::union_all(file_expressions); Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr))) @@ -911,7 +919,12 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: files_arg.span, ) })?; - expect_fileset_expression(diagnostics, files_arg, ctx.path_converter)? + expect_fileset_expression( + diagnostics, + files_arg, + ctx.path_converter, + context.fileset_aliases_map, + )? } else { // TODO: defaults to CLI path arguments? // https://github.com/martinvonz/jj/issues/2933#issuecomment-1925870731 @@ -968,16 +981,22 @@ pub fn expect_fileset_expression( diagnostics: &mut RevsetDiagnostics, node: &ExpressionNode, path_converter: &RepoPathUiConverter, + aliases_map: &FilesetAliasesMap, ) -> Result { // 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) }); @@ -2633,6 +2652,7 @@ pub struct RevsetParseContext<'a> { date_pattern_context: DatePatternContext, extensions: &'a RevsetExtensions, workspace: Option>, + fileset_aliases_map: &'a FilesetAliasesMap, } impl<'a> RevsetParseContext<'a> { @@ -2642,6 +2662,7 @@ impl<'a> RevsetParseContext<'a> { date_pattern_context: DatePatternContext, extensions: &'a RevsetExtensions, workspace: Option>, + fileset_aliases_map: &'a FilesetAliasesMap, ) -> Self { Self { aliases_map, @@ -2649,6 +2670,7 @@ impl<'a> RevsetParseContext<'a> { date_pattern_context, extensions, workspace, + fileset_aliases_map, } } @@ -2656,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 } @@ -2681,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( + aliases: impl IntoIterator, impl Into)>, + ) -> AliasesMap + 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, 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, 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, impl Into)>, ) -> Result, 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, @@ -2710,6 +2751,7 @@ mod tests { chrono::Utc::now().fixed_offset().into(), &extensions, None, + &fileset_aliases_map, ); super::parse(&mut RevsetDiagnostics::new(), revset_str, &context) } @@ -2728,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, @@ -2739,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) } @@ -2746,17 +2787,15 @@ mod tests { fn parse_with_modifier( revset_str: &str, ) -> Result<(Rc, Option), 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, impl Into)>, ) -> Result<(Rc, Option), 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, @@ -2764,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, 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; 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, 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 =