Merge pull request #825 from marcoarnulfo:fix/xls-external-name-cached-value

crates/xlsx/src/tests/xls_tests.rs

@@ -494,6 +494,47 @@ fn sheets_share_one_record_budget() {
 }
 
 #[test]
+fn formulas_using_a_skipped_name_keep_their_saved_results() {
+    // Lbl "ExtRange" = [Source.xlsx]Sheet1!$A$1:$A$3 (PtgArea3d through an external SupBook),
+    // "Local" = Sheet1!$A$1 and "Twice" = ExtRange*2. Only the first is skipped on import; a
+    // name that uses it is still read as it was.
+    let ext = cat(&[&[0x3B], &1u16.to_le_bytes(), &u16s(&[0, 2, 0, 0])]);
+    let local = cat(&[&[0x3A], &0u16.to_le_bytes(), &u16s(&[0, 0])]);
+    let twice = cat(&[&[0x23], &1u32.to_le_bytes(), &ptg_int(2), &[0x05]]);
+    let lbl = |name: &str, rgce: &[u8]| {
+        rec(0x0018, &cat(&[&u16s(&[0]), &[0, name.len() as u8], &u16s(&[rgce.len() as u16, 0, 0]), &[0; 4], &[0], name.as_bytes(), rgce]))
+    };
+    let boundsheet = |pos: u32| rec(0x0085, &cat(&[&pos.to_le_bytes(), &[0, 0, 6, 0], b"Sheet1"]));
+    let globals = |pos: u32| {
+        cat(&[
+            &bof(0x0005),
+            &boundsheet(pos),
+            &rec(0x01AE, &u16s(&[1, 0x0401])),
+            &rec(0x01AE, &cat(&[&u16s(&[1]), &xlstr("Source.xlsx"), &xlstr("Sheet1")])),
+            &rec(0x0017, &cat(&[&u16s(&[2]), &u16s(&[0, 0, 0]), &u16s(&[1, 0, 0])])),
+            &lbl("ExtRange", &ext),
+            &lbl("Local", &local),
+            &lbl("Twice", &twice),
+            &eof(),
+        ])
+    };
+    let sum_ext = cat(&[&[0x43], &1u32.to_le_bytes(), &[0x22, 1], &4u16.to_le_bytes()]);
+    let local_x2 = cat(&[&[0x43], &2u32.to_le_bytes(), &ptg_int(2), &[0x05]]);
+    let sheet = cat(&[
+        &bof(0x0010),
+        &rec(0x0203, &cat(&[&cell(0, 0, 0), &10.0f64.to_le_bytes()])),
+        &formula(5, 1, num(60.0), &sum_ext),
+        &formula(6, 1, num(20.0), &local_x2),
+        &eof(),
+    ]);
+    let pos = globals(0).len() as u32;
+    let (wb, report) = read_xls(&compound("Workbook", &cat(&[&globals(pos), &sheet]))).unwrap();
+    let names: Vec<_> = wb.names.iter().map(|n| (n.name.as_str(), n.formula.as_str())).collect();
+    assert_eq!(names, [("Local", "Sheet1!$A$1"), ("Twice", "ExtRange*2")]);
+    assert_eq!(ftext(&wb, "B6"), None);
+    assert_eq!(v(&wb, 0, "B6"), Value::Number(60.0));
+    assert_eq!(ftext(&wb, "B7").as_deref(), Some("Local*2"));
+    assert!(report.warnings.iter().any(|w| w.contains("defined names") && w.contains("saved results")), "{:?}", report.warnings);
 fn reads_iterative_calculation_settings() {
     // CalcMode, CalcCount, CalcIter and CalcDelta repeat in every worksheet substream; here only
     // the first sheet carries them, so the sheet after it must not reset them to the defaults.

crates/xlsx/src/xls/formula.rs

@@ -51,6 +51,8 @@ pub(super) struct Links {
     pub sheets: Vec<String>,
     /// Defined names (Lbl records), in order; `None` for names that can't be referenced.
     pub names: Vec<String>,
+    /// Per defined name: whether the import skips it (its formula can't be decoded).
+    pub skipped: Vec<bool>,
     pub books: Vec<SupBook>,
     pub xti: Vec<Xti>,
 }
@@ -330,13 +332,13 @@ pub(super) fn decode(rgce: &[u8], rgcb: &[u8], base: CellRef, mode: Mode, links:
                 }
             }
             0x23 => {
-                let i = t.u32()?;
-                let name = usize::try_from(i)
-                    .ok()
-                    .and_then(|i| i.checked_sub(1))
-                    .and_then(|i| links.names.get(i))
-                    .filter(|n| !n.is_empty())
-                    .ok_or(Unsupported("reference to a missing defined name"))?;
+                let i = usize::try_from(t.u32()?).ok().and_then(|i| i.checked_sub(1));
+                let name = i.and_then(|i| links.names.get(i)).filter(|n| !n.is_empty()).ok_or(Unsupported("reference to a missing defined name"))?;
+                // A cell using a name the import drops would calculate to #NAME?: keep its
+                // saved result instead, as for references to other workbooks.
+                if mode != Mode::Name && i.and_then(|i| links.skipped.get(i)) == Some(&true) {
+                    return Err(Unsupported("defined names GridCraft skipped"));
+                }
                 push(&mut stack, kids, Expr::Name(name.clone()))?;
             }
             0x24 | 0x2C => {
@@ -479,6 +481,7 @@ mod tests {
         Links {
             sheets: vec!["Sheet1".into(), "My Data".into()],
             names: vec!["Rate".into()],
+            skipped: vec![false],
             books: vec![
                 SupBook { internal: true, addin: false, names: vec![] },
                 SupBook { internal: false, addin: true, names: vec!["_xlfn.IFERROR".into()] },

crates/xlsx/src/xls/mod.rs

@@ -313,6 +313,9 @@ fn globals(stream: &[u8], budget: &mut Budget, report: &mut ReadReport) -> Resul
             _ => {}
         }
     }
+    // The same test `defined_names` applies, before any cell formula is decoded.
+    let skipped = g.names.iter().map(|l| !l.rgce.is_empty() && decode(&l.rgce, &l.rgcb, CellRef::new(0, 0), Mode::Name, &g.links).is_err()).collect();
+    g.links.skipped = skipped;
     Ok(g)
 }