ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

revset: implement function alias expansion

Browse source 
Function parameters are processed as local symbols while substituting
alias expression. This isn't as efficient as Mercurial which caches
a tree of fully-expanded function template, but that wouldn't matter in
practice.
This commit is contained in:
Yuya Nishihara 2022-11-26 20:41:55 +09:00
parent d8feed9be4
commit 70292f79b7
5 changed files with 247 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGELOG.md
View file

@ -45,8 +45,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* The new revset function `empty()` finds commits modifying no files.
* Added support for revset aliases. New symbols can be configured by
`revset-aliases.<name> = <expression>`.
* Added support for revset aliases. New symbols and functions can be configured
by `revset-aliases.<name> = <expression>`.
* It is now possible to specify configuration options on the command line
with the new `--config-toml` global option.

5
docs/revsets.md
View file

@ -116,13 +116,14 @@ revsets (expressions) as arguments.
## Aliases
New symbols can be defined in the config file, by using any combination
of the predefined symbols/functions and other aliases.
New symbols and functions can be defined in the config file, by using any
combination of the predefined symbols/functions and other aliases.
For example:
```toml
[revset-aliases]
'mine' = 'author(martinvonz)'
'user(x)' = 'author(x) | committer(x)'
```

12
lib/src/revset.pest
View file

