revset: report bad number of arguments with span

This commit is contained in:
Yuya Nishihara 2022-11-01 23:18:33 +09:00
parent b938b5e907
commit 1c4888f769
2 changed files with 90 additions and 33 deletions

View file

@ -622,8 +622,8 @@ fn parse_primary_rule(
match first.as_rule() { match first.as_rule() {
Rule::expression => parse_expression_rule(first.into_inner(), workspace_ctx), Rule::expression => parse_expression_rule(first.into_inner(), workspace_ctx),
Rule::function_name => { Rule::function_name => {
let argument_pairs = pairs.next().unwrap().into_inner(); let arguments_pair = pairs.next().unwrap();
parse_function_expression(first, argument_pairs, workspace_ctx) parse_function_expression(first, arguments_pair, workspace_ctx)
} }
Rule::symbol => parse_symbol_rule(first.into_inner()), Rule::symbol => parse_symbol_rule(first.into_inner()),
_ => { _ => {
@ -655,46 +655,46 @@ fn parse_symbol_rule(mut pairs: Pairs<Rule>) -> Result<Rc<RevsetExpression>, Rev
fn parse_function_expression( fn parse_function_expression(
name_pair: Pair<Rule>, name_pair: Pair<Rule>,
argument_pairs: Pairs<Rule>, arguments_pair: Pair<Rule>,
workspace_ctx: Option<&RevsetWorkspaceContext>, workspace_ctx: Option<&RevsetWorkspaceContext>,
) -> Result<Rc<RevsetExpression>, RevsetParseError> { ) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let name = name_pair.as_str(); let name = name_pair.as_str();
match name { match name {
"parents" => { "parents" => {
let arg = expect_one_argument(name, argument_pairs)?; let arg = expect_one_argument(name, arguments_pair)?;
let expression = parse_expression_rule(arg.into_inner(), workspace_ctx)?; let expression = parse_expression_rule(arg.into_inner(), workspace_ctx)?;
Ok(expression.parents()) Ok(expression.parents())
} }
"children" => { "children" => {
let arg = expect_one_argument(name, argument_pairs)?; let arg = expect_one_argument(name, arguments_pair)?;
let expression = parse_expression_rule(arg.into_inner(), workspace_ctx)?; let expression = parse_expression_rule(arg.into_inner(), workspace_ctx)?;
Ok(expression.children()) Ok(expression.children())
} }
"ancestors" => { "ancestors" => {
let arg = expect_one_argument(name, argument_pairs)?; let arg = expect_one_argument(name, arguments_pair)?;
let expression = parse_expression_rule(arg.into_inner(), workspace_ctx)?; let expression = parse_expression_rule(arg.into_inner(), workspace_ctx)?;
Ok(expression.ancestors()) Ok(expression.ancestors())
} }
"descendants" => { "descendants" => {
let arg = expect_one_argument(name, argument_pairs)?; let arg = expect_one_argument(name, arguments_pair)?;
let expression = parse_expression_rule(arg.into_inner(), workspace_ctx)?; let expression = parse_expression_rule(arg.into_inner(), workspace_ctx)?;
Ok(expression.descendants()) Ok(expression.descendants())
} }
"connected" => { "connected" => {
let arg = expect_one_argument(name, argument_pairs)?; let arg = expect_one_argument(name, arguments_pair)?;
let candidates = parse_expression_rule(arg.into_inner(), workspace_ctx)?; let candidates = parse_expression_rule(arg.into_inner(), workspace_ctx)?;
Ok(candidates.connected()) Ok(candidates.connected())
} }
"none" => { "none" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::none()) Ok(RevsetExpression::none())
} }
"all" => { "all" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::all()) Ok(RevsetExpression::all())
} }
"heads" => { "heads" => {
if let Some(arg) = expect_one_optional_argument(name, argument_pairs)? { if let Some(arg) = expect_one_optional_argument(name, arguments_pair)? {
let candidates = parse_expression_rule(arg.into_inner(), workspace_ctx)?; let candidates = parse_expression_rule(arg.into_inner(), workspace_ctx)?;
Ok(candidates.heads()) Ok(candidates.heads())
} else { } else {
@ -702,40 +702,40 @@ fn parse_function_expression(
} }
} }
"roots" => { "roots" => {
let arg = expect_one_argument(name, argument_pairs)?; let arg = expect_one_argument(name, arguments_pair)?;
let candidates = parse_expression_rule(arg.into_inner(), workspace_ctx)?; let candidates = parse_expression_rule(arg.into_inner(), workspace_ctx)?;
Ok(candidates.roots()) Ok(candidates.roots())
} }
"public_heads" => { "public_heads" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::public_heads()) Ok(RevsetExpression::public_heads())
} }
"branches" => { "branches" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::branches()) Ok(RevsetExpression::branches())
} }
"remote_branches" => { "remote_branches" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::remote_branches()) Ok(RevsetExpression::remote_branches())
} }
"tags" => { "tags" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::tags()) Ok(RevsetExpression::tags())
} }
"git_refs" => { "git_refs" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::git_refs()) Ok(RevsetExpression::git_refs())
} }
"git_head" => { "git_head" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::git_head()) Ok(RevsetExpression::git_head())
} }
"merges" => { "merges" => {
expect_no_arguments(name, argument_pairs)?; expect_no_arguments(name, arguments_pair)?;
Ok(RevsetExpression::all().with_parent_count(2..u32::MAX)) Ok(RevsetExpression::all().with_parent_count(2..u32::MAX))
} }
"description" | "author" | "committer" => { "description" | "author" | "committer" => {
let arg = expect_one_argument(name, argument_pairs)?; let arg = expect_one_argument(name, arguments_pair)?;
let needle = parse_function_argument_to_string(name, arg)?; let needle = parse_function_argument_to_string(name, arg)?;
let candidates = RevsetExpression::all(); let candidates = RevsetExpression::all();
match name { match name {
@ -749,7 +749,9 @@ fn parse_function_expression(
} }
"file" => { "file" => {
if let Some(ctx) = workspace_ctx { if let Some(ctx) = workspace_ctx {
let paths = argument_pairs let arguments_span = arguments_pair.as_span();
let paths = arguments_pair
.into_inner()
.map(|arg| { .map(|arg| {
let span = arg.as_span(); let span = arg.as_span();
let needle = parse_function_argument_to_string(name, arg)?; let needle = parse_function_argument_to_string(name, arg)?;
@ -764,11 +766,12 @@ fn parse_function_expression(
}) })
.collect::<Result<Vec<_>, RevsetParseError>>()?; .collect::<Result<Vec<_>, RevsetParseError>>()?;
if paths.is_empty() { if paths.is_empty() {
Err(RevsetParseError::new( Err(RevsetParseError::with_span(
RevsetParseErrorKind::InvalidFunctionArguments { RevsetParseErrorKind::InvalidFunctionArguments {
name: name.to_owned(), name: name.to_owned(),
message: "Expected at least 1 argument".to_string(), message: "Expected at least 1 argument".to_string(),
}, },
arguments_span,
)) ))
} else { } else {
Ok(RevsetExpression::all().with_file(paths)) Ok(RevsetExpression::all().with_file(paths))
@ -786,52 +789,56 @@ fn parse_function_expression(
} }
} }
fn expect_no_arguments( fn expect_no_arguments(name: &str, arguments_pair: Pair<Rule>) -> Result<(), RevsetParseError> {
name: &str, let span = arguments_pair.as_span();
mut argument_pairs: Pairs<Rule>, let mut argument_pairs = arguments_pair.into_inner();
) -> Result<(), RevsetParseError> {
if argument_pairs.next().is_none() { if argument_pairs.next().is_none() {
Ok(()) Ok(())
} else { } else {
Err(RevsetParseError::new( Err(RevsetParseError::with_span(
RevsetParseErrorKind::InvalidFunctionArguments { RevsetParseErrorKind::InvalidFunctionArguments {
name: name.to_owned(), name: name.to_owned(),
message: "Expected 0 arguments".to_string(), message: "Expected 0 arguments".to_string(),
}, },
span,
)) ))
} }
} }
fn expect_one_argument<'i>( fn expect_one_argument<'i>(
name: &str, name: &str,
argument_pairs: Pairs<'i, Rule>, arguments_pair: Pair<'i, Rule>,
) -> Result<Pair<'i, Rule>, RevsetParseError> { ) -> Result<Pair<'i, Rule>, RevsetParseError> {
let mut argument_pairs = argument_pairs.fuse(); let span = arguments_pair.as_span();
let mut argument_pairs = arguments_pair.into_inner().fuse();
if let (Some(arg), None) = (argument_pairs.next(), argument_pairs.next()) { if let (Some(arg), None) = (argument_pairs.next(), argument_pairs.next()) {
Ok(arg) Ok(arg)
} else { } else {
Err(RevsetParseError::new( Err(RevsetParseError::with_span(
RevsetParseErrorKind::InvalidFunctionArguments { RevsetParseErrorKind::InvalidFunctionArguments {
name: name.to_owned(), name: name.to_owned(),
message: "Expected 1 argument".to_string(), message: "Expected 1 argument".to_string(),
}, },
span,
)) ))
} }
} }
fn expect_one_optional_argument<'i>( fn expect_one_optional_argument<'i>(
name: &str, name: &str,
argument_pairs: Pairs<'i, Rule>, arguments_pair: Pair<'i, Rule>,
) -> Result<Option<Pair<'i, Rule>>, RevsetParseError> { ) -> Result<Option<Pair<'i, Rule>>, RevsetParseError> {
let mut argument_pairs = argument_pairs.fuse(); let span = arguments_pair.as_span();
let mut argument_pairs = arguments_pair.into_inner().fuse();
if let (opt_arg, None) = (argument_pairs.next(), argument_pairs.next()) { if let (opt_arg, None) = (argument_pairs.next(), argument_pairs.next()) {
Ok(opt_arg) Ok(opt_arg)
} else { } else {
Err(RevsetParseError::new( Err(RevsetParseError::with_span(
RevsetParseErrorKind::InvalidFunctionArguments { RevsetParseErrorKind::InvalidFunctionArguments {
name: name.to_owned(), name: name.to_owned(),
message: "Expected 0 or 1 arguments".to_string(), message: "Expected 0 or 1 arguments".to_string(),
}, },
span,
)) ))
} }
} }

View file

@ -22,6 +22,56 @@ fn test_bad_function_call() {
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo"); let repo_path = test_env.env_root().join("repo");
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "all(or:nothing)"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:5
|
1 | all(or:nothing)
| ^--------^
|
= Invalid arguments to revset function "all": Expected 0 arguments
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "parents()"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:9
|
1 | parents()
| ^
|
= Invalid arguments to revset function "parents": Expected 1 argument
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "parents(foo, bar)"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:9
|
1 | parents(foo, bar)
| ^------^
|
= Invalid arguments to revset function "parents": Expected 1 argument
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "heads(foo, bar)"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:7
|
1 | heads(foo, bar)
| ^------^
|
= Invalid arguments to revset function "heads": Expected 0 or 1 arguments
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file()"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:6
|
1 | file()
| ^
|
= Invalid arguments to revset function "file": Expected at least 1 argument
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file(a, not:a-string)"]); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file(a, not:a-string)"]);
insta::assert_snapshot!(stderr, @r###" insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:9 Error: Failed to parse revset: --> 1:9