mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-01-19 19:08:08 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

revset: represent program modifier as AST node

Browse source 
The goal is to remove special case from parsing functions and provide slightly
better error message. I don't know if we'd want to use "all:" in aliases, but
there are no strong reasons to disable it.
This commit is contained in:
Yuya Nishihara 2024-06-03 16:26:52 +09:00
parent 3c1f6d5b5d
commit eda7069aee
4 changed files with 88 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGELOG.md
View file

@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Breaking changes
* In revset aliases, top-level `kind:pattern` expression is now parsed as
modifier. Surround with parentheses if it should be parsed as string/file
pattern.
### Deprecations
### New features

21
cli/tests/test_revset_output.rs
View file

@ -549,29 +549,26 @@ fn test_all_modifier() {
= Modifier "ale" doesn't exist
"###);
// Modifier shouldn't be allowed in aliases (This restriction might be
// lifted later. "all:" in alias could be allowed if it is expanded to the
// top-level expression.)
// Modifier shouldn't be allowed in sub expression
let stderr = test_env.jj_cmd_failure(
&repo_path,
&["new", "x", "--config-toml=revset-aliases.x='all:@'"],
&["new", "x..", "--config-toml=revset-aliases.x='all:@'"],
);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: Alias "x" cannot be expanded
Caused by:
1: --> 1:1
|
1 | x
1 | x..
| ^
|
= Alias "x" cannot be expanded
2: --> 1:4
2: --> 1:1
|
1 | all:@
| ^
| ^-^
|
= ':' is not an infix operator
Hint: Did you mean '::' for DAG range?
= Modifier "all:" is not allowed in sub expression
"###);
// immutable_heads() alias may be parsed as a top-level expression, but
@ -586,12 +583,12 @@ fn test_all_modifier() {
);
insta::assert_snapshot!(stderr, @r###"
Config error: Invalid `revset-aliases.immutable_heads()`
Caused by: --> 1:4
Caused by: --> 1:1
|
1 | all:@
| ^
| ^-^
|
= ':' is not an infix operator
= Modifier "all:" is not allowed in sub expression
For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md.
"###);
}

62
lib/src/revset.rs
View file

@ -830,6 +830,13 @@ pub fn lower_expression(
}
}
ExpressionKind::FunctionCall(function) => lower_function_call(function, context),
ExpressionKind::Modifier(modifier) => {
let name = modifier.name;
Err(RevsetParseError::expression(
format!(r#"Modifier "{name}:" is not allowed in sub expression"#),
modifier.name_span,
))
}
ExpressionKind::AliasExpanded(id, subst) => {
lower_expression(subst, context).map_err(|e| e.within_alias_expansion(*id, node.span))
}
@ -850,20 +857,20 @@ pub fn parse_with_modifier(
revset_str: &str,
context: &RevsetParseContext,
) -> Result<(Rc<RevsetExpression>, Option<RevsetModifier>), RevsetParseError> {
let (node, modifier) = revset_parser::parse_program_with_modifier(revset_str)?;
let modifier = modifier
.map(|n| match n.name {
"all" => Ok(RevsetModifier::All),
name => Err(RevsetParseError::with_span(
RevsetParseErrorKind::NoSuchModifier(name.to_owned()),
n.name_span,
)),
})
.transpose()?;
let node = revset_parser::parse_program(revset_str)?;
let node = dsl_util::expand_aliases(node, context.aliases_map)?;
let expression = lower_expression(&node, context)
.map_err(|err| err.extend_function_candidates(context.aliases_map.function_names()))?;
Ok((expression, modifier))
revset_parser::expect_program_with(
&node,
|node| lower_expression(node, context),
|name, span| match name {
"all" => Ok(RevsetModifier::All),
_ => Err(RevsetParseError::with_span(
RevsetParseErrorKind::NoSuchModifier(name.to_owned()),
span,
)),
},
)
.map_err(|err| err.extend_function_candidates(context.aliases_map.function_names()))
}
/// `Some` for rewritten expression, or `None` to reuse the original expression.
@ -2605,10 +2612,6 @@ mod tests {
);
// String pattern isn't allowed at top level.
assert_matches!(
parse(r#"exact:"foo""#),
Err(RevsetParseErrorKind::NotInfixOperator { .. })
);
assert_matches!(
parse(r#"(exact:"foo")"#),
Err(RevsetParseErrorKind::NotInfixOperator { .. })
@ -2902,8 +2905,10 @@ mod tests {
parse_with_aliases("author(A)", [("A", "a")]).unwrap(),
parse("author(a)").unwrap()
);
// However, parentheses are required because top-level x:y is parsed as
// program modifier.
assert_eq!(
parse_with_aliases("author(A)", [("A", "exact:a")]).unwrap(),
parse_with_aliases("author(A)", [("A", "(exact:a)")]).unwrap(),
parse("author(exact:a)").unwrap()
);
@ -2933,6 +2938,16 @@ mod tests {
parse_with_modifier("all:ALL").unwrap()
);
// Top-level alias can be substituted to modifier expression.
assert_eq!(
parse_with_aliases_and_modifier("A", [("A", "all:a")]),
parse_with_modifier("all:a")
);
assert_eq!(
parse_with_aliases_and_modifier("A-", [("A", "all:a")]),
Err(RevsetParseErrorKind::BadAliasExpansion("A".to_owned()))
);
// Multi-level substitution.
assert_eq!(
parse_with_aliases("A", [("A", "BC"), ("BC", "b|C"), ("C", "c")]).unwrap(),
@ -2954,11 +2969,6 @@ mod tests {
parse_with_aliases("A", [("A", "a(")]),
Err(RevsetParseErrorKind::BadAliasExpansion("A".to_owned()))
);
// Modifier isn't allowed in alias definition.
assert_eq!(
parse_with_aliases_and_modifier("A", [("A", "all:a")]),
Err(RevsetParseErrorKind::BadAliasExpansion("A".to_owned()))
);
}
#[test]
@ -3016,6 +3026,12 @@ mod tests {
parse("author(a)|committer(a)").unwrap()
);
// Modifier expression body as parameter.
assert_eq!(
parse_with_aliases_and_modifier("F(a|b)", [("F(x)", "all:x")]).unwrap(),
parse_with_modifier("all:(a|b)").unwrap()
);
// Function and symbol aliases reside in separate namespaces.
assert_eq!(
parse_with_aliases("A()", [("A()", "A"), ("A", "a")]).unwrap(),

52
lib/src/revset_parser.rs
View file

@ -314,6 +314,8 @@ pub enum ExpressionKind<'i> {
Unary(UnaryOp, Box<ExpressionNode<'i>>),
Binary(BinaryOp, Box<ExpressionNode<'i>>, Box<ExpressionNode<'i>>),
FunctionCall(Box<FunctionCallNode<'i>>),
/// `name: body`
Modifier(Box<ModifierNode<'i>>),
/// Identity node to preserve the span in the source text.
AliasExpanded(AliasId<'i>, Box<ExpressionNode<'i>>),
}
@ -342,6 +344,14 @@ impl<'i> FoldableExpression<'i> for ExpressionKind<'i> {
Ok(ExpressionKind::Binary(op, lhs, rhs))
}
ExpressionKind::FunctionCall(function) => folder.fold_function_call(function, span),
ExpressionKind::Modifier(modifier) => {
let modifier = Box::new(ModifierNode {
name: modifier.name,
name_span: modifier.name_span,
body: folder.fold_expression(modifier.body)?,
});
Ok(ExpressionKind::Modifier(modifier))
}
ExpressionKind::AliasExpanded(id, subst) => {
let subst = Box::new(folder.fold_expression(*subst)?);
Ok(ExpressionKind::AliasExpanded(id, subst))
@ -399,42 +409,36 @@ pub enum BinaryOp {
pub type ExpressionNode<'i> = dsl_util::ExpressionNode<'i, ExpressionKind<'i>>;
pub type FunctionCallNode<'i> = dsl_util::FunctionCallNode<'i, ExpressionKind<'i>>;
/// Expression with modifier `name: body`.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct ModifierNode<'i> {
/// Modifier name.
pub name: &'i str,
/// Span of the modifier name.
pub name_span: pest::Span<'i>,
/// Expression body.
pub body: ExpressionNode<'i>,
}
pub(super) fn parse_program(revset_str: &str) -> Result<ExpressionNode, RevsetParseError> {
let mut pairs = RevsetParser::parse(Rule::program, revset_str)?;
let first = pairs.next().unwrap();
parse_expression_node(first.into_inner())
}
pub(super) fn parse_program_with_modifier(
revset_str: &str,
) -> Result<(ExpressionNode, Option<ModifierNode>), RevsetParseError> {
let mut pairs = RevsetParser::parse(Rule::program_with_modifier, revset_str)?;
let first = pairs.next().unwrap();
match first.as_rule() {
Rule::expression => {
let node = parse_expression_node(first.into_inner())?;
Ok((node, None))
}
Rule::expression => parse_expression_node(first.into_inner()),
Rule::program_modifier => {
let (lhs, op) = first.into_inner().collect_tuple().unwrap();
let rhs = pairs.next().unwrap();
assert_eq!(lhs.as_rule(), Rule::identifier);
assert_eq!(op.as_rule(), Rule::pattern_kind_op);
assert_eq!(rhs.as_rule(), Rule::expression);
let modifier = ModifierNode {
let span = lhs.as_span().start_pos().span(&rhs.as_span().end_pos());
let modifier = Box::new(ModifierNode {
name: lhs.as_str(),
name_span: lhs.as_span(),
};
let node = parse_expression_node(rhs.into_inner())?;
Ok((node, Some(modifier)))
body: parse_expression_node(rhs.into_inner())?,
});
let expr = ExpressionKind::Modifier(modifier);
Ok(ExpressionNode::new(expr, span))
}
r => panic!("unexpected revset parse rule: {r:?}"),
}
@ -728,6 +732,22 @@ impl AliasDefinitionParser for RevsetAliasParser {
}
}
/// Applies the given functions to the top-level expression body node with an
/// optional modifier. Alias expansion nodes are unwrapped accordingly.
pub(super) fn expect_program_with<B, M>(
node: &ExpressionNode,
parse_body: impl FnOnce(&ExpressionNode) -> Result<B, RevsetParseError>,
parse_modifier: impl FnOnce(&str, pest::Span<'_>) -> Result<M, RevsetParseError>,
) -> Result<(B, Option<M>), RevsetParseError> {
expect_literal_with(node, |node| match &node.kind {
ExpressionKind::Modifier(modifier) => {
let parsed_modifier = parse_modifier(modifier.name, modifier.name_span)?;
Ok((parse_body(&modifier.body)?, Some(parsed_modifier)))
}
_ => Ok((parse_body(node)?, None)),
})
}
pub(super) fn expect_pattern_with<T, E: Into<Box<dyn error::Error + Send + Sync>>>(
type_name: &str,
node: &ExpressionNode,