templater: save alias chain to report type/name error in original context

Since type/name checking is made after alias substitution, we need to preserve
the original context to generate a readable error message.

We could instead attach a stack of (alias_id, span) to ExpressionNode, but
the extra AliasExpanded node helps to capture downstream error by a single
.map_err() call.
This commit is contained in:
Yuya Nishihara 2023-02-12 21:44:33 +09:00
parent bfdaaa4257
commit b44148871a
2 changed files with 51 additions and 5 deletions

View file

@ -225,6 +225,8 @@ enum ExpressionKind<'i> {
List(Vec<ExpressionNode<'i>>), List(Vec<ExpressionNode<'i>>),
FunctionCall(FunctionCallNode<'i>), FunctionCall(FunctionCallNode<'i>),
MethodCall(MethodCallNode<'i>), MethodCall(MethodCallNode<'i>),
/// Identity node to preserve the span in the source template text.
AliasExpanded(TemplateAliasId<'i>, Box<ExpressionNode<'i>>),
} }
#[derive(Clone, Debug, PartialEq)] #[derive(Clone, Debug, PartialEq)]
@ -464,10 +466,10 @@ fn expand_aliases<'i>(
} }
fn expand_defn<'i>( fn expand_defn<'i>(
id: TemplateAliasId<'_>, id: TemplateAliasId<'i>,
defn: &'i str, defn: &'i str,
locals: &HashMap<&str, ExpressionNode<'i>>, locals: &HashMap<&str, ExpressionNode<'i>>,
span: pest::Span<'_>, span: pest::Span<'i>,
state: State<'_, 'i>, state: State<'_, 'i>,
) -> TemplateParseResult<ExpressionNode<'i>> { ) -> TemplateParseResult<ExpressionNode<'i>> {
// The stack should be short, so let's simply do linear search and duplicate. // The stack should be short, so let's simply do linear search and duplicate.
@ -487,6 +489,9 @@ fn expand_aliases<'i>(
// Parsed defn could be cached if needed. // Parsed defn could be cached if needed.
parse_template(defn) parse_template(defn)
.and_then(|node| expand_node(node, expanding_state)) .and_then(|node| expand_node(node, expanding_state))
.map(|node| {
ExpressionNode::new(ExpressionKind::AliasExpanded(id, Box::new(node)), span)
})
.map_err(|e| e.within_alias_expansion(id, span)) .map_err(|e| e.within_alias_expansion(id, span))
} }
@ -559,6 +564,12 @@ fn expand_aliases<'i>(
}); });
Ok(node) Ok(node)
} }
ExpressionKind::AliasExpanded(id, subst) => {
// Just in case the original tree contained AliasExpanded node.
let subst = Box::new(expand_node(*subst, state)?);
node.kind = ExpressionKind::AliasExpanded(id, subst);
Ok(node)
}
} }
} }
@ -1088,6 +1099,8 @@ fn build_expression<'a, C: 'a>(
} }
ExpressionKind::FunctionCall(function) => build_global_function(function, build_keyword), ExpressionKind::FunctionCall(function) => build_global_function(function, build_keyword),
ExpressionKind::MethodCall(method) => build_method_call(method, build_keyword), ExpressionKind::MethodCall(method) => build_method_call(method, build_keyword),
ExpressionKind::AliasExpanded(id, subst) => build_expression(subst, build_keyword)
.map_err(|e| e.within_alias_expansion(*id, node.span)),
} }
} }
@ -1181,6 +1194,7 @@ mod tests {
let function = normalize_function_call(method.function); let function = normalize_function_call(method.function);
ExpressionKind::MethodCall(MethodCallNode { object, function }) ExpressionKind::MethodCall(MethodCallNode { object, function })
} }
ExpressionKind::AliasExpanded(_, subst) => normalize_tree(*subst).kind,
}; };
ExpressionNode { ExpressionNode {
kind: normalized_kind, kind: normalized_kind,

View file

@ -428,9 +428,14 @@ fn test_templater_alias() {
= expected identifier = expected identifier
"###); "###);
// TODO: outer template substitution should be reported too
insta::assert_snapshot!(render_err("commit_id name_error"), @r###" insta::assert_snapshot!(render_err("commit_id name_error"), @r###"
Error: Failed to parse template: --> 1:1 Error: Failed to parse template: --> 1:11
|
1 | commit_id name_error
| ^--------^
|
= Alias "name_error" cannot be expanded
--> 1:1
| |
1 | unknown_id 1 | unknown_id
| ^--------^ | ^--------^
@ -438,6 +443,27 @@ fn test_templater_alias() {
= Keyword "unknown_id" doesn't exist = Keyword "unknown_id" doesn't exist
"###); "###);
insta::assert_snapshot!(render_err(r#"identity(identity(commit_id.short("")))"#), @r###"
Error: Failed to parse template: --> 1:1
|
1 | identity(identity(commit_id.short("")))
| ^-------------------------------------^
|
= Alias "identity()" cannot be expanded
--> 1:10
|
1 | identity(identity(commit_id.short("")))
| ^---------------------------^
|
= Alias "identity()" cannot be expanded
--> 1:35
|
1 | identity(identity(commit_id.short("")))
| ^^
|
= Expected argument of type "Integer"
"###);
insta::assert_snapshot!(render_err("commit_id recurse"), @r###" insta::assert_snapshot!(render_err("commit_id recurse"), @r###"
Error: Failed to parse template: --> 1:11 Error: Failed to parse template: --> 1:11
| |
@ -483,7 +509,13 @@ fn test_templater_alias() {
"###); "###);
insta::assert_snapshot!(render_err(r#"coalesce(label("x", "not boolean"), "")"#), @r###" insta::assert_snapshot!(render_err(r#"coalesce(label("x", "not boolean"), "")"#), @r###"
Error: Failed to parse template: --> 1:10 Error: Failed to parse template: --> 1:1
|
1 | coalesce(label("x", "not boolean"), "")
| ^-------------------------------------^
|
= Alias "coalesce()" cannot be expanded
--> 1:10
| |
1 | coalesce(label("x", "not boolean"), "") 1 | coalesce(label("x", "not boolean"), "")
| ^-----------------------^ | ^-----------------------^