M6: calculation checks: a notation's last name keeps the escaped space that ends it

parse_calculate trimmed the notation with str::trim, which also took an
escaped space ending its last name: "Qty * Total\ " read back as
"Qty * Total\", a field the form doesn't have, while the script's
JavaScript still read "Total ". It now trims to its first and last
token, as the field-reading scan does. notation_storable compares the
names and operators read back with those given, so a notation ending in
a lone \ (which escapes the space after it) is refused; a proptest reads
back notations of any names (from the review).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P6yUtvWGPsJpMLyh6pxSXG
This commit is contained in:
Grzegorz Martens
2026-10-29 20:48:38 +01:00
co-authored by Claude Opus 5.5
parent fd3634cc38
commit d7c8e6ad1b
5 changed files with 47 additions and 11 deletions
+2 -2
View File
@@ -1,7 +1,7 @@
//! Form tools: list fields, fill them (several at once, one undo step), clear the form, and
//! prepare a form (add fields one at a time or as a table, change and delete them).
use pdfcraft_engine::form_scripts::{CalcOp, Calculate, Format, Validate, notation_storable};
use pdfcraft_engine::form_scripts::{CalcOp, Calculate, Format, NOTATION_NOT_STORABLE, Validate, notation_storable};
use pdfcraft_engine::{CalculationIssue, Edit, FieldProps, FieldValue, FormField, FormFieldKind, NewField, field_flags};
use serde_json::{Value, json};
@@ -136,7 +136,7 @@ fn calculate_arg(v: &Value) -> Result<Calculate> {
let n = n.as_str().ok_or_else(|| bad("notation must be a string such as \"Price * Qty\""))?;
// Refused rather than stored as a different formula.
if !notation_storable(n) {
return Err(bad("the notation can't be stored as written: it is empty, or holds EVCALC or */, which would end it early"));
return Err(bad(NOTATION_NOT_STORABLE));
}
return Ok(Calculate::Notation(n.to_string()));
}
@@ -0,0 +1,7 @@
# Seeds for failure cases proptest has generated in the past. It is
# automatically read and these particular cases re-run before any
# novel cases are generated.
#
# It is recommended to check this file in to source control so that
# everyone who runs the test benefits from these saved cases.
cc 81dc3893ad0b46189d4e80199f44f8e58fcf0d193f67e266e37f3846f756a11e # shrinks to names = [" "]
+1 -1
View File
@@ -22,7 +22,7 @@ use calc::{Notation, eval_notation, notation_name, notation_tokens, unique};
pub use calc::{calculate, calculation_reads, notation_js, reads_built_names, rename_in_calculation};
pub use format::{DATE_PRESETS, DateTime, TIME_PRESETS, format_date, format_value, js_number, keystroke, make_number, parse_date, validate};
use script::{Arg, call};
pub use script::{calculate_js, format_js, notation_storable, parse_calculate, parse_format, parse_validate, validate_js};
pub use script::{NOTATION_NOT_STORABLE, calculate_js, format_js, notation_storable, parse_calculate, parse_format, parse_validate, validate_js};
/// The Format tab.
#[derive(Clone, Debug, PartialEq, Default)]
+21 -7
View File
@@ -1,4 +1,4 @@
use super::calc::{js_tokens, notation_js, notation_range};
use super::calc::{js_tokens, notation_js, notation_range, notation_tokens};
use super::format::{DATE_PRESETS, TIME_PRESETS};
use super::{CalcOp, Calculate, Format, Validate};
@@ -255,7 +255,7 @@ pub fn parse_calculate(js: &str) -> Option<Calculate> {
if js.contains("BVCALC")
&& let Some(range) = notation_range(js, &js_tokens(js).1)
{
return Some(Calculate::Notation(js.get(range)?.trim().to_string()));
return Some(Calculate::Notation(notation_text(js.get(range)?).to_string()));
}
let args = call(js, "AFSimple_Calculate")?;
let op = match args.first().and_then(Arg::string)?.as_str() {
@@ -325,13 +325,27 @@ pub fn validate_js(v: &Validate) -> Option<String> {
}
}
/// Why [`notation_storable`] refuses a notation, in the words the Calculate tab and the tools say it.
pub const NOTATION_NOT_STORABLE: &str = "the notation can't be stored as written: it is empty, or holds EVCALC or */, or ends in a lone \\";
/// Whether simplified field notation `expr` can be stored: the script keeps it in a comment,
/// between `BVCALC` and `EVCALC`, and it has to read back from there as itself. An empty one, or
/// one holding `EVCALC` or `*/` (either ends it early), can't.
/// between `BVCALC` and `EVCALC`, and it has to read back from there as the same names and
/// operators. One holding `EVCALC` or `*/` ends early, and one ending in a lone `\` escapes the
/// space after it. An empty one is refused too: it would be stored as `event.value = ;`.
pub fn notation_storable(expr: &str) -> bool {
!expr.trim().is_empty()
&& calculate_js(&Calculate::Notation(expr.to_owned())).and_then(|js| parse_calculate(&js))
== Some(Calculate::Notation(expr.trim().to_owned()))
let tokens = |s: &str| notation_tokens(s).into_iter().map(|(range, _)| s.get(range).unwrap_or_default().to_owned()).collect::<Vec<_>>();
let read = calculate_js(&Calculate::Notation(expr.to_owned())).and_then(|js| parse_calculate(&js));
!tokens(expr).is_empty() && matches!(read, Some(Calculate::Notation(n)) if tokens(&n) == tokens(expr))
}
/// `src` from its first notation token to its last: the spaces around it left out, but not an
/// escaped space that ends the last name (`Total\ `), as `str::trim` would.
fn notation_text(src: &str) -> &str {
let tokens = notation_tokens(src);
match (tokens.first(), tokens.last()) {
(Some((first, _)), Some((last, _))) => src.get(first.start..last.end).unwrap_or_default(),
_ => "",
}
}
pub fn calculate_js(c: &Calculate) -> Option<String> {
+16 -1
View File
@@ -222,7 +222,12 @@ fn hostile_scripts_are_read_in_one_pass() {
fn a_notation_is_stored_only_when_it_reads_back() {
assert!(notation_storable("Price * Qty"));
assert!(notation_storable(" a + b "), "the spaces around it are left out when it is read");
for expr in ["", " ", "a EVCALC b", "a */ x /*", "a */ b"] {
// A last name that ends in a space keeps it, escaped, when the notation is read back.
let total = "Qty * Total\\ ";
assert!(notation_storable(total));
let read = parse_calculate(&calculate_js(&Calculate::Notation(total.into())).unwrap());
assert_eq!(read.as_ref().map(Calculate::reads), Some(vec!["Qty".to_owned(), "Total ".to_owned()]));
for expr in ["", " ", "a EVCALC b", "a */ x /*", "a */ b", "a\\"] {
assert!(!notation_storable(expr), "{expr:?}");
}
}
@@ -274,6 +279,16 @@ proptest::proptest! {
proptest::prop_assert_eq!(back, Some(js));
}
/// A notation of any names, written into a script, reads back from it as written.
#[test]
fn a_notation_reads_back_as_written(names in proptest::collection::vec("\\PC{1,8}", 1..5)) {
proptest::prop_assume!(!names.iter().any(|n| n.contains("VCALC")));
let expr = names.iter().map(|n| notation_name(n)).collect::<Vec<_>>().join(" + ");
let js = calculate_js(&Calculate::Notation(expr.clone())).unwrap();
proptest::prop_assert_eq!(parse_calculate(&js), Some(Calculate::Notation(expr.clone())));
proptest::prop_assert!(notation_storable(&expr), "{expr:?}");
}
/// What a formula reads is the same whether it is read from the tab's settings or from
/// the script they are written as.
#[test]