templater: turn some integer overflow into evaluation error

FWIW, this kind of errors can be checked at parsing phase if we implement
constant folding. I don't think that would matter in practice, though.
This commit is contained in:
Yuya Nishihara 2024-02-27 20:37:00 +09:00
parent 0519954d4f
commit 26c182a0b0
5 changed files with 37 additions and 37 deletions

View file

@ -712,10 +712,10 @@ fn builtin_commit_or_change_id_methods<'repo>(
map.insert("short", |language, build_ctx, self_property, function| {
let ([], [len_node]) = template_parser::expect_arguments(function)?;
let len_property = len_node
.map(|node| template_builder::expect_integer_expression(language, build_ctx, node))
.map(|node| template_builder::expect_usize_expression(language, build_ctx, node))
.transpose()?;
let out_property = TemplateFunction::new((self_property, len_property), |(id, len)| {
Ok(id.short(len.map_or(12, |l| l.try_into().unwrap_or(0))))
Ok(id.short(len.unwrap_or(12)))
});
Ok(language.wrap_string(out_property))
});
@ -725,14 +725,10 @@ fn builtin_commit_or_change_id_methods<'repo>(
let id_prefix_context = &language.id_prefix_context;
let ([], [len_node]) = template_parser::expect_arguments(function)?;
let len_property = len_node
.map(|node| template_builder::expect_integer_expression(language, build_ctx, node))
.map(|node| template_builder::expect_usize_expression(language, build_ctx, node))
.transpose()?;
let out_property = TemplateFunction::new((self_property, len_property), |(id, len)| {
Ok(id.shortest(
language.repo,
id_prefix_context,
len.and_then(|l| l.try_into().ok()).unwrap_or(0),
))
Ok(id.shortest(language.repo, id_prefix_context, len.unwrap_or(0)))
});
Ok(language.wrap_shortest_id_prefix(out_property))
},

View file

@ -238,11 +238,11 @@ fn builtin_operation_id_methods() -> OperationTemplateBuildMethodFnMap<Operation
map.insert("short", |language, build_ctx, self_property, function| {
let ([], [len_node]) = template_parser::expect_arguments(function)?;
let len_property = len_node
.map(|node| template_builder::expect_integer_expression(language, build_ctx, node))
.map(|node| template_builder::expect_usize_expression(language, build_ctx, node))
.transpose()?;
let out_property = TemplateFunction::new((self_property, len_property), |(id, len)| {
let mut hex = id.hex();
hex.truncate(len.map_or(12, |l| l.try_into().unwrap_or(0)));
hex.truncate(len.unwrap_or(12));
Ok(hex)
});
Ok(language.wrap_string(out_property))

View file

@ -24,7 +24,8 @@ use crate::template_parser::{
use crate::templater::{
ConcatTemplate, ConditionalTemplate, IntoTemplate, LabelTemplate, ListPropertyTemplate,
ListTemplate, Literal, PlainTextFormattedProperty, PropertyPlaceholder, ReformatTemplate,
SeparateTemplate, Template, TemplateFunction, TemplateProperty, TimestampRange,
SeparateTemplate, Template, TemplateFunction, TemplateProperty, TemplatePropertyError,
TimestampRange,
};
use crate::{text_util, time_util};
@ -437,8 +438,10 @@ fn build_unary_operation<'a, L: TemplateLanguage<'a>>(
}
UnaryOp::Negate => {
let arg = expect_integer_expression(language, build_ctx, arg_node)?;
// TODO: propagate error on overflow?
language.wrap_integer(TemplateFunction::new(arg, |v| Ok(v.saturating_neg())))
language.wrap_integer(TemplateFunction::new(arg, |v| {
v.checked_neg()
.ok_or_else(|| TemplatePropertyError("Attempt to negate with overflow".into()))
}))
}
};
Ok(Expression::unlabeled(property))
@ -853,13 +856,11 @@ fn build_global_function<'a, L: TemplateLanguage<'a>>(
let property = match function.name {
"fill" => {
let [width_node, content_node] = template_parser::expect_exact_arguments(function)?;
let width = expect_integer_expression(language, build_ctx, width_node)?;
let width = expect_usize_expression(language, build_ctx, width_node)?;
let content = expect_template_expression(language, build_ctx, content_node)?;
let template = ReformatTemplate::new(content, move |context, formatter, recorded| {
match width.extract(context) {
Ok(width) => {
text_util::write_wrapped(formatter, recorded, width.try_into().unwrap_or(0))
}
Ok(width) => text_util::write_wrapped(formatter, recorded, width),
Err(err) => err.format(&(), formatter),
}
});
@ -1029,6 +1030,17 @@ pub fn expect_integer_expression<'a, L: TemplateLanguage<'a>>(
.ok_or_else(|| TemplateParseError::expected_type("Integer", node.span))
}
/// If the given expression `node` is of `Integer` type, converts it to `usize`.
pub fn expect_usize_expression<'a, L: TemplateLanguage<'a>>(
language: &L,
build_ctx: &BuildContext<L::Property>,
node: &ExpressionNode,
) -> TemplateParseResult<Box<dyn TemplateProperty<L::Context, Output = usize> + 'a>> {
let i64_property = expect_integer_expression(language, build_ctx, node)?;
let usize_property = TemplateFunction::new(i64_property, |v| Ok(usize::try_from(v)?));
Ok(Box::new(usize_property))
}
pub fn expect_plain_text_expression<'a, L: TemplateLanguage<'a>>(
language: &L,
build_ctx: &BuildContext<L::Property>,
@ -1490,8 +1502,10 @@ mod tests {
insta::assert_snapshot!(env.render_ok(r#"--2"#), @"2");
insta::assert_snapshot!(env.render_ok(r#"-(3)"#), @"-3");
// No panic on integer overflow. Runtime error might be better.
insta::assert_snapshot!(env.render_ok(r#"-i64_min"#), @"9223372036854775807");
// No panic on integer overflow.
insta::assert_snapshot!(
env.render_ok(r#"-i64_min"#),
@"<Error: Attempt to negate with overflow>");
}
#[test]
@ -1866,20 +1880,11 @@ mod tests {
dog
"###);
// Filling to negatives are clamped to the same as zero
// Filling to negative width is an error
insta::assert_snapshot!(
env.render_ok(r#"fill(-10, "The quick fox jumps over the " ++
label("error", "lazy") ++ " dog\n")"#),
@r###"
The
quick
fox
jumps
over
the
lazy
dog
"###);
@"<Error: out of range integral type conversion attempted>");
// Word-wrap, then indent
insta::assert_snapshot!(

View file

@ -647,8 +647,8 @@ fn test_log_short_shortest_length_parameter() {
"###);
insta::assert_snapshot!(
render(r#"commit_id.short(-100) ++ "|" ++ commit_id.shortest(-100)"#), @r###"
@ |2
|0
@ <Error: out of range integral type conversion attempted>|<Error: out of range integral type conversion attempted>
<Error: out of range integral type conversion attempted>|<Error: out of range integral type conversion attempted>
"###);
insta::assert_snapshot!(
render(r#"commit_id.short(100) ++ "|" ++ commit_id.shortest(100)"#), @r###"

View file

@ -163,12 +163,11 @@ fn test_op_log_template() {
00000 false @ 1970-01-01 00:00:00.000 +00:00 1970-01-01 00:00:00.000 +00:00 less than a microsecond
"###);
// Negative length shouldn't cause panic (and is clamped.)
// TODO: If we add runtime error, this will probably error out.
// Negative length shouldn't cause panic.
insta::assert_snapshot!(render(r#"id.short(-1) ++ "|""#), @r###"
@ |
|
|
@ <Error: out of range integral type conversion attempted>|
<Error: out of range integral type conversion attempted>|
<Error: out of range integral type conversion attempted>|
"###);
// Test the default template, i.e. with relative start time and duration. We