diff --git a/cli/examples/custom-commit-templater/main.rs b/cli/examples/custom-commit-templater/main.rs index 7b75b9df3..37a81179a 100644 --- a/cli/examples/custom-commit-templater/main.rs +++ b/cli/examples/custom-commit-templater/main.rs @@ -29,7 +29,7 @@ use jj_lib::extensions_map::ExtensionsMap; use jj_lib::object_id::ObjectId; use jj_lib::repo::Repo; use jj_lib::revset::{ - self, PartialSymbolResolver, RevsetExpression, RevsetFilterExtension, + self, FunctionCallNode, PartialSymbolResolver, RevsetExpression, RevsetFilterExtension, RevsetFilterExtensionWrapper, RevsetFilterPredicate, RevsetParseError, RevsetResolutionError, SymbolResolverExtension, }; @@ -180,11 +180,10 @@ impl RevsetFilterExtension for EvenDigitsFilter { } fn even_digits( - name: &str, - arguments_pair: pest::iterators::Pair, + function: &FunctionCallNode, _state: revset::ParseState, ) -> Result, RevsetParseError> { - revset::expect_no_arguments(name, arguments_pair)?; + function.expect_no_arguments()?; Ok(RevsetExpression::filter(RevsetFilterPredicate::Extension( RevsetFilterExtensionWrapper(Rc::new(EvenDigitsFilter)), ))) diff --git a/cli/tests/test_revset_output.rs b/cli/tests/test_revset_output.rs index ee49eacbf..26e7098b2 100644 --- a/cli/tests/test_revset_output.rs +++ b/cli/tests/test_revset_output.rs @@ -131,13 +131,13 @@ fn test_bad_function_call() { let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file()"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: Function "file": Expected at least 1 argument + Error: Failed to parse revset: Function "file": Expected at least 1 arguments Caused by: --> 1:6 | 1 | file() | ^ | - = Function "file": Expected at least 1 argument + = Function "file": Expected at least 1 arguments "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file(a, not@a-string)"]); @@ -385,6 +385,12 @@ fn test_alias() { 1 | author(x) | ^ | + = Function parameter "x" cannot be expanded + 3: --> 1:11 + | + 1 | my_author(none()) + | ^----^ + | = Function "author": Expected function argument of string pattern "###); diff --git a/lib/src/dsl_util.rs b/lib/src/dsl_util.rs index f67ea8f55..30144ae8e 100644 --- a/lib/src/dsl_util.rs +++ b/lib/src/dsl_util.rs @@ -441,8 +441,6 @@ pub enum AliasDeclaration { // AliasDeclarationParser and AliasDefinitionParser can be merged into a single // trait, but it's unclear whether doing that would simplify the abstraction. -// For now, they have to be separate traits because revset isn't migrated to -// ExpressionNode tree yet. /// Parser for symbol and function alias declaration. pub trait AliasDeclarationParser { diff --git a/lib/src/revset.rs b/lib/src/revset.rs index d94cecb98..e43c2cb60 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -25,14 +25,12 @@ use std::{error, fmt}; use itertools::Itertools; use once_cell::sync::Lazy; -use pest::iterators::Pair; use thiserror::Error; use crate::backend::{BackendError, BackendResult, ChangeId, CommitId}; use crate::commit::Commit; -use crate::dsl_util::{collect_similar, AliasId}; +use crate::dsl_util::{collect_similar, AliasExpandError as _}; use crate::fileset::{FilePattern, FilesetExpression}; -use crate::git; use crate::graph::GraphEdge; use crate::hex_util::to_forward_hex; use crate::id_prefix::IdPrefixContext; @@ -40,16 +38,13 @@ use crate::object_id::{HexPrefix, PrefixResolution}; use crate::op_store::WorkspaceId; use crate::repo::Repo; use crate::repo_path::RepoPathUiConverter; -// TODO: introduce AST types and remove parse_expression_rule, Rule from the -// re-exports pub use crate::revset_parser::{ - expect_arguments, expect_exact_arguments, expect_named_arguments, expect_named_arguments_vec, - expect_no_arguments, parse_expression_rule, BinaryOp, ExpressionKind, ExpressionNode, - FunctionCallNode, RevsetAliasesMap, RevsetParseError, RevsetParseErrorKind, Rule, UnaryOp, + BinaryOp, ExpressionKind, ExpressionNode, FunctionCallNode, RevsetAliasesMap, RevsetParseError, + RevsetParseErrorKind, UnaryOp, }; -use crate::revset_parser::{parse_program, parse_program_with_modifier}; use crate::store::Store; use crate::str_util::StringPattern; +use crate::{dsl_util, git, revset_parser}; /// Error occurred during symbol resolution. #[derive(Debug, Error)] @@ -555,163 +550,117 @@ impl ResolvedExpression { } } -// TODO: split to parsing and name resolution parts and remove pub(super) +// TODO: maybe replace with RevsetParseContext? #[derive(Clone, Copy, Debug)] pub struct ParseState<'a> { - pub(super) function_map: &'a HashMap<&'static str, RevsetFunction>, - pub(super) aliases_map: &'a RevsetAliasesMap, - aliases_expanding: &'a [AliasId<'a>], - pub(super) locals: &'a HashMap<&'a str, Rc>, + function_map: &'a HashMap<&'static str, RevsetFunction>, user_email: &'a str, - pub(super) workspace_ctx: &'a Option>, - /// Whether or not `kind:"pattern"` syntax is allowed. - pub(super) allow_string_pattern: bool, + workspace_ctx: &'a Option>, } impl<'a> ParseState<'a> { - fn new( - context: &'a RevsetParseContext, - locals: &'a HashMap<&str, Rc>, - ) -> Self { + fn new(context: &'a RevsetParseContext) -> Self { ParseState { function_map: &context.extensions.function_map, - aliases_map: context.aliases_map, - aliases_expanding: &[], - locals, user_email: &context.user_email, workspace_ctx: &context.workspace, - allow_string_pattern: false, } } - - pub(super) fn with_alias_expanding( - self, - id: AliasId<'_>, - locals: &HashMap<&str, Rc>, - span: pest::Span<'_>, - f: impl FnOnce(ParseState<'_>) -> Result, - ) -> Result { - // The stack should be short, so let's simply do linear search and duplicate. - if self.aliases_expanding.contains(&id) { - return Err(RevsetParseError::with_span( - RevsetParseErrorKind::RecursiveAlias(id.to_string()), - span, - )); - } - let mut aliases_expanding = self.aliases_expanding.to_vec(); - aliases_expanding.push(id); - let expanding_state = ParseState { - function_map: self.function_map, - aliases_map: self.aliases_map, - aliases_expanding: &aliases_expanding, - locals, - user_email: self.user_email, - workspace_ctx: self.workspace_ctx, - allow_string_pattern: self.allow_string_pattern, - }; - f(expanding_state).map_err(|e| { - RevsetParseError::with_span( - RevsetParseErrorKind::BadAliasExpansion(id.to_string()), - span, - ) - .with_source(e) - }) - } } pub type RevsetFunction = - fn(&str, Pair, ParseState) -> Result, RevsetParseError>; + fn(&FunctionCallNode, ParseState) -> Result, RevsetParseError>; static BUILTIN_FUNCTION_MAP: Lazy> = Lazy::new(|| { // Not using maplit::hashmap!{} or custom declarative macro here because // code completion inside macro is quite restricted. let mut map: HashMap<&'static str, RevsetFunction> = HashMap::new(); - map.insert("parents", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let expression = parse_expression_rule(arg.into_inner(), state)?; + map.insert("parents", |function, state| { + let [arg] = function.expect_exact_arguments()?; + let expression = parse_expression_rule(arg, state)?; Ok(expression.parents()) }); - map.insert("children", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let expression = parse_expression_rule(arg.into_inner(), state)?; + map.insert("children", |function, state| { + let [arg] = function.expect_exact_arguments()?; + let expression = parse_expression_rule(arg, state)?; Ok(expression.children()) }); - map.insert("ancestors", |name, arguments_pair, state| { - let ([heads_arg], [depth_opt_arg]) = expect_arguments(name, arguments_pair)?; - let heads = parse_expression_rule(heads_arg.into_inner(), state)?; + map.insert("ancestors", |function, state| { + let ([heads_arg], [depth_opt_arg]) = function.expect_arguments()?; + let heads = parse_expression_rule(heads_arg, state)?; let generation = if let Some(depth_arg) = depth_opt_arg { - let depth = parse_function_argument_as_literal("integer", name, depth_arg, state)?; + let depth = parse_function_argument_as_literal("integer", function.name, depth_arg)?; 0..depth } else { GENERATION_RANGE_FULL }; Ok(heads.ancestors_range(generation)) }); - map.insert("descendants", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let expression = parse_expression_rule(arg.into_inner(), state)?; + map.insert("descendants", |function, state| { + let [arg] = function.expect_exact_arguments()?; + let expression = parse_expression_rule(arg, state)?; Ok(expression.descendants()) }); - map.insert("connected", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let candidates = parse_expression_rule(arg.into_inner(), state)?; + map.insert("connected", |function, state| { + let [arg] = function.expect_exact_arguments()?; + let candidates = parse_expression_rule(arg, state)?; Ok(candidates.connected()) }); - map.insert("reachable", |name, arguments_pair, state| { - let [source_arg, domain_arg] = expect_exact_arguments(name, arguments_pair)?; - let sources = parse_expression_rule(source_arg.into_inner(), state)?; - let domain = parse_expression_rule(domain_arg.into_inner(), state)?; + map.insert("reachable", |function, state| { + let [source_arg, domain_arg] = function.expect_exact_arguments()?; + let sources = parse_expression_rule(source_arg, state)?; + let domain = parse_expression_rule(domain_arg, state)?; Ok(sources.reachable(&domain)) }); - map.insert("none", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("none", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::none()) }); - map.insert("all", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("all", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::all()) }); - map.insert("working_copies", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("working_copies", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::working_copies()) }); - map.insert("heads", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let candidates = parse_expression_rule(arg.into_inner(), state)?; + map.insert("heads", |function, state| { + let [arg] = function.expect_exact_arguments()?; + let candidates = parse_expression_rule(arg, state)?; Ok(candidates.heads()) }); - map.insert("roots", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let candidates = parse_expression_rule(arg.into_inner(), state)?; + map.insert("roots", |function, state| { + let [arg] = function.expect_exact_arguments()?; + let candidates = parse_expression_rule(arg, state)?; Ok(candidates.roots()) }); - map.insert("visible_heads", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("visible_heads", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::visible_heads()) }); - map.insert("root", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("root", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::root()) }); - map.insert("branches", |name, arguments_pair, state| { - let ([], [opt_arg]) = expect_arguments(name, arguments_pair)?; + map.insert("branches", |function, _state| { + let ([], [opt_arg]) = function.expect_arguments()?; let pattern = if let Some(arg) = opt_arg { - parse_function_argument_to_string_pattern(name, arg, state)? + parse_function_argument_to_string_pattern(function.name, arg)? } else { StringPattern::everything() }; Ok(RevsetExpression::branches(pattern)) }); - map.insert("remote_branches", |name, arguments_pair, state| { + map.insert("remote_branches", |function, _state| { let ([], [branch_opt_arg, remote_opt_arg]) = - expect_named_arguments(name, &["", "remote"], arguments_pair)?; + function.expect_named_arguments(&["", "remote"])?; let branch_pattern = if let Some(branch_arg) = branch_opt_arg { - parse_function_argument_to_string_pattern(name, branch_arg, state)? + parse_function_argument_to_string_pattern(function.name, branch_arg)? } else { StringPattern::everything() }; let remote_pattern = if let Some(remote_arg) = remote_opt_arg { - parse_function_argument_to_string_pattern(name, remote_arg, state)? + parse_function_argument_to_string_pattern(function.name, remote_arg)? } else { StringPattern::everything() }; @@ -720,186 +669,251 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: remote_pattern, )) }); - map.insert("tags", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("tags", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::tags()) }); - map.insert("git_refs", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("git_refs", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::git_refs()) }); - map.insert("git_head", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("git_head", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::git_head()) }); - map.insert("latest", |name, arguments_pair, state| { - let ([candidates_arg], [count_opt_arg]) = expect_arguments(name, arguments_pair)?; - let candidates = parse_expression_rule(candidates_arg.into_inner(), state)?; + map.insert("latest", |function, state| { + let ([candidates_arg], [count_opt_arg]) = function.expect_arguments()?; + let candidates = parse_expression_rule(candidates_arg, state)?; let count = if let Some(count_arg) = count_opt_arg { - parse_function_argument_as_literal("integer", name, count_arg, state)? + parse_function_argument_as_literal("integer", function.name, count_arg)? } else { 1 }; Ok(candidates.latest(count)) }); - map.insert("merges", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("merges", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::filter( RevsetFilterPredicate::ParentCount(2..u32::MAX), )) }); - map.insert("description", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let pattern = parse_function_argument_to_string_pattern(name, arg, state)?; + map.insert("description", |function, _state| { + let [arg] = function.expect_exact_arguments()?; + let pattern = parse_function_argument_to_string_pattern(function.name, arg)?; Ok(RevsetExpression::filter( RevsetFilterPredicate::Description(pattern), )) }); - map.insert("author", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let pattern = parse_function_argument_to_string_pattern(name, arg, state)?; + map.insert("author", |function, _state| { + let [arg] = function.expect_exact_arguments()?; + let pattern = parse_function_argument_to_string_pattern(function.name, arg)?; Ok(RevsetExpression::filter(RevsetFilterPredicate::Author( pattern, ))) }); - map.insert("mine", |name, arguments_pair, state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("mine", |function, state| { + function.expect_no_arguments()?; Ok(RevsetExpression::filter(RevsetFilterPredicate::Author( StringPattern::Exact(state.user_email.to_owned()), ))) }); - map.insert("committer", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let pattern = parse_function_argument_to_string_pattern(name, arg, state)?; + map.insert("committer", |function, _state| { + let [arg] = function.expect_exact_arguments()?; + let pattern = parse_function_argument_to_string_pattern(function.name, arg)?; Ok(RevsetExpression::filter(RevsetFilterPredicate::Committer( pattern, ))) }); - map.insert("empty", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("empty", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::is_empty()) }); - map.insert("file", |name, arguments_pair, state| { - let arguments_span = arguments_pair.as_span(); + map.insert("file", |function, state| { if let Some(ctx) = state.workspace_ctx { - let file_expressions: Vec<_> = arguments_pair - .into_inner() + let ([arg], args) = function.expect_some_arguments()?; + let file_expressions = itertools::chain([arg], args) .map(|arg| { - parse_function_argument_to_file_pattern(name, arg, state, ctx.path_converter) + parse_function_argument_to_file_pattern(function.name, arg, ctx.path_converter) }) .map_ok(FilesetExpression::pattern) .try_collect()?; - if file_expressions.is_empty() { - Err(RevsetParseError::invalid_arguments( - name, - "Expected at least 1 argument", - arguments_span, - )) - } else { - let expr = FilesetExpression::union_all(file_expressions); - Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr))) - } + let expr = FilesetExpression::union_all(file_expressions); + Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr))) } else { Err(RevsetParseError::with_span( RevsetParseErrorKind::FsPathWithoutWorkspace, - arguments_span, + function.args_span, // TODO: better to use name_span? )) } }); - map.insert("conflict", |name, arguments_pair, _state| { - expect_no_arguments(name, arguments_pair)?; + map.insert("conflict", |function, _state| { + function.expect_no_arguments()?; Ok(RevsetExpression::filter(RevsetFilterPredicate::HasConflict)) }); - map.insert("present", |name, arguments_pair, state| { - let [arg] = expect_exact_arguments(name, arguments_pair)?; - let expression = parse_expression_rule(arg.into_inner(), state)?; + map.insert("present", |function, state| { + let [arg] = function.expect_exact_arguments()?; + let expression = parse_expression_rule(arg, state)?; Ok(Rc::new(RevsetExpression::Present(expression))) }); map }); +// TODO: rename function to expect_file_pattern()? pub fn parse_function_argument_to_file_pattern( name: &str, - pair: Pair, - state: ParseState, + node: &ExpressionNode, path_converter: &RepoPathUiConverter, ) -> Result { let parse_pattern = |value: &str, kind: Option<&str>| match kind { Some(kind) => FilePattern::from_str_kind(path_converter, value, kind), None => FilePattern::cwd_prefix_path(path_converter, value), }; - parse_function_argument_as_pattern("file pattern", name, pair, state, parse_pattern) + parse_function_argument_as_pattern("file pattern", name, node, parse_pattern) } +// TODO: rename function to expect_string_pattern()? pub fn parse_function_argument_to_string_pattern( name: &str, - pair: Pair, - state: ParseState, + node: &ExpressionNode, ) -> Result { let parse_pattern = |value: &str, kind: Option<&str>| match kind { Some(kind) => StringPattern::from_str_kind(value, kind), None => Ok(StringPattern::Substring(value.to_owned())), }; - parse_function_argument_as_pattern("string pattern", name, pair, state, parse_pattern) + parse_function_argument_as_pattern("string pattern", name, node, parse_pattern) } +// TODO: move to revset_parser module? +// TODO: rename function to expect_pattern_with()? fn parse_function_argument_as_pattern>>( type_name: &str, function_name: &str, - pair: Pair, - state: ParseState, + node: &ExpressionNode, parse_pattern: impl FnOnce(&str, Option<&str>) -> Result, ) -> Result { - let span = pair.as_span(); let wrap_error = |err: E| { - RevsetParseError::invalid_arguments(function_name, format!("Invalid {type_name}"), span) - .with_source(err) + RevsetParseError::invalid_arguments( + function_name, + format!("Invalid {type_name}"), + node.span, + ) + .with_source(err) }; - let expression = { - let mut inner_state = state; - inner_state.allow_string_pattern = true; - parse_expression_rule(pair.into_inner(), inner_state)? - }; - match expression.as_ref() { - RevsetExpression::CommitRef(RevsetCommitRef::Symbol(symbol)) => { - parse_pattern(symbol, None).map_err(wrap_error) - } - RevsetExpression::StringPattern { kind, value } => { + revset_parser::expect_literal_with(node, |node| match &node.kind { + ExpressionKind::Identifier(name) => parse_pattern(name, None).map_err(wrap_error), + ExpressionKind::String(name) => parse_pattern(name, None).map_err(wrap_error), + ExpressionKind::StringPattern { kind, value } => { parse_pattern(value, Some(kind)).map_err(wrap_error) } _ => Err(RevsetParseError::invalid_arguments( function_name, format!("Expected function argument of {type_name}"), - span, + node.span, )), - } + }) } +// TODO: move to revset_parser module? +// TODO: rename function to something like expect_literal()? pub fn parse_function_argument_as_literal( type_name: &str, name: &str, - pair: Pair, - state: ParseState, + node: &ExpressionNode, ) -> Result { - let span = pair.as_span(); let make_error = || { RevsetParseError::invalid_arguments( name, format!("Expected function argument of type {type_name}"), - span, + node.span, ) }; - let expression = { - // Don't suggest :: operator for :, which is invalid in this context. - let mut inner_state = state; - inner_state.allow_string_pattern = true; - parse_expression_rule(pair.into_inner(), inner_state)? - }; - match expression.as_ref() { - RevsetExpression::CommitRef(RevsetCommitRef::Symbol(symbol)) => { - symbol.parse().map_err(|_| make_error()) - } + revset_parser::expect_literal_with(node, |node| match &node.kind { + ExpressionKind::Identifier(name) => name.parse().map_err(|_| make_error()), + ExpressionKind::String(name) => name.parse().map_err(|_| make_error()), _ => Err(make_error()), + }) +} + +/// Resolves function call by using the given function map. +fn lower_function_call( + function: &FunctionCallNode, + state: ParseState, +) -> Result, RevsetParseError> { + if let Some(func) = state.function_map.get(function.name) { + func(function, state) + } else { + Err(RevsetParseError::with_span( + RevsetParseErrorKind::NoSuchFunction { + name: function.name.to_owned(), + candidates: collect_similar(function.name, state.function_map.keys()), + }, + function.name_span, + )) + } +} + +// TODO: rename function to lower_expression()?: +pub fn parse_expression_rule( + node: &ExpressionNode, + state: ParseState, +) -> Result, RevsetParseError> { + match &node.kind { + ExpressionKind::Identifier(name) => Ok(RevsetExpression::symbol((*name).to_owned())), + ExpressionKind::String(name) => Ok(RevsetExpression::symbol(name.to_owned())), + ExpressionKind::StringPattern { .. } => Err(RevsetParseError::with_span( + RevsetParseErrorKind::NotInfixOperator { + op: ":".to_owned(), + similar_op: "::".to_owned(), + description: "DAG range".to_owned(), + }, + node.span, + )), + ExpressionKind::RemoteSymbol { name, remote } => Ok(RevsetExpression::remote_symbol( + name.to_owned(), + remote.to_owned(), + )), + ExpressionKind::AtWorkspace(name) => Ok(RevsetExpression::working_copy(WorkspaceId::new( + name.to_owned(), + ))), + ExpressionKind::AtCurrentWorkspace => { + let ctx = state.workspace_ctx.as_ref().ok_or_else(|| { + RevsetParseError::with_span( + RevsetParseErrorKind::WorkingCopyWithoutWorkspace, + node.span, + ) + })?; + Ok(RevsetExpression::working_copy(ctx.workspace_id.clone())) + } + ExpressionKind::DagRangeAll => Ok(RevsetExpression::all()), + ExpressionKind::RangeAll => { + Ok(RevsetExpression::root().range(&RevsetExpression::visible_heads())) + } + ExpressionKind::Unary(op, arg_node) => { + let arg = parse_expression_rule(arg_node, state)?; + match op { + UnaryOp::Negate => Ok(arg.negated()), + UnaryOp::DagRangePre => Ok(arg.ancestors()), + UnaryOp::DagRangePost => Ok(arg.descendants()), + UnaryOp::RangePre => Ok(RevsetExpression::root().range(&arg)), + UnaryOp::RangePost => Ok(arg.range(&RevsetExpression::visible_heads())), + UnaryOp::Parents => Ok(arg.parents()), + UnaryOp::Children => Ok(arg.children()), + } + } + ExpressionKind::Binary(op, lhs_node, rhs_node) => { + let lhs = parse_expression_rule(lhs_node, state)?; + let rhs = parse_expression_rule(rhs_node, state)?; + match op { + BinaryOp::Union => Ok(lhs.union(&rhs)), + BinaryOp::Intersection => Ok(lhs.intersection(&rhs)), + BinaryOp::Difference => Ok(lhs.minus(&rhs)), + BinaryOp::DagRange => Ok(lhs.dag_range_to(&rhs)), + BinaryOp::Range => Ok(lhs.range(&rhs)), + } + } + ExpressionKind::FunctionCall(function) => lower_function_call(function, state), + ExpressionKind::AliasExpanded(id, subst) => parse_expression_rule(subst, state) + .map_err(|e| e.within_alias_expansion(*id, node.span)), } } @@ -907,18 +921,23 @@ pub fn parse( revset_str: &str, context: &RevsetParseContext, ) -> Result, RevsetParseError> { - let locals = HashMap::new(); - let state = ParseState::new(context, &locals); - parse_program(revset_str, state) + let node = revset_parser::parse_program(revset_str)?; + let node = dsl_util::expand_aliases(node, context.aliases_map)?; + let state = ParseState::new(context); + parse_expression_rule(&node, state) + .map_err(|err| err.extend_function_candidates(context.aliases_map.function_names())) } pub fn parse_with_modifier( revset_str: &str, context: &RevsetParseContext, ) -> Result<(Rc, Option), RevsetParseError> { - let locals = HashMap::new(); - let state = ParseState::new(context, &locals); - parse_program_with_modifier(revset_str, state) + let (node, modifier) = revset_parser::parse_program_with_modifier(revset_str)?; + let node = dsl_util::expand_aliases(node, context.aliases_map)?; + let state = ParseState::new(context); + let expression = parse_expression_rule(&node, state) + .map_err(|err| err.extend_function_candidates(context.aliases_map.function_names()))?; + Ok((expression, modifier)) } /// `Some` for rewritten expression, or `None` to reuse the original expression. @@ -3115,7 +3134,7 @@ mod tests { parse_with_aliases("F(x=y)", [("F(x)", "x")]), Err(RevsetParseErrorKind::InvalidFunctionArguments { name: "F".to_owned(), - message: r#"Unexpected keyword argument "x""#.to_owned() + message: "Unexpected keyword arguments".to_owned() }) ); diff --git a/lib/src/revset_parser.rs b/lib/src/revset_parser.rs index 4caa85101..2a64dab44 100644 --- a/lib/src/revset_parser.rs +++ b/lib/src/revset_parser.rs @@ -14,8 +14,7 @@ #![allow(missing_docs)] -use std::collections::{HashMap, HashSet}; -use std::rc::Rc; +use std::collections::HashSet; use std::{error, mem}; use itertools::Itertools as _; @@ -27,13 +26,12 @@ use pest_derive::Parser; use thiserror::Error; use crate::dsl_util::{ - self, collect_similar, AliasDeclaration, AliasDeclarationParser, AliasExpandError, - AliasExpandableExpression, AliasId, AliasesMap, ExpressionFolder, FoldableExpression, - InvalidArguments, StringLiteralParser, + self, collect_similar, AliasDeclaration, AliasDeclarationParser, AliasDefinitionParser, + AliasExpandError, AliasExpandableExpression, AliasId, AliasesMap, ExpressionFolder, + FoldableExpression, InvalidArguments, KeywordArgument, StringLiteralParser, }; -use crate::op_store::WorkspaceId; // TODO: remove reverse dependency on revset module -use crate::revset::{ParseState, RevsetExpression, RevsetModifier}; +use crate::revset::RevsetModifier; #[derive(Parser)] #[grammar = "revset.pest"] @@ -208,7 +206,6 @@ impl RevsetParseError { /// If this is a `NoSuchFunction` error, expands the candidates list with /// the given `other_functions`. - #[allow(unused)] // TODO: remove pub(super) fn extend_function_candidates(mut self, other_functions: I) -> Self where I: IntoIterator, @@ -407,25 +404,21 @@ pub enum BinaryOp { pub type ExpressionNode<'i> = dsl_util::ExpressionNode<'i, ExpressionKind<'i>>; pub type FunctionCallNode<'i> = dsl_util::FunctionCallNode<'i, ExpressionKind<'i>>; -pub(super) fn parse_program( - revset_str: &str, - state: ParseState, -) -> Result, RevsetParseError> { +pub(super) fn parse_program(revset_str: &str) -> Result { let mut pairs = RevsetParser::parse(Rule::program, revset_str)?; let first = pairs.next().unwrap(); - parse_expression_rule(first.into_inner(), state) + parse_expression_node(first.into_inner()) } pub(super) fn parse_program_with_modifier( revset_str: &str, - state: ParseState, -) -> Result<(Rc, Option), RevsetParseError> { +) -> Result<(ExpressionNode, Option), RevsetParseError> { let mut pairs = RevsetParser::parse(Rule::program_with_modifier, revset_str)?; let first = pairs.next().unwrap(); match first.as_rule() { Rule::expression => { - let expression = parse_expression_rule(first.into_inner(), state)?; - Ok((expression, None)) + let node = parse_expression_node(first.into_inner())?; + Ok((node, None)) } Rule::program_modifier => { let (lhs, op) = first.into_inner().collect_tuple().unwrap(); @@ -442,17 +435,14 @@ pub(super) fn parse_program_with_modifier( )); } }; - let expression = parse_expression_rule(rhs.into_inner(), state)?; - Ok((expression, Some(modififer))) + let node = parse_expression_node(rhs.into_inner())?; + Ok((node, Some(modififer))) } r => panic!("unexpected revset parse rule: {r:?}"), } } -pub fn parse_expression_rule( - pairs: Pairs, - state: ParseState, -) -> Result, RevsetParseError> { +fn parse_expression_node(pairs: Pairs) -> Result { fn not_prefix_op( op: &Pair, similar_op: impl Into, @@ -522,141 +512,122 @@ pub fn parse_expression_rule( | Op::postfix(Rule::compat_parents_op)) }); PRATT - .map_primary(|primary| match primary.as_rule() { - Rule::primary => parse_primary_rule(primary, state), - Rule::dag_range_all_op => Ok(RevsetExpression::all()), - Rule::range_all_op => { - Ok(RevsetExpression::root().range(&RevsetExpression::visible_heads())) - } - r => panic!("unexpected primary rule {r:?}"), + .map_primary(|primary| { + let expr = match primary.as_rule() { + Rule::primary => return parse_primary_node(primary), + Rule::dag_range_all_op => ExpressionKind::DagRangeAll, + Rule::range_all_op => ExpressionKind::RangeAll, + r => panic!("unexpected primary rule {r:?}"), + }; + Ok(ExpressionNode::new(expr, primary.as_span())) }) - .map_prefix(|op, rhs| match op.as_rule() { - Rule::negate_op => Ok(rhs?.negated()), - Rule::dag_range_pre_op => Ok(rhs?.ancestors()), - Rule::compat_dag_range_pre_op => Err(not_prefix_op(&op, "::", "ancestors")), - Rule::range_pre_op => Ok(RevsetExpression::root().range(&rhs?)), - r => panic!("unexpected prefix operator rule {r:?}"), + .map_prefix(|op, rhs| { + let op_kind = match op.as_rule() { + Rule::negate_op => UnaryOp::Negate, + Rule::dag_range_pre_op => UnaryOp::DagRangePre, + Rule::compat_dag_range_pre_op => Err(not_prefix_op(&op, "::", "ancestors"))?, + Rule::range_pre_op => UnaryOp::RangePre, + r => panic!("unexpected prefix operator rule {r:?}"), + }; + let rhs = Box::new(rhs?); + let span = op.as_span().start_pos().span(&rhs.span.end_pos()); + let expr = ExpressionKind::Unary(op_kind, rhs); + Ok(ExpressionNode::new(expr, span)) }) - .map_postfix(|lhs, op| match op.as_rule() { - Rule::dag_range_post_op => Ok(lhs?.descendants()), - Rule::compat_dag_range_post_op => Err(not_postfix_op(&op, "::", "descendants")), - Rule::range_post_op => Ok(lhs?.range(&RevsetExpression::visible_heads())), - Rule::parents_op => Ok(lhs?.parents()), - Rule::children_op => Ok(lhs?.children()), - Rule::compat_parents_op => Err(not_postfix_op(&op, "-", "parents")), - r => panic!("unexpected postfix operator rule {r:?}"), + .map_postfix(|lhs, op| { + let op_kind = match op.as_rule() { + Rule::dag_range_post_op => UnaryOp::DagRangePost, + Rule::compat_dag_range_post_op => Err(not_postfix_op(&op, "::", "descendants"))?, + Rule::range_post_op => UnaryOp::RangePost, + Rule::parents_op => UnaryOp::Parents, + Rule::children_op => UnaryOp::Children, + Rule::compat_parents_op => Err(not_postfix_op(&op, "-", "parents"))?, + r => panic!("unexpected postfix operator rule {r:?}"), + }; + let lhs = Box::new(lhs?); + let span = lhs.span.start_pos().span(&op.as_span().end_pos()); + let expr = ExpressionKind::Unary(op_kind, lhs); + Ok(ExpressionNode::new(expr, span)) }) - .map_infix(|lhs, op, rhs| match op.as_rule() { - Rule::union_op => Ok(lhs?.union(&rhs?)), - Rule::compat_add_op => Err(not_infix_op(&op, "|", "union")), - Rule::intersection_op => Ok(lhs?.intersection(&rhs?)), - Rule::difference_op => Ok(lhs?.minus(&rhs?)), - Rule::compat_sub_op => Err(not_infix_op(&op, "~", "difference")), - Rule::dag_range_op => Ok(lhs?.dag_range_to(&rhs?)), - Rule::compat_dag_range_op => Err(not_infix_op(&op, "::", "DAG range")), - Rule::range_op => Ok(lhs?.range(&rhs?)), - r => panic!("unexpected infix operator rule {r:?}"), + .map_infix(|lhs, op, rhs| { + let op_kind = match op.as_rule() { + Rule::union_op => BinaryOp::Union, + Rule::compat_add_op => Err(not_infix_op(&op, "|", "union"))?, + Rule::intersection_op => BinaryOp::Intersection, + Rule::difference_op => BinaryOp::Difference, + Rule::compat_sub_op => Err(not_infix_op(&op, "~", "difference"))?, + Rule::dag_range_op => BinaryOp::DagRange, + Rule::compat_dag_range_op => Err(not_infix_op(&op, "::", "DAG range"))?, + Rule::range_op => BinaryOp::Range, + r => panic!("unexpected infix operator rule {r:?}"), + }; + let lhs = Box::new(lhs?); + let rhs = Box::new(rhs?); + let span = lhs.span.start_pos().span(&rhs.span.end_pos()); + let expr = ExpressionKind::Binary(op_kind, lhs, rhs); + Ok(ExpressionNode::new(expr, span)) }) .parse(pairs) } -fn parse_primary_rule( - pair: Pair, - state: ParseState, -) -> Result, RevsetParseError> { +fn parse_primary_node(pair: Pair) -> Result { let span = pair.as_span(); let mut pairs = pair.into_inner(); let first = pairs.next().unwrap(); - match first.as_rule() { - Rule::expression => parse_expression_rule(first.into_inner(), state), + let expr = match first.as_rule() { + Rule::expression => return parse_expression_node(first.into_inner()), Rule::function_name => { let arguments_pair = pairs.next().unwrap(); - parse_function_expression(first, arguments_pair, state, span) + let function = Box::new(parse_function_call_node(first, arguments_pair)?); + ExpressionKind::FunctionCall(function) } - Rule::string_pattern => parse_string_pattern_rule(first, state), + Rule::string_pattern => parse_string_pattern(first), // Symbol without "@" may be substituted by aliases. Primary expression including "@" // is considered an indecomposable unit, and no alias substitution would be made. - Rule::symbol if pairs.peek().is_none() => parse_symbol_rule(first.into_inner(), state), + Rule::symbol if pairs.peek().is_none() => parse_symbol(first.into_inner()), Rule::symbol => { - let name = parse_symbol_rule_as_literal(first.into_inner()); + let name = parse_as_string_literal(first.into_inner()); assert_eq!(pairs.next().unwrap().as_rule(), Rule::at_op); if let Some(second) = pairs.next() { // infix "@" assert_eq!(second.as_rule(), Rule::symbol); - let remote = parse_symbol_rule_as_literal(second.into_inner()); - Ok(RevsetExpression::remote_symbol(name, remote)) + let remote = parse_as_string_literal(second.into_inner()); + ExpressionKind::RemoteSymbol { name, remote } } else { // postfix "@" - Ok(RevsetExpression::working_copy(WorkspaceId::new(name))) + ExpressionKind::AtWorkspace(name) } } - Rule::at_op => { - // nullary "@" - let ctx = state.workspace_ctx.as_ref().ok_or_else(|| { - RevsetParseError::with_span(RevsetParseErrorKind::WorkingCopyWithoutWorkspace, span) - })?; - Ok(RevsetExpression::working_copy(ctx.workspace_id.clone())) - } - _ => { - panic!("unexpected revset parse rule: {:?}", first.as_str()); - } - } + // nullary "@" + Rule::at_op => ExpressionKind::AtCurrentWorkspace, + r => panic!("unexpected revset parse rule: {r:?}"), + }; + Ok(ExpressionNode::new(expr, span)) } -fn parse_string_pattern_rule( - pair: Pair, - state: ParseState, -) -> Result, RevsetParseError> { +// TODO: inline +fn parse_string_pattern(pair: Pair) -> ExpressionKind { assert_eq!(pair.as_rule(), Rule::string_pattern); let (lhs, op, rhs) = pair.into_inner().collect_tuple().unwrap(); assert_eq!(lhs.as_rule(), Rule::identifier); assert_eq!(op.as_rule(), Rule::pattern_kind_op); assert_eq!(rhs.as_rule(), Rule::symbol); - if state.allow_string_pattern { - let kind = lhs.as_str().to_owned(); - let value = parse_symbol_rule_as_literal(rhs.into_inner()); - Ok(Rc::new(RevsetExpression::StringPattern { kind, value })) - } else { - Err(RevsetParseError::with_span( - RevsetParseErrorKind::NotInfixOperator { - op: op.as_str().to_owned(), - similar_op: "::".to_owned(), - description: "DAG range".to_owned(), - }, - op.as_span(), - )) - } + let kind = lhs.as_str(); + let value = parse_as_string_literal(rhs.into_inner()); + ExpressionKind::StringPattern { kind, value } } -/// Parses symbol to expression, expands aliases as needed. -fn parse_symbol_rule( - pairs: Pairs, - state: ParseState, -) -> Result, RevsetParseError> { +// TODO: inline +fn parse_symbol(pairs: Pairs) -> ExpressionKind { let first = pairs.peek().unwrap(); match first.as_rule() { - Rule::identifier => { - let name = first.as_str(); - if let Some(expr) = state.locals.get(name) { - Ok(expr.clone()) - } else if let Some((id, defn)) = state.aliases_map.get_symbol(name) { - let locals = HashMap::new(); // Don't spill out the current scope - state.with_alias_expanding(id, &locals, first.as_span(), |state| { - parse_program(defn, state) - }) - } else { - Ok(RevsetExpression::symbol(name.to_owned())) - } - } - _ => { - let text = parse_symbol_rule_as_literal(pairs); - Ok(RevsetExpression::symbol(text)) - } + Rule::identifier => ExpressionKind::Identifier(first.as_str()), + _ => ExpressionKind::String(parse_as_string_literal(pairs)), } } -/// Parses part of compound symbol to string without alias substitution. -fn parse_symbol_rule_as_literal(mut pairs: Pairs) -> String { +/// Parses part of compound symbol to string. +fn parse_as_string_literal(mut pairs: Pairs) -> String { let first = pairs.next().unwrap(); match first.as_rule() { Rule::identifier => first.as_str().to_owned(), @@ -672,44 +643,21 @@ fn parse_symbol_rule_as_literal(mut pairs: Pairs) -> String { } } -fn parse_function_expression( - name_pair: Pair, - arguments_pair: Pair, - state: ParseState, - primary_span: pest::Span<'_>, -) -> Result, RevsetParseError> { +fn parse_function_call_node<'i>( + name_pair: Pair<'i, Rule>, + args_pair: Pair<'i, Rule>, +) -> Result, RevsetParseError> { + let name_span = name_pair.as_span(); + let args_span = args_pair.as_span(); let name = name_pair.as_str(); - if let Some((id, params, defn)) = state.aliases_map.get_function(name) { - // Resolve arguments in the current scope, and pass them in to the alias - // expansion scope. - let (required, optional) = - expect_named_arguments_vec(name, &[], arguments_pair, params.len(), params.len())?; - assert!(optional.is_empty()); - let args: Vec<_> = required - .into_iter() - .map(|arg| parse_expression_rule(arg.into_inner(), state)) - .try_collect()?; - let locals = params.iter().map(|s| s.as_str()).zip(args).collect(); - state.with_alias_expanding(id, &locals, primary_span, |state| { - parse_program(defn, state) - }) - } else if let Some(func) = state.function_map.get(name) { - func(name, arguments_pair, state) - } else { - Err(RevsetParseError::with_span( - RevsetParseErrorKind::NoSuchFunction { - name: name.to_owned(), - candidates: collect_similar( - name, - itertools::chain( - state.function_map.keys().copied(), - state.aliases_map.function_names(), - ), - ), - }, - name_pair.as_span(), - )) - } + let (args, keyword_args) = parse_function_call_args(name, args_pair)?; + Ok(FunctionCallNode { + name, + name_span, + args, + keyword_args, + args_span, + }) } pub type RevsetAliasesMap = AliasesMap; @@ -750,125 +698,62 @@ impl AliasDeclarationParser for RevsetAliasParser { } } -type OptionalArg<'i> = Option>; +impl AliasDefinitionParser for RevsetAliasParser { + type Output<'i> = ExpressionKind<'i>; + type Error = RevsetParseError; -pub fn expect_no_arguments( - function_name: &str, - arguments_pair: Pair, -) -> Result<(), RevsetParseError> { - let ([], []) = expect_arguments(function_name, arguments_pair)?; - Ok(()) + fn parse_definition<'i>(&self, source: &'i str) -> Result, Self::Error> { + parse_program(source) + } } -pub fn expect_exact_arguments<'i, const N: usize>( +// TODO: inline +fn parse_function_call_args<'i>( function_name: &str, arguments_pair: Pair<'i, Rule>, -) -> Result<[Pair<'i, Rule>; N], RevsetParseError> { - let (args, []) = expect_arguments(function_name, arguments_pair)?; - Ok(args) -} - -pub fn expect_arguments<'i, const N: usize, const M: usize>( - function_name: &str, - arguments_pair: Pair<'i, Rule>, -) -> Result<([Pair<'i, Rule>; N], [OptionalArg<'i>; M]), RevsetParseError> { - expect_named_arguments(function_name, &[], arguments_pair) -} - -/// Extracts N required arguments and M optional arguments. -/// -/// `argument_names` is a list of argument names. Unnamed positional arguments -/// should be padded with `""`. -pub fn expect_named_arguments<'i, const N: usize, const M: usize>( - function_name: &str, - argument_names: &[&str], - arguments_pair: Pair<'i, Rule>, -) -> Result<([Pair<'i, Rule>; N], [OptionalArg<'i>; M]), RevsetParseError> { - let (required, optional) = - expect_named_arguments_vec(function_name, argument_names, arguments_pair, N, N + M)?; - Ok((required.try_into().unwrap(), optional.try_into().unwrap())) -} - -pub fn expect_named_arguments_vec<'i>( - function_name: &str, - argument_names: &[&str], - arguments_pair: Pair<'i, Rule>, - min_arg_count: usize, - max_arg_count: usize, -) -> Result<(Vec>, Vec>), RevsetParseError> { - assert!(argument_names.len() <= max_arg_count); - let arguments_span = arguments_pair.as_span(); - let make_count_error = || { - let message = if min_arg_count == max_arg_count { - format!("Expected {min_arg_count} arguments") - } else { - format!("Expected {min_arg_count} to {max_arg_count} arguments") - }; - RevsetParseError::invalid_arguments(function_name, message, arguments_span) - }; - - let mut pos_iter = Some(0..max_arg_count); - let mut extracted_pairs = vec![None; max_arg_count]; +) -> Result< + ( + Vec>, + Vec>>, + ), + RevsetParseError, +> { + let mut args = Vec::new(); + let mut keyword_args = Vec::new(); for pair in arguments_pair.into_inner() { let span = pair.as_span(); match pair.as_rule() { Rule::expression => { - let pos = pos_iter - .as_mut() - .ok_or_else(|| { - RevsetParseError::invalid_arguments( - function_name, - "Positional argument follows keyword argument", - span, - ) - })? - .next() - .ok_or_else(make_count_error)?; - assert!(extracted_pairs[pos].is_none()); - extracted_pairs[pos] = Some(pair); + if !keyword_args.is_empty() { + return Err(RevsetParseError::invalid_arguments( + function_name, + "Positional argument follows keyword argument", + span, + )); + } + args.push(parse_expression_node(pair.into_inner())?); } Rule::keyword_argument => { - pos_iter = None; // No more positional arguments let mut pairs = pair.into_inner(); let name = pairs.next().unwrap(); let expr = pairs.next().unwrap(); assert_eq!(name.as_rule(), Rule::identifier); assert_eq!(expr.as_rule(), Rule::expression); - let pos = argument_names - .iter() - .position(|&n| n == name.as_str()) - .ok_or_else(|| { - RevsetParseError::invalid_arguments( - function_name, - format!(r#"Unexpected keyword argument "{}""#, name.as_str()), - span, - ) - })?; - if extracted_pairs[pos].is_some() { - return Err(RevsetParseError::invalid_arguments( - function_name, - format!(r#"Got multiple values for keyword "{}""#, name.as_str()), - span, - )); - } - extracted_pairs[pos] = Some(expr); + let arg = KeywordArgument { + name: name.as_str(), + name_span: name.as_span(), + value: parse_expression_node(expr.into_inner())?, + }; + keyword_args.push(arg); } r => panic!("unexpected argument rule {r:?}"), } } - - assert_eq!(extracted_pairs.len(), max_arg_count); - let optional = extracted_pairs.split_off(min_arg_count); - let required = extracted_pairs.into_iter().flatten().collect_vec(); - if required.len() != min_arg_count { - return Err(make_count_error()); - } - Ok((required, optional)) + Ok((args, keyword_args)) } /// Applies the give function to the innermost `node` by unwrapping alias /// expansion nodes. -#[allow(unused)] // TODO: remove pub(super) fn expect_literal_with( node: &ExpressionNode, f: impl FnOnce(&ExpressionNode) -> Result,