mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-24 10:46:50 -06:00
fix(engine): reject unparseable conditional-format formulas at creation
Closes #1999 Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1335,6 +1335,47 @@ fn preset_style(name: &str) -> Style {
|
||||
s
|
||||
}
|
||||
|
||||
/// #1999: a formula that doesn't parse must never reach the workbook — Excel refuses to open
|
||||
/// files carrying it, even in repair mode. Validated here so the API and the New Rule dialog
|
||||
/// (which dispatches this command) share one gate, the way Excel's object model refuses.
|
||||
fn check_cf_formula(cmd: &str, what: &str, formula: &str) -> Result<()> {
|
||||
if gridcraft_formula::parse(formula.trim_start_matches('=')).is_err() {
|
||||
return Err(bad(cmd, format!("invalid {what} formula `{formula}` — it does not parse; Excel would reject the saved file")));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn check_cf_value(what: &str, v: &gridcraft_model::features::CfValueKind) -> Result<()> {
|
||||
if let gridcraft_model::features::CfValueKind::Formula(f) = v {
|
||||
check_cf_formula("home.conditionalFormat", what, f)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn check_cf_rule(rule: &CfRule) -> Result<()> {
|
||||
const CMD: &str = "home.conditionalFormat";
|
||||
match rule {
|
||||
CfRule::CellIs { a, b, .. } => {
|
||||
check_cf_formula(CMD, "comparison value", a)?;
|
||||
if let Some(b) = b {
|
||||
check_cf_formula(CMD, "second comparison value", b)?;
|
||||
}
|
||||
}
|
||||
CfRule::Expression { formula, .. } => check_cf_formula(CMD, "rule", formula)?,
|
||||
CfRule::DataBar { min, max, .. } => {
|
||||
check_cf_value("data bar minimum", min)?;
|
||||
check_cf_value("data bar maximum", max)?;
|
||||
}
|
||||
CfRule::ColorScale { stops } => {
|
||||
for (kind, _) in stops {
|
||||
check_cf_value("color scale threshold", kind)?;
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn add_cf(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
let ranges = target_ranges(s, p)?;
|
||||
let sheet = target_sheet(s, p)?;
|
||||
@@ -1443,6 +1484,7 @@ fn add_cf(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
},
|
||||
other => return Err(bad("home.conditionalFormat", format!("unknown rule type `{other}`"))),
|
||||
};
|
||||
check_cf_rule(&rule)?;
|
||||
edit(s, |cx| {
|
||||
let sh = cx.sheet_mut(sheet)?;
|
||||
let priority = sh.cond_formats.iter().map(|c| c.priority).max().unwrap_or(0) + 1;
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
//! #1999: a conditional-format rule whose formula doesn't parse must be rejected at
|
||||
//! creation (Excel's object model refuses it too), not saved and poison the file.
|
||||
|
||||
use gridcraft_engine::Session;
|
||||
use serde_json::json;
|
||||
|
||||
fn session() -> Session {
|
||||
let mut s = Session::new();
|
||||
s.new_workbook();
|
||||
s
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_cf_formulas_are_rejected() {
|
||||
for rule in [
|
||||
// exact repros from the issue's measured table
|
||||
json!({"type": "expression", "formula": "=A1>", "preset": "lightRedFill"}),
|
||||
json!({"type": "cellIs", "operator": "greaterThan", "value": "=1+", "preset": "lightRedFill"}),
|
||||
json!({"type": "dataBar", "min": {"type": "formula", "value": "1+"}}),
|
||||
// more garbage that the parser rejects
|
||||
json!({"type": "expression", "formula": "=IF(A1", "preset": "lightRedFill"}),
|
||||
json!({"type": "cellIs", "operator": "equal", "value": "=((A1", "preset": "lightRedFill"}),
|
||||
] {
|
||||
let err = session()
|
||||
.execute("home.conditionalFormat", json!({"range": "A1:A5", "rule": rule.clone()}))
|
||||
.expect_err("invalid formula {rule} must be rejected");
|
||||
let msg = err.to_string();
|
||||
assert!(msg.to_lowercase().contains("formula"), "error should mention the formula: {msg}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn valid_cf_formulas_still_work() {
|
||||
let mut s = session();
|
||||
// bare comparison without '=' (the issue: this one saves fine today)
|
||||
s.execute("home.conditionalFormat", json!({"range": "A1:A5", "rule": {"type": "expression", "formula": "A1>5", "preset": "lightRedFill"}}))
|
||||
.unwrap();
|
||||
// leading '=' is accepted and stripped
|
||||
s.execute("home.conditionalFormat", json!({"range": "B1:B5", "rule": {"type": "expression", "formula": "=B1>5", "preset": "lightRedFill"}}))
|
||||
.unwrap();
|
||||
// cellIs numeric and text values are not formulas, must pass untouched
|
||||
s.execute(
|
||||
"home.conditionalFormat",
|
||||
json!({"range": "C1:C5", "rule": {"type": "cellIs", "operator": "greaterThan", "value": "5", "preset": "lightRedFill"}}),
|
||||
)
|
||||
.unwrap();
|
||||
s.execute(
|
||||
"home.conditionalFormat",
|
||||
json!({"range": "D1:D5", "rule": {"type": "cellIs", "operator": "equal", "value": "hello there", "preset": "lightRedFill"}}),
|
||||
)
|
||||
.unwrap();
|
||||
// dataBar formula thresholds that parse
|
||||
s.execute("home.conditionalFormat", json!({"range": "E1:E5", "rule": {"type": "dataBar", "min": {"type": "formula", "value": "MIN(E1:E5)"}, "max": {"type": "formula", "value": "MAX(E1:E5)"}}}))
|
||||
.unwrap();
|
||||
// non-formula rule types are untouched
|
||||
s.execute("home.conditionalFormat", json!({"range": "F1:F5", "rule": {"type": "duplicate", "preset": "lightRedFill"}})).unwrap();
|
||||
}
|
||||
Reference in New Issue
Block a user