From d4537ab656018a3d597e32840e3a8f770fff173c Mon Sep 17 00:00:00 2001 From: Bryce Berger Date: Sun, 24 Nov 2024 02:39:19 -0500 Subject: [PATCH] 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 | 60 ++++++++++++++-- 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, 248 insertions(+), 18 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 0454b78909..cd966438a6 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -61,8 +61,11 @@ use jj_lib::config::ConfigError; use jj_lib::config::ConfigSource; 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; @@ -340,6 +343,14 @@ impl CommandHelper { load_template_aliases(ui, &self.data.stacked_config) } + /// Loads fileset aliases from the configs. + /// + /// For most commands that depend on a loaded repo, you should use + /// `WorkspaceCommandHelper::fileset_aliases_map()` instead. + fn load_fileset_aliases(&self, ui: &Ui) -> Result { + load_fileset_aliases(ui, &self.data.layered_configs) + } + /// Parses template of the given language into evaluation tree. /// /// This function also loads template aliases from the settings. Use @@ -713,6 +724,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, @@ -726,6 +738,7 @@ impl WorkspaceCommandEnvironment { let revset_aliases_map = revset_util::load_revset_aliases(ui, &command.data.stacked_config)?; let template_aliases_map = command.load_template_aliases(ui)?; + let fileset_aliases_map = command.load_fileset_aliases(ui)?; let path_converter = RepoPathUiConverter::Fs { cwd: command.cwd().to_owned(), base: workspace.workspace_root().to_owned(), @@ -734,6 +747,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(), @@ -1273,7 +1287,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)) @@ -1289,6 +1310,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()) @@ -1564,6 +1586,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 @@ -2694,11 +2720,31 @@ fn load_template_aliases( stacked_config: &StackedConfig, ) -> Result { const TABLE_KEY: &str = "template-aliases"; - let mut aliases_map = TemplateAliasesMap::new(); + load_aliases(ui, stacked_config, TABLE_KEY) +} + +fn load_fileset_aliases( + ui: &Ui, + stacked_config: &StackedConfig, +) -> Result { + const TABLE_KEY: &str = "fileset-aliases"; + load_aliases(ui, stacked_config, TABLE_KEY) +} + +fn load_aliases

( + ui: &Ui, + stacked_config: &StackedConfig, + table_key: &str, +) -> 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 = if let Some(table) = layer.data.get_table(TABLE_KEY).optional()? { + let table = if let Some(table) = layer.data.get_table(table_key).optional()? { table } else { continue; @@ -2707,11 +2753,15 @@ fn load_template_aliases( let r = value .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(), - r#"Failed to load "{TABLE_KEY}.{decl}": {s}"# + r#"Failed to load "{table_key}.{decl}": {s}"# )?; } } diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 67ec8d9e96..40960f82b9 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -646,6 +646,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 903d84b1ce..98e620539b 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 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 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 { 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 { 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> 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)] @@ -346,6 +427,33 @@ 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::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; @@ -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> = 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> = 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 { // 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) });