From a63dbcc329161582ac14f9b4a173ffb6a5801b44 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 24 Apr 2024 20:02:50 +0900 Subject: [PATCH] templater: include actual type name in error messages --- cli/src/template_builder.rs | 38 +++++++++++++++++++------------ cli/src/template_parser.rs | 5 ++-- cli/tests/test_commit_template.rs | 4 ++-- cli/tests/test_templater.rs | 4 ++-- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs index ddaeaba4c..b70b9d620 100644 --- a/cli/src/template_builder.rs +++ b/cli/src/template_builder.rs @@ -1094,9 +1094,11 @@ pub fn expect_boolean_expression<'a, L: TemplateLanguage<'a> + ?Sized>( build_ctx: &BuildContext, node: &ExpressionNode, ) -> TemplateParseResult + 'a>> { - build_expression(language, build_ctx, node)? + let expression = build_expression(language, build_ctx, node)?; + let actual_type = expression.type_name(); + expression .try_into_boolean() - .ok_or_else(|| TemplateParseError::expected_type("Boolean", node.span)) + .ok_or_else(|| TemplateParseError::expected_type("Boolean", actual_type, node.span)) } pub fn expect_integer_expression<'a, L: TemplateLanguage<'a> + ?Sized>( @@ -1104,9 +1106,11 @@ pub fn expect_integer_expression<'a, L: TemplateLanguage<'a> + ?Sized>( build_ctx: &BuildContext, node: &ExpressionNode, ) -> TemplateParseResult + 'a>> { - build_expression(language, build_ctx, node)? + let expression = build_expression(language, build_ctx, node)?; + let actual_type = expression.type_name(); + expression .try_into_integer() - .ok_or_else(|| TemplateParseError::expected_type("Integer", node.span)) + .ok_or_else(|| TemplateParseError::expected_type("Integer", actual_type, node.span)) } /// If the given expression `node` is of `Integer` type, converts it to `isize`. @@ -1138,9 +1142,11 @@ pub fn expect_plain_text_expression<'a, L: TemplateLanguage<'a> + ?Sized>( ) -> TemplateParseResult + 'a>> { // Since any formattable type can be converted to a string property, // the expected type is not a String, but a Template. - build_expression(language, build_ctx, node)? + let expression = build_expression(language, build_ctx, node)?; + let actual_type = expression.type_name(); + expression .try_into_plain_text() - .ok_or_else(|| TemplateParseError::expected_type("Template", node.span)) + .ok_or_else(|| TemplateParseError::expected_type("Template", actual_type, node.span)) } pub fn expect_template_expression<'a, L: TemplateLanguage<'a> + ?Sized>( @@ -1148,9 +1154,11 @@ pub fn expect_template_expression<'a, L: TemplateLanguage<'a> + ?Sized>( build_ctx: &BuildContext, node: &ExpressionNode, ) -> TemplateParseResult> { - build_expression(language, build_ctx, node)? + let expression = build_expression(language, build_ctx, node)?; + let actual_type = expression.type_name(); + expression .try_into_template() - .ok_or_else(|| TemplateParseError::expected_type("Template", node.span)) + .ok_or_else(|| TemplateParseError::expected_type("Template", actual_type, node.span)) } #[cfg(test)] @@ -1326,7 +1334,7 @@ mod tests { 1 | true && 123 | ^-^ | - = Expected expression of type "Boolean" + = Expected expression of type "Boolean", but actual type is "Integer" "###); insta::assert_snapshot!(env.parse_err(r#"description.first_line().foo()"#), @r###" @@ -1360,7 +1368,7 @@ mod tests { 1 | (-empty) | ^---^ | - = Expected expression of type "Integer" + = Expected expression of type "Integer", but actual type is "Boolean" "###); insta::assert_snapshot!(env.parse_err(r#"("foo" ++ "bar").baz()"#), @r###" @@ -1430,7 +1438,7 @@ mod tests { 1 | if(label("foo", "bar"), "baz") | ^-----------------^ | - = Expected expression of type "Boolean" + = Expected expression of type "Boolean", but actual type is "Template" "###); insta::assert_snapshot!(env.parse_err(r#"|x| description"#), @r###" @@ -1455,7 +1463,7 @@ mod tests { 1 | self | ^--^ | - = Expected expression of type "Template" + = Expected expression of type "Template", but actual type is "Self" "###); } @@ -1480,7 +1488,7 @@ mod tests { 1 | if(0, true, false) | ^ | - = Expected expression of type "Boolean" + = Expected expression of type "Boolean", but actual type is "Integer" "###); insta::assert_snapshot!(env.parse_err(r#"if(label("", ""), true, false)"#), @r###" @@ -1489,7 +1497,7 @@ mod tests { 1 | if(label("", ""), true, false) | ^-----------^ | - = Expected expression of type "Boolean" + = Expected expression of type "Boolean", but actual type is "Template" "###); insta::assert_snapshot!(env.parse_err(r#"if(sl0.map(|x| x), true, false)"#), @r###" --> 1:4 @@ -1497,7 +1505,7 @@ mod tests { 1 | if(sl0.map(|x| x), true, false) | ^------------^ | - = Expected expression of type "Boolean" + = Expected expression of type "Boolean", but actual type is "ListTemplate" "###); } diff --git a/cli/src/template_parser.rs b/cli/src/template_parser.rs index 0c68a9d83..c74205c29 100644 --- a/cli/src/template_parser.rs +++ b/cli/src/template_parser.rs @@ -154,8 +154,9 @@ impl TemplateParseError { ) } - pub fn expected_type(type_name: &str, span: pest::Span<'_>) -> Self { - let message = format!(r#"Expected expression of type "{type_name}""#); + pub fn expected_type(expected: &str, actual: &str, span: pest::Span<'_>) -> Self { + let message = + format!(r#"Expected expression of type "{expected}", but actual type is "{actual}""#); TemplateParseError::expression(message, span) } diff --git a/cli/tests/test_commit_template.rs b/cli/tests/test_commit_template.rs index a0cace113..1be13f16b 100644 --- a/cli/tests/test_commit_template.rs +++ b/cli/tests/test_commit_template.rs @@ -54,13 +54,13 @@ fn test_log_parents() { // Commit object isn't printable let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-T", "parents"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse template: Expected expression of type "Template" + Error: Failed to parse template: Expected expression of type "Template", but actual type is "List" Caused by: --> 1:1 | 1 | parents | ^-----^ | - = Expected expression of type "Template" + = Expected expression of type "Template", but actual type is "List" "###); // Redundant argument passed to keyword method diff --git a/cli/tests/test_templater.rs b/cli/tests/test_templater.rs index 08e2308a3..6ab4fbc83 100644 --- a/cli/tests/test_templater.rs +++ b/cli/tests/test_templater.rs @@ -208,7 +208,7 @@ fn test_templater_alias() { 1 | identity(identity(commit_id.short(""))) | ^^ | - = Expected expression of type "Integer" + = Expected expression of type "Integer", but actual type is "String" "###); insta::assert_snapshot!(render_err("commit_id ++ recurse"), @r###" @@ -273,7 +273,7 @@ fn test_templater_alias() { 1 | coalesce(label("x", "not boolean"), "") | ^-----------------------^ | - = Expected expression of type "Boolean" + = Expected expression of type "Boolean", but actual type is "Template" "###); }