From 1c4888f76903d0aa4c2c5a38c0e5ff3cae49239b Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 1 Nov 2022 23:18:33 +0900 Subject: [PATCH] revset: report bad number of arguments with span --- lib/src/revset.rs | 73 ++++++++++++++++++++----------------- tests/test_revset_output.rs | 50 +++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 33 deletions(-) diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 971764a8c..526823483 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -622,8 +622,8 @@ fn parse_primary_rule( match first.as_rule() { Rule::expression => parse_expression_rule(first.into_inner(), workspace_ctx), Rule::function_name => { - let argument_pairs = pairs.next().unwrap().into_inner(); - parse_function_expression(first, argument_pairs, workspace_ctx) + let arguments_pair = pairs.next().unwrap(); + parse_function_expression(first, arguments_pair, workspace_ctx) } Rule::symbol => parse_symbol_rule(first.into_inner()), _ => { @@ -655,46 +655,46 @@ fn parse_symbol_rule(mut pairs: Pairs) -> Result, Rev fn parse_function_expression( name_pair: Pair, - argument_pairs: Pairs, + arguments_pair: Pair, workspace_ctx: Option<&RevsetWorkspaceContext>, ) -> Result, RevsetParseError> { let name = name_pair.as_str(); match name { "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)?; Ok(expression.parents()) } "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)?; Ok(expression.children()) } "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)?; Ok(expression.ancestors()) } "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)?; Ok(expression.descendants()) } "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)?; Ok(candidates.connected()) } "none" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::none()) } "all" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::all()) } "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)?; Ok(candidates.heads()) } else { @@ -702,40 +702,40 @@ fn parse_function_expression( } } "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)?; Ok(candidates.roots()) } "public_heads" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::public_heads()) } "branches" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::branches()) } "remote_branches" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::remote_branches()) } "tags" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::tags()) } "git_refs" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::git_refs()) } "git_head" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::git_head()) } "merges" => { - expect_no_arguments(name, argument_pairs)?; + expect_no_arguments(name, arguments_pair)?; Ok(RevsetExpression::all().with_parent_count(2..u32::MAX)) } "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 candidates = RevsetExpression::all(); match name { @@ -749,7 +749,9 @@ fn parse_function_expression( } "file" => { 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| { let span = arg.as_span(); let needle = parse_function_argument_to_string(name, arg)?; @@ -764,11 +766,12 @@ fn parse_function_expression( }) .collect::, RevsetParseError>>()?; if paths.is_empty() { - Err(RevsetParseError::new( + Err(RevsetParseError::with_span( RevsetParseErrorKind::InvalidFunctionArguments { name: name.to_owned(), message: "Expected at least 1 argument".to_string(), }, + arguments_span, )) } else { Ok(RevsetExpression::all().with_file(paths)) @@ -786,52 +789,56 @@ fn parse_function_expression( } } -fn expect_no_arguments( - name: &str, - mut argument_pairs: Pairs, -) -> Result<(), RevsetParseError> { +fn expect_no_arguments(name: &str, arguments_pair: Pair) -> Result<(), RevsetParseError> { + let span = arguments_pair.as_span(); + let mut argument_pairs = arguments_pair.into_inner(); if argument_pairs.next().is_none() { Ok(()) } else { - Err(RevsetParseError::new( + Err(RevsetParseError::with_span( RevsetParseErrorKind::InvalidFunctionArguments { name: name.to_owned(), message: "Expected 0 arguments".to_string(), }, + span, )) } } fn expect_one_argument<'i>( name: &str, - argument_pairs: Pairs<'i, Rule>, + arguments_pair: Pair<'i, Rule>, ) -> Result, 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()) { Ok(arg) } else { - Err(RevsetParseError::new( + Err(RevsetParseError::with_span( RevsetParseErrorKind::InvalidFunctionArguments { name: name.to_owned(), message: "Expected 1 argument".to_string(), }, + span, )) } } fn expect_one_optional_argument<'i>( name: &str, - argument_pairs: Pairs<'i, Rule>, + arguments_pair: Pair<'i, Rule>, ) -> Result>, 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()) { Ok(opt_arg) } else { - Err(RevsetParseError::new( + Err(RevsetParseError::with_span( RevsetParseErrorKind::InvalidFunctionArguments { name: name.to_owned(), message: "Expected 0 or 1 arguments".to_string(), }, + span, )) } } diff --git a/tests/test_revset_output.rs b/tests/test_revset_output.rs index d51ba2c90..1de2a351d 100644 --- a/tests/test_revset_output.rs +++ b/tests/test_revset_output.rs @@ -22,6 +22,56 @@ fn test_bad_function_call() { test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); 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)"]); insta::assert_snapshot!(stderr, @r###" Error: Failed to parse revset: --> 1:9