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`.