From 88904e2b63d36aff3c594a33901eec6df8c51f57 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Thu, 15 Apr 2021 17:54:48 -0700 Subject: [PATCH] revsets: add support for function syntax This adds `parents(foo)` and `ancestors(foo)` as alternative ways of writing `:foo` and `*:foo`. I haven't added support for for whitespace yet; the parsing is very strict. The error messages will also need to be improved later. --- lib/src/revset.pest | 19 +++++++++ lib/src/revset.rs | 83 ++++++++++++++++++++++++++++++++++++---- lib/tests/test_revset.rs | 38 +++++++++++++++++- 3 files changed, 131 insertions(+), 9 deletions(-) diff --git a/lib/src/revset.pest b/lib/src/revset.pest index 0c6d207ff..677c1e59c 100644 --- a/lib/src/revset.pest +++ b/lib/src/revset.pest @@ -13,12 +13,31 @@ // limitations under the License. symbol = @{ (ASCII_ALPHANUMERIC | "@" | "/" | ".")+ } +literal_string = { "\"" ~ (!"\"" ~ ANY)+ ~ "\"" } parents = { ":" } ancestors = { "*:" } +function_name = @{ ASCII_ALPHANUMERIC+ } +// The grammar accepts a string literal or an expression for function +// arguments. We then decide when walking the parse tree if we +// should interpret the string value of the literal string or the expression +// as a string or an expression, or maybe as an integer. For example, +// parents(foo) might interpret "foo" as an expression, while limit(*:foo,5) +// might interpret its second argument as an integer. Also, parents("foo") +// might be disallowed, just as description(heads()) might be. +function_argument = { + literal_string + | expression +} +function_arguments = { + (function_argument ~ ",")* ~ function_argument + | "" +} + expression = { parents ~ expression | ancestors ~ expression + | function_name ~ "(" ~ function_arguments ~ ")" | symbol } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 1d3fab308..a60c35965 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -87,7 +87,11 @@ pub struct RevsetParser; #[derive(Debug, Error, PartialEq, Eq)] pub enum RevsetParseError { #[error("{0}")] - RevsetParseError(String), + SyntaxError(String), + #[error("Revset function \"{0}\" doesn't exist")] + NoSuchFunction(String), + #[error("Invalid arguments to revset function \"{name}\": {message}")] + InvalidFunctionArguments { name: String, message: String }, } #[derive(Debug, PartialEq, Eq)] @@ -100,9 +104,7 @@ pub enum RevsetExpression { fn parse_expression_rule(mut pairs: Pairs) -> Result { let first = pairs.next().unwrap(); match first.as_rule() { - Rule::symbol => { - Ok(RevsetExpression::Symbol(first.as_str().to_owned())) - } + Rule::symbol => Ok(RevsetExpression::Symbol(first.as_str().to_owned())), Rule::parents => { let expression = pairs.next().unwrap(); Ok(RevsetExpression::Parents(Box::new(parse_expression_rule( @@ -115,9 +117,74 @@ fn parse_expression_rule(mut pairs: Pairs) -> Result { - panic!("unexpected revset parse rule: {:?}", first); + Rule::function_name => { + let name = first.as_str().to_owned(); + let argument_pairs = pairs.next().unwrap().into_inner(); + parse_function_expression(name, argument_pairs) } + _ => { + panic!("unxpected revset parse rule: {:?}", first.as_str()); + } + } +} + +fn parse_function_expression( + name: String, + mut argument_pairs: Pairs, +) -> Result { + let arg_count = argument_pairs.clone().count(); + match name.as_str() { + "parents" => { + if arg_count == 1 { + Ok(RevsetExpression::Parents(Box::new( + parse_function_argument_to_expression( + &name, + argument_pairs.next().unwrap().into_inner(), + )?, + ))) + } else { + Err(RevsetParseError::InvalidFunctionArguments { + name, + message: "Expected 1 argument".to_string(), + }) + } + } + "ancestors" => { + if arg_count == 1 { + Ok(RevsetExpression::Ancestors(Box::new( + parse_function_argument_to_expression( + &name, + argument_pairs.next().unwrap().into_inner(), + )?, + ))) + } else { + Err(RevsetParseError::InvalidFunctionArguments { + name, + message: "Expected 1 argument".to_string(), + }) + } + } + _ => Err(RevsetParseError::NoSuchFunction(name)), + } +} + +fn parse_function_argument_to_expression( + name: &str, + mut pairs: Pairs, +) -> Result { + // Make a clone of the pairs for error messages + let pairs_clone = pairs.clone(); + let first = pairs.next().unwrap(); + assert!(pairs.next().is_none()); + match first.as_rule() { + Rule::expression => Ok(parse_expression_rule(first.into_inner())?), + _ => Err(RevsetParseError::InvalidFunctionArguments { + name: name.to_string(), + message: format!( + "Expected function argument of type expression, found: {}", + pairs_clone.as_str() + ), + }), } } @@ -126,8 +193,8 @@ pub fn parse(revset_str: &str) -> Result { let first = pairs.next().unwrap(); assert!(pairs.next().is_none()); if first.as_span().end() != revset_str.len() { - return Err(RevsetParseError::RevsetParseError(format!( - "Failed to parse revset {} past position {}", + return Err(RevsetParseError::SyntaxError(format!( + "Failed to parse revset \"{}\" past position {}", revset_str, first.as_span().end() ))); diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index aaf44e1ef..e4d3c162a 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -15,7 +15,7 @@ use jujube_lib::commit_builder::CommitBuilder; use jujube_lib::repo::RepoRef; use jujube_lib::revset::{ - evaluate_expression, parse, resolve_symbol, RevsetError, RevsetExpression, + evaluate_expression, parse, resolve_symbol, RevsetError, RevsetExpression, RevsetParseError, }; use jujube_lib::store::{CommitId, MillisSinceEpoch, Signature, Timestamp}; use jujube_lib::testutils; @@ -247,6 +247,42 @@ fn test_parse_revset() { ); } +#[test] +fn test_parse_revset_function() { + assert_eq!( + parse("parents(@)"), + Ok(RevsetExpression::Parents(Box::new( + RevsetExpression::Symbol("@".to_string()) + ))) + ); + assert_eq!( + parse("parents(\"@\")"), + Err(RevsetParseError::InvalidFunctionArguments { + name: "parents".to_string(), + message: "Expected function argument of type expression, found: \"@\"".to_string() + }) + ); + assert_eq!( + parse("ancestors(parents(@))"), + Ok(RevsetExpression::Ancestors(Box::new( + RevsetExpression::Parents(Box::new(RevsetExpression::Symbol("@".to_string()))) + ))) + ); + assert_eq!( + parse("parents(@"), + Err(RevsetParseError::SyntaxError( + "Failed to parse revset \"parents(@\" past position 7".to_string() + )) + ); + assert_eq!( + parse("parents(@,@)"), + Err(RevsetParseError::InvalidFunctionArguments { + name: "parents".to_string(), + message: "Expected 1 argument".to_string() + }) + ); +} + fn resolve_commit_ids(repo: RepoRef, revset_str: &str) -> Vec { let expression = parse(revset_str).unwrap(); evaluate_expression(repo, &expression)