diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 9141691..6b7ca27 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -1099,7 +1099,15 @@ impl<'h> Evaluator<'h> { v.scalar().to_bool().unwrap_or(true) } }; - let text = if a1 { text } else { r1c1_to_a1(&text, self.at).unwrap_or(text) }; + let text = if a1 { + text + } else { + match r1c1_to_a1(&text, self.at) { + Ok(Some(a1_text)) => a1_text, + Ok(None) => text, + Err(e) => return err(e), + } + }; match gridcraft_formula::parse(&text) { Ok(e @ (Expr::Ref(_) | Expr::Name(_) | Expr::Struct(_))) => match self.eval(&e) { r @ Ev::R(_) => r, @@ -1426,37 +1434,42 @@ fn same_kind(a: &Value, b: &Value) -> bool { std::mem::discriminant(a) == std::mem::discriminant(b) || (a.is_empty() || b.is_empty()) } -/// Converts an R1C1 reference text to A1 (for INDIRECT(…, FALSE)). -pub fn r1c1_to_a1(s: &str, at: CellRef) -> Option { +/// Converts an R1C1 reference text to A1 (for INDIRECT(…, FALSE)). `Ok(None)` means the text is +/// not R1C1; `Err` means it is R1C1 but points past the sheet. +pub fn r1c1_to_a1(s: &str, at: CellRef) -> Result, CellError> { let (sheet, body) = match s.rsplit_once('!') { Some((sh, b)) => (format!("{sh}!"), b), None => (String::new(), s), }; - let conv = |part: &str| -> Option { + let conv = |part: &str| -> Result, CellError> { let u = part.to_ascii_uppercase(); - let rest = u.strip_prefix('R')?; - let (rpart, cpart) = rest.split_once('C')?; - let num = |p: &str, base: u32| -> Option<(u32, bool)> { - if p.is_empty() { - return Some((base, false)); - } - if let Some(inner) = p.strip_prefix('[').and_then(|x| x.strip_suffix(']')) { - let d: i64 = inner.parse().ok()?; - let v = base as i64 + d; - return if v >= 0 { Some((v as u32, false)) } else { None }; - } - let v: u32 = p.parse().ok()?; - Some((v.checked_sub(1)?, true)) + let Some((rpart, cpart)) = u.strip_prefix('R').and_then(|rest| rest.split_once('C')) else { return Ok(None) }; + // `None`: not R1C1. `Some(None)`: R1C1, but past the sheet. + let num = |p: &str, base: u32, max: u32| -> Option> { + let (v, absolute) = if p.is_empty() { + (Some(i64::from(base)), false) + } else if let Some(inner) = p.strip_prefix('[').and_then(|x| x.strip_suffix(']')) { + (i64::from(base).checked_add(inner.parse().ok()?), false) + } else { + (p.parse::().ok()?.checked_sub(1).map(i64::from), true) + }; + Some(v.filter(|v| (0..i64::from(max)).contains(v)).map(|v| (v as u32, absolute))) }; - let (r, ra) = num(rpart, at.row)?; - let (c, ca) = num(cpart, at.col)?; - Some(format!("{}{}{}{}", if ca { "$" } else { "" }, gridcraft_core::col_to_letters(c), if ra { "$" } else { "" }, r + 1)) + let (Some(r), Some(c)) = (num(rpart, at.row, MAX_ROWS), num(cpart, at.col, MAX_COLS)) else { return Ok(None) }; + let (Some((r, ra)), Some((c, ca))) = (r, c) else { return Err(CellError::Ref) }; + Ok(Some(format!("{}{}{}{}", if ca { "$" } else { "" }, gridcraft_core::col_to_letters(c), if ra { "$" } else { "" }, r + 1))) }; let out = match body.split_once(':') { - Some((a, b)) => format!("{}:{}", conv(a)?, conv(b)?), - None => conv(body)?, + Some((a, b)) => match (conv(a)?, conv(b)?) { + (Some(a), Some(b)) => format!("{a}:{b}"), + _ => return Ok(None), + }, + None => match conv(body)? { + Some(x) => x, + None => return Ok(None), + }, }; - Some(format!("{sheet}{out}")) + Ok(Some(format!("{sheet}{out}"))) } /// The aggregates that can fold a reference without building an array. diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index e8ec305..c5ac79d 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -427,6 +427,37 @@ fn references_functions() { assert_eq!(t.num("B6"), 105.0); } +#[test] +fn indirect_r1c1_out_of_bounds() { + for reference in ["R[9223372036854775807]C", "RC[9223372036854775807]", "R[4294967296]C", "R[4294967291]C", "R[2000000]C", "RC[20000]"] { + let mut t = T::new(); + t.set("D5", &format!("=ROW(INDIRECT(\"{reference}\",FALSE))")); + assert_eq!(t.get("D5"), Value::Error(CellError::Ref), "{reference}"); + } +} + +#[test] +fn indirect_r1c1_past_the_sheet_does_not_fall_back_to_a1() { + let mut t = T::new(); + t.set("RC16385", "78"); + for reference in ["RC16385", "rc16385", "RC1:RC16385"] { + t.set("D5", &format!("=INDIRECT(\"{reference}\",FALSE)")); + assert_eq!(t.get("D5"), Value::Error(CellError::Ref), "{reference}"); + } +} + +#[test] +fn indirect_r1c1_valid_references() { + let mut t = T::new(); + t.set("E6", "11"); + t.set("A1", "22"); + t.set("D4", "33"); + for (reference, expected) in [("R[1]C[1]", 11.0), ("R1C1", 22.0), ("R[-1]C", 33.0)] { + t.set("D5", &format!("=INDIRECT(\"{reference}\",FALSE)")); + assert_eq!(t.num("D5"), expected, "{reference}"); + } +} + #[test] fn relative_names() { let mut t = T::new();