@ -47,6 +47,10 @@ function_arguments = {
(whitespace* ~ expression ~ whitespace* ~ ",")* ~ whitespace* ~ expression ~ whitespace*
| whitespace*
}
formal_parameters = {
(whitespace* ~ identifier ~ whitespace* ~ ",")* ~ whitespace* ~ identifier ~ whitespace*
| whitespace*
}
primary = {
function_name ~ "(" ~ function_arguments ~ ")"
@ -69,6 +73,10 @@ expression = {
program = _{ SOI ~ expression ~ EOI }
alias_declaration = _{
SOI ~ identifier ~ EOI
alias_declaration_part = _{
function_name ~ "(" ~ formal_parameters ~ ")"
| identifier
}
alias_declaration = _{
SOI ~ alias_declaration_part ~ EOI
}

187
lib/src/revset.rs
View file

@ -223,6 +223,8 @@ pub enum RevsetParseErrorKind {
FsPathParseError(#[source] FsPathParseError),
#[error("Cannot resolve file pattern without workspace")]
FsPathWithoutWorkspace,
#[error("Redefinition of function parameter")]
RedefinedFunctionParameter,
#[error(r#"Alias "{0}" cannot be expanded"#)]
BadAliasExpansion(String),
#[error(r#"Alias "{0}" expanded recursively"#)]
@ -521,6 +523,7 @@ impl RevsetExpression {
#[derive(Clone, Debug, Default)]
pub struct RevsetAliasesMap {
symbol_aliases: HashMap<String, String>,
function_aliases: HashMap<String, (Vec<String>, String)>,
}
impl RevsetAliasesMap {
@ -541,6 +544,9 @@ impl RevsetAliasesMap {
RevsetAliasDeclaration::Symbol(name) => {
self.symbol_aliases.insert(name, defn.into());
}
RevsetAliasDeclaration::Function(name, params) => {
self.function_aliases.insert(name, (params, defn.into()));
}
}
Ok(())
}
@ -550,13 +556,28 @@ impl RevsetAliasesMap {
.get_key_value(name)
.map(|(name, defn)| (RevsetAliasId::Symbol(name), defn.as_ref()))
}
fn get_function<'a>(
&'a self,
name: &str,
) -> Option<(RevsetAliasId<'a>, &'a [String], &'a str)> {
self.function_aliases
.get_key_value(name)
.map(|(name, (params, defn))| {
(
RevsetAliasId::Function(name),
params.as_ref(),
defn.as_ref(),
)
})
}
}
/// Parsed declaration part of alias rule.
#[derive(Clone, Debug)]
enum RevsetAliasDeclaration {
Symbol(String),
// TODO: Function(String, Vec<String>)
Function(String, Vec<String>),
}
impl RevsetAliasDeclaration {
@ -565,6 +586,26 @@ impl RevsetAliasDeclaration {
let first = pairs.next().unwrap();
match first.as_rule() {
Rule::identifier => Ok(RevsetAliasDeclaration::Symbol(first.as_str().to_owned())),
Rule::function_name => {
let name = first.as_str().to_owned();
let params_pair = pairs.next().unwrap();
let params_span = params_pair.as_span();
let params = params_pair
.into_inner()
.map(|pair| match pair.as_rule() {
Rule::identifier => pair.as_str().to_owned(),
r => panic!("unxpected formal parameter rule {r:?}"),
})
.collect_vec();
if params.iter().all_unique() {
Ok(RevsetAliasDeclaration::Function(name, params))
} else {
Err(RevsetParseError::with_span(
RevsetParseErrorKind::RedefinedFunctionParameter,
params_span,
))
}
}
r => panic!("unxpected alias declaration rule {r:?}"),
}
}
@ -574,13 +615,14 @@ impl RevsetAliasDeclaration {
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum RevsetAliasId<'a> {
Symbol(&'a str),
// TODO: Function(&'a str)
Function(&'a str),
}
impl fmt::Display for RevsetAliasId<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
RevsetAliasId::Symbol(name) => write!(f, "{name}"),
RevsetAliasId::Function(name) => write!(f, "{name}()"),
}
}
}
@ -589,6 +631,7 @@ impl fmt::Display for RevsetAliasId<'_> {
struct ParseState<'a> {
aliases_map: &'a RevsetAliasesMap,
aliases_expanding: &'a [RevsetAliasId<'a>],
locals: &'a HashMap<&'a str, Rc<RevsetExpression>>,
workspace_ctx: Option<&'a RevsetWorkspaceContext<'a>>,
}
@ -596,6 +639,7 @@ impl ParseState<'_> {
fn with_alias_expanding<T>(
self,
id: RevsetAliasId<'_>,
locals: &HashMap<&str, Rc<RevsetExpression>>,
span: pest::Span<'_>,
f: impl FnOnce(ParseState<'_>) -> Result<T, RevsetParseError>,
) -> Result<T, RevsetParseError> {
@ -611,6 +655,7 @@ impl ParseState<'_> {
let expanding_state = ParseState {
aliases_map: self.aliases_map,
aliases_expanding: &aliases_expanding,
locals,
workspace_ctx: self.workspace_ctx,
};
f(expanding_state).map_err(|e| {
@ -698,9 +743,13 @@ fn parse_symbol_rule(
match first.as_rule() {
Rule::identifier => {
let name = first.as_str();
// TODO: look up locals while expanding function alias
if let Some((id, defn)) = state.aliases_map.get_symbol(name) {
state.with_alias_expanding(id, first.as_span(), |state| parse_program(defn, state))
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()))
}
@ -726,6 +775,39 @@ fn parse_function_expression(
name_pair: Pair<Rule>,
arguments_pair: Pair<Rule>,
state: ParseState,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
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 arguments_span = arguments_pair.as_span();
let args = arguments_pair
.into_inner()
.map(|arg| parse_expression_rule(arg.into_inner(), state))
.collect::<Result<Vec<_>, RevsetParseError>>()?;
if params.len() == args.len() {
let locals = params.iter().map(|s| s.as_str()).zip(args).collect();
state.with_alias_expanding(id, &locals, name_pair.as_span(), |state| {
parse_program(defn, state)
})
} else {
Err(RevsetParseError::with_span(
RevsetParseErrorKind::InvalidFunctionArguments {
name: name.to_owned(),
message: format!("Expected {} arguments", params.len()),
},
arguments_span,
))
}
} else {
parse_builtin_function(name_pair, arguments_pair, state)
}
}
fn parse_builtin_function(
name_pair: Pair<Rule>,
arguments_pair: Pair<Rule>,
state: ParseState,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let name = name_pair.as_str();
match name {
@ -958,6 +1040,7 @@ pub fn parse(
let state = ParseState {
aliases_map,
aliases_expanding: &[],
locals: &HashMap::new(),
workspace_ctx,
};
parse_program(revset_str, state)
@ -2117,6 +2200,100 @@ mod tests {
);
}
#[test]
fn test_expand_function_alias() {
assert_eq!(
parse_with_aliases("F()", [("F( )", "a")]).unwrap(),
parse("a").unwrap()
);
assert_eq!(
parse_with_aliases("F(a)", [("F( x )", "x")]).unwrap(),
parse("a").unwrap()
);
assert_eq!(
parse_with_aliases("F(a, b)", [("F( x, y )", "x|y")]).unwrap(),
parse("a|b").unwrap()
);
// Arguments should be resolved in the current scope.
assert_eq!(
parse_with_aliases("F(a:y,b:x)", [("F(x,y)", "x|y")]).unwrap(),
parse("(a:y)|(b:x)").unwrap()
);
// F(a) -> G(a)&y -> (x|a)&y
assert_eq!(
parse_with_aliases("F(a)", [("F(x)", "G(x)&y"), ("G(y)", "x|y")]).unwrap(),
parse("(x|a)&y").unwrap()
);
// F(G(a)) -> F(x|a) -> G(x|a)&y -> (x|(x|a))&y
assert_eq!(
parse_with_aliases("F(G(a))", [("F(x)", "G(x)&y"), ("G(y)", "x|y")]).unwrap(),
parse("(x|(x|a))&y").unwrap()
);
// Function parameter should precede the symbol alias.
assert_eq!(
parse_with_aliases("F(a)|X", [("F(X)", "X"), ("X", "x")]).unwrap(),
parse("a|x").unwrap()
);
// Function parameter shouldn't be expanded in symbol alias.
assert_eq!(
parse_with_aliases("F(a)", [("F(x)", "x|A"), ("A", "x")]).unwrap(),
parse("a|x").unwrap()
);
// String literal should not be substituted with function parameter.
assert_eq!(
parse_with_aliases("F(a)", [("F(x)", r#"x|"x""#)]).unwrap(),
parse("a|x").unwrap()
);
// Pass string literal as parameter.
assert_eq!(
parse_with_aliases("F(a)", [("F(x)", "author(x)|committer(x)")]).unwrap(),
parse("author(a)|committer(a)").unwrap()
);
// Function and symbol aliases reside in separate namespaces.
assert_eq!(
parse_with_aliases("A()", [("A()", "A"), ("A", "a")]).unwrap(),
parse("a").unwrap()
);
// Invalid number of arguments.
assert_eq!(
parse_with_aliases("F(a)", [("F()", "x")]),
Err(RevsetParseErrorKind::InvalidFunctionArguments {
name: "F".to_owned(),
message: "Expected 0 arguments".to_owned()
})
);
assert_eq!(
parse_with_aliases("F()", [("F(x)", "x")]),
Err(RevsetParseErrorKind::InvalidFunctionArguments {
name: "F".to_owned(),
message: "Expected 1 arguments".to_owned()
})
);
assert_eq!(
parse_with_aliases("F(a,b,c)", [("F(x,y)", "x|y")]),
Err(RevsetParseErrorKind::InvalidFunctionArguments {
name: "F".to_owned(),
message: "Expected 2 arguments".to_owned()
})
);
// Infinite recursion, where the top-level error isn't of RecursiveAlias kind.
assert_eq!(
parse_with_aliases(
"F(a)",
[("F(x)", "G(x)"), ("G(x)", "H(x)"), ("H(x)", "F(x)")]
),
Err(RevsetParseErrorKind::BadAliasExpansion("F()".to_owned()))
);
}
#[test]
fn test_optimize_subtree() {
// Check that transform_expression_bottom_up() never rewrites enum variant

52
tests/test_revset_output.rs
View file

@ -132,7 +132,10 @@ fn test_alias() {
'my-root' = 'root'
'syntax-error' = 'whatever &'
'recurse' = 'recurse1'
'recurse1' = 'recurse'
'recurse1' = 'recurse2()'
'recurse2()' = 'recurse'
'identity(x)' = 'x'
'my_author(x)' = 'author(x)'
"###,
);
@ -142,6 +145,12 @@ fn test_alias() {
(no description set)
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "identity(my-root)"]);
insta::assert_snapshot!(stdout, @r###"
o 000000000000 1970-01-01 00:00:00.000 +00:00 000000000000
(no description set)
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root & syntax-error"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:8
@ -158,6 +167,32 @@ fn test_alias() {
= expected dag_range_pre_op, range_pre_op, or primary
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "identity()"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:10
|
1 | identity()
| ^
|
= Invalid arguments to revset function "identity": Expected 1 arguments
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "my_author(none())"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:1
|
1 | my_author(none())
| ^-------^
|
= Alias "my_author()" cannot be expanded
--> 1:8
|
1 | author(x)
| ^
|
= Invalid arguments to revset function "author": Expected function argument of type string
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root & recurse"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:8
@ -174,6 +209,12 @@ fn test_alias() {
= Alias "recurse1" cannot be expanded
--> 1:1
|
1 | recurse2()
| ^------^
|
= Alias "recurse2()" cannot be expanded
--> 1:1
|
1 | recurse
| ^-----^
|
@ -192,6 +233,7 @@ fn test_bad_alias_decl() {
[revset-aliases]
'my-root' = 'root'
'"bad"' = 'root'
'badfn(a, a)' = 'root'
"###,
);
@ -210,6 +252,12 @@ fn test_bad_alias_decl() {
1 | "bad"
| ^---
|
= expected identifier
= expected identifier or function_name
Failed to load "revset-aliases.badfn(a, a)": --> 1:7
|
1 | badfn(a, a)
| ^--^
|
= Redefinition of function parameter
"###);
}