From f6b0b7788ec9d56d9fd03a45516a8727640c75dc Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 15 Mar 2023 12:22:41 +0900 Subject: [PATCH] templater: unify variants of argument count error, embed function name This is similar to the structure of RevsetParseError. It's unlikely we would need to discriminate parsing errors, so let's avoid wasting time on naming things. --- src/template_parser.rs | 83 ++++++++++++++--------------------------- tests/test_templater.rs | 16 ++++---- 2 files changed, 37 insertions(+), 62 deletions(-) diff --git a/src/template_parser.rs b/src/template_parser.rs index 4e3693385..bd3936786 100644 --- a/src/template_parser.rs +++ b/src/template_parser.rs @@ -14,7 +14,6 @@ use std::collections::HashMap; use std::num::ParseIntError; -use std::ops::{RangeFrom, RangeInclusive}; use std::{error, fmt}; use itertools::Itertools as _; @@ -48,13 +47,8 @@ pub enum TemplateParseErrorKind { NoSuchFunction(String), #[error(r#"Method "{name}" doesn't exist for type "{type_name}""#)] NoSuchMethod { type_name: String, name: String }, - // TODO: clean up argument error variants - #[error("Expected {0} arguments")] - InvalidArgumentCountExact(usize), - #[error("Expected {} to {} arguments", .0.start(), .0.end())] - InvalidArgumentCountRange(RangeInclusive), - #[error("Expected at least {} arguments", .0.start)] - InvalidArgumentCountRangeFrom(RangeFrom), + #[error(r#"Function "{name}": {message}"#)] + InvalidArguments { name: String, message: String }, #[error(r#"Expected argument of type "{0}""#)] InvalidArgumentType(String), #[error("Invalid time format")] @@ -121,30 +115,13 @@ impl TemplateParseError { ) } - pub fn invalid_argument_count_exact(count: usize, span: pest::Span<'_>) -> Self { + pub fn invalid_arguments(function: &FunctionCallNode, message: impl Into) -> Self { TemplateParseError::with_span( - TemplateParseErrorKind::InvalidArgumentCountExact(count), - span, - ) - } - - pub fn invalid_argument_count_range( - count: RangeInclusive, - span: pest::Span<'_>, - ) -> Self { - TemplateParseError::with_span( - TemplateParseErrorKind::InvalidArgumentCountRange(count), - span, - ) - } - - pub fn invalid_argument_count_range_from( - count: RangeFrom, - span: pest::Span<'_>, - ) -> Self { - TemplateParseError::with_span( - TemplateParseErrorKind::InvalidArgumentCountRangeFrom(count), - span, + TemplateParseErrorKind::InvalidArguments { + name: function.name.to_owned(), + message: message.into(), + }, + function.args_span, ) } @@ -541,9 +518,9 @@ pub fn expand_aliases<'i>( ExpressionKind::FunctionCall(function) => { if let Some((id, params, defn)) = state.aliases_map.get_function(function.name) { if function.args.len() != params.len() { - return Err(TemplateParseError::invalid_argument_count_exact( - params.len(), - function.args_span, + return Err(TemplateParseError::invalid_arguments( + &function, + format!("Expected {} arguments", params.len()), )); } // Resolve arguments in the current scope, and pass them in to the alias @@ -596,9 +573,9 @@ pub fn expect_no_arguments(function: &FunctionCallNode) -> TemplateParseResult<( if function.args.is_empty() { Ok(()) } else { - Err(TemplateParseError::invalid_argument_count_exact( - 0, - function.args_span, + Err(TemplateParseError::invalid_arguments( + function, + "Expected 0 arguments", )) } } @@ -607,11 +584,9 @@ pub fn expect_no_arguments(function: &FunctionCallNode) -> TemplateParseResult<( pub fn expect_exact_arguments<'a, 'i, const N: usize>( function: &'a FunctionCallNode<'i>, ) -> TemplateParseResult<&'a [ExpressionNode<'i>; N]> { - function - .args - .as_slice() - .try_into() - .map_err(|_| TemplateParseError::invalid_argument_count_exact(N, function.args_span)) + function.args.as_slice().try_into().map_err(|_| { + TemplateParseError::invalid_arguments(function, format!("Expected {N} arguments")) + }) } /// Extracts N required arguments and remainders. @@ -622,9 +597,9 @@ pub fn expect_some_arguments<'a, 'i, const N: usize>( let (required, rest) = function.args.split_at(N); Ok((required.try_into().unwrap(), rest)) } else { - Err(TemplateParseError::invalid_argument_count_range_from( - N.., - function.args_span, + Err(TemplateParseError::invalid_arguments( + function, + format!("Expected at least {N} arguments"), )) } } @@ -643,9 +618,9 @@ pub fn expect_arguments<'a, 'i, const N: usize, const M: usize>( optional.resize(M, None); Ok((required.try_into().unwrap(), optional.try_into().unwrap())) } else { - Err(TemplateParseError::invalid_argument_count_range( - count_range, - function.args_span, + Err(TemplateParseError::invalid_arguments( + function, + format!("Expected {min} to {max} arguments", min = N, max = N + M), )) } } @@ -997,20 +972,20 @@ mod tests { ); // Invalid number of arguments. - assert_eq!( + assert_matches!( with_aliases([("F()", "x")]).parse("F(a)").unwrap_err().kind, - TemplateParseErrorKind::InvalidArgumentCountExact(0), + TemplateParseErrorKind::InvalidArguments { .. } ); - assert_eq!( + assert_matches!( with_aliases([("F(x)", "x")]).parse("F()").unwrap_err().kind, - TemplateParseErrorKind::InvalidArgumentCountExact(1), + TemplateParseErrorKind::InvalidArguments { .. } ); - assert_eq!( + assert_matches!( with_aliases([("F(x,y)", "x ++ y")]) .parse("F(a,b,c)") .unwrap_err() .kind, - TemplateParseErrorKind::InvalidArgumentCountExact(2), + TemplateParseErrorKind::InvalidArguments { .. } ); // Infinite recursion, where the top-level error isn't of RecursiveAlias kind. diff --git a/tests/test_templater.rs b/tests/test_templater.rs index e54709da2..47d13e2e0 100644 --- a/tests/test_templater.rs +++ b/tests/test_templater.rs @@ -180,7 +180,7 @@ fn test_templater_parse_error() { 1 | description.contains() | ^ | - = Expected 1 arguments + = Function "contains": Expected 1 arguments "###); insta::assert_snapshot!(render_err(r#"description.first_line("foo")"#), @r###" @@ -189,7 +189,7 @@ fn test_templater_parse_error() { 1 | description.first_line("foo") | ^---^ | - = Expected 0 arguments + = Function "first_line": Expected 0 arguments "###); insta::assert_snapshot!(render_err(r#"label()"#), @r###" @@ -198,7 +198,7 @@ fn test_templater_parse_error() { 1 | label() | ^ | - = Expected 2 arguments + = Function "label": Expected 2 arguments "###); insta::assert_snapshot!(render_err(r#"label("foo", "bar", "baz")"#), @r###" Error: Failed to parse template: --> 1:7 @@ -206,7 +206,7 @@ fn test_templater_parse_error() { 1 | label("foo", "bar", "baz") | ^-----------------^ | - = Expected 2 arguments + = Function "label": Expected 2 arguments "###); insta::assert_snapshot!(render_err(r#"if()"#), @r###" @@ -215,7 +215,7 @@ fn test_templater_parse_error() { 1 | if() | ^ | - = Expected 2 to 3 arguments + = Function "if": Expected 2 to 3 arguments "###); insta::assert_snapshot!(render_err(r#"if("foo", "bar", "baz", "quux")"#), @r###" Error: Failed to parse template: --> 1:4 @@ -223,7 +223,7 @@ fn test_templater_parse_error() { 1 | if("foo", "bar", "baz", "quux") | ^-------------------------^ | - = Expected 2 to 3 arguments + = Function "if": Expected 2 to 3 arguments "###); insta::assert_snapshot!(render_err(r#"if(label("foo", "bar"), "baz")"#), @r###" @@ -693,7 +693,7 @@ fn test_templater_alias() { 1 | identity() | ^ | - = Expected 1 arguments + = Function "identity": Expected 1 arguments "###); insta::assert_snapshot!(render_err("identity(commit_id, commit_id)"), @r###" Error: Failed to parse template: --> 1:10 @@ -701,7 +701,7 @@ fn test_templater_alias() { 1 | identity(commit_id, commit_id) | ^------------------^ | - = Expected 1 arguments + = Function "identity": Expected 1 arguments "###); insta::assert_snapshot!(render_err(r#"coalesce(label("x", "not boolean"), "")"#), @r###"