Skip to content

Commit

Permalink
Check validity of the format string at compile time
Browse files Browse the repository at this point in the history
  • Loading branch information
ogoffart committed May 5, 2023
1 parent 77f16be commit 0267079
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 1 deletion.
74 changes: 73 additions & 1 deletion internal/compiler/passes/resolving.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,78 @@ impl Expression {
ctx.diag,
)
});
let values = subs.collect::<Vec<_>>();

// check format string
{
let mut arg_idx = 0;
let mut pos_max = 0;
let mut pos = 0;
while let Some(mut p) = string[pos..].find(|x| x == '{' || x == '}') {
if string.len() - pos < p + 1 {
ctx.diag.push_error(
"Unescaped trailing '{' in format string. Escape '{' with '{{'".into(),
&node,
);
break;
}
p += pos;

// Skip escaped }
if string.get(p..=p) == Some("}") {
if string.get(p + 1..=p + 1) == Some("}") {
pos = p + 2;
continue;
} else {
ctx.diag.push_error(
"Unescaped '}' in format string. Escape '}' with '}}'".into(),
&node,
);
break;
}
}

// Skip escaped {
if string.get(p + 1..=p + 1) == Some("{") {
pos = p + 2;
continue;
}

// Find the argument
let end = if let Some(end) = string[p..].find('}') {
end + p
} else {
ctx.diag.push_error(
"Unterminated placeholder in format string. '{' must be escaped with '{{'"
.into(),
&node,
);
break;
};
let argument = &string[p + 1..end];
if argument.is_empty() {
arg_idx += 1;
} else if let Ok(n) = argument.parse::<u16>() {
pos_max = pos_max.max(n as usize + 1);
} else {
ctx.diag
.push_error("Invalid '{...}' placeholder in format string. The placeholder must be a number, or braces must be escaped with '{{' and '}}'".into(), &node);
break;
};
pos = end + 1;
}
if arg_idx > 0 && pos_max > 0 {
ctx.diag.push_error(
"Cannot mix positional and non-positional placeholder in format string".into(),
&node,
);
} else if arg_idx > values.len() || pos_max > values.len() {
ctx.diag.push_error(
format!("Format string contains {} placeholders, but only {} extra arguments were given", arg_idx.max(pos_max), values.len()),
&node,
);
}
}

Expression::FunctionCall {
function: Box::new(Expression::BuiltinFunctionReference(
Expand All @@ -547,7 +619,7 @@ impl Expression {
Expression::StringLiteral(string),
Expression::StringLiteral(String::new()), // TODO
Expression::StringLiteral(domain),
Expression::Array { element_ty: Type::String, values: subs.collect() },
Expression::Array { element_ty: Type::String, values },
],
source_location: Some(node.to_source_location()),
}
Expand Down
36 changes: 36 additions & 0 deletions internal/compiler/tests/syntax/basic/tr2.slint
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright © SixtyFPS GmbH <[email protected]>
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-commercial

export component X {
property <string> t1: @tr("fo{}oo", 42px);
// ^error{Cannot convert length to string. Divide by 1px to convert to a plain number}
property <string> t2: @tr("foo{0️⃣}bar", 45);
// ^error{Invalid '\{...\}' placeholder in format string. The placeholder must be a number, or braces must be escaped with '\{\{' and '\}\}'}
property <string> t6: @tr("foo{", 45);
// ^error{Unterminated placeholder in format string. '\{' must be escaped with '\{\{'}
property <string> t7: @tr("foo{bar", 45);
// ^error{Unterminated placeholder in format string. '\{' must be escaped with '\{\{'}
property <string> t8: @tr("foo{bar}bar", 45);
// ^error{Invalid '\{...\}' placeholder in format string. The placeholder must be a number, or braces must be escaped with '\{\{' and '\}\}'}
property <string> t9: @tr("foo{}bar");
// ^error{Format string contains 1 placeholders, but only 0 extra arguments were given}
property <string> t10: @tr("foo{}ba{}r", 42);
// ^error{Format string contains 2 placeholders, but only 1 extra arguments were given}
property <string> t11: @tr("foo{65535}ba{2147483647}r");
// ^error{Invalid '\{...\}' placeholder in format string. The placeholder must be a number, or braces must be escaped with '\{\{' and '\}\}'}
// ^^error{Format string contains 65536 placeholders, but only 0 extra arguments were given}
property <string> t12: @tr("foo{45}bar", 44);
// ^error{Format string contains 46 placeholders, but only 1 extra arguments were given}
property <string> t13: @tr("foo{0}ba{}r", 44, 42);
// ^error{Cannot mix positional and non-positional placeholder in format string}
property <string> t14: @tr("foo{}ba{1}r", 44, 42);
// ^error{Cannot mix positional and non-positional placeholder in format string}
property <string> t15: @tr("fooba🥸{3}🥸r", 44, 42, 45);
// ^error{Format string contains 4 placeholders, but only 3 extra arguments were given}
property <string> t16: @tr("foo{} {}ba{}r", 44, 42);
// ^error{Format string contains 3 placeholders, but only 2 extra arguments were given}
property <string> t17: @tr("fo{}o}r", 44, 42);
// ^error{Unescaped '\}' in format string. Escape '\}' with '\}\}'}


}

0 comments on commit 0267079

Please sign in to comment.