fix: preserve formula references across parsing and edits

CHANGELOG.md

@@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- Spreadsheet formulas keep quoted sheet spans when rewritten, read bare
+  whole-row ranges, and report cross-sheet ranges as unresolved. Extreme
+  reference shifts give `#REF!` without signed overflow.
+
 - Number formats preserve fraction literals and percent signs, avoid integer
   overflow for large fractions, and fall back to General when a scientific
   mantissa or fixed fraction cannot be represented.

src/odr/internal/formula/formula_ast.cpp

@@ -17,11 +17,11 @@ constexpr std::int64_t index_limit = std::numeric_limits<std::uint32_t>::max();
   if (!coordinate.has_value() || coordinate->absolute) {
     return true;
   }
-  const std::int64_t moved = static_cast<std::int64_t>(coordinate->index) + by;
-  if (moved < 0 || moved > index_limit) {
+  const auto index = static_cast<std::int64_t>(coordinate->index);
+  if (by < -index || by > index_limit - index) {
     return false;
   }
-  coordinate->index = static_cast<std::uint32_t>(moved);
+  coordinate->index = static_cast<std::uint32_t>(index + by);
   return true;
 }
 

src/odr/internal/formula/formula_dependencies.cpp

@@ -1,5 +1,7 @@
 #include <odr/internal/formula/formula_dependencies.hpp>
 
+#include <odr/internal/util/string_util.hpp>
+
 #include <algorithm>
 #include <limits>
 #include <variant>
@@ -28,17 +30,29 @@ Extent extent_of(const CellReference &from, const CellReference &to) {
           {std::max(from_column, to_column), std::max(from_row, to_row)}}};
 }
 
+void add_extent(const CellReference &from, const CellReference &to,
+                References &into) {
+  if ((from.sheet.has_value() && from.sheet->find(':') != std::string::npos) ||
+      (to.document.has_value() && to.document != from.document) ||
+      (to.sheet.has_value() &&
+       (!from.sheet.has_value() ||
+        !util::string::equals_ignore_case(*from.sheet, *to.sheet)))) {
+    into.complete = false;
+  }
+  into.extents.push_back(extent_of(from, to));
+}
+
 const CellReference *as_cell(const Node &node) {
   return std::get_if<CellReference>(&node.content);
 }
 
 void collect(const Node &node, References &into) {
   if (const auto *cell = as_cell(node)) {
-    into.extents.push_back(extent_of(*cell, *cell));
+    add_extent(*cell, *cell, into);
     return;
   }
   if (const auto *range = std::get_if<RangeReference>(&node.content)) {
-    into.extents.push_back(extent_of(range->from, range->to));
+    add_extent(range->from, range->to, into);
     return;
   }
   if (node.holds<NameReference>()) {
@@ -52,7 +66,7 @@ void collect(const Node &node, References &into) {
     const CellReference *from = as_cell(node.children.front());
     const CellReference *to = as_cell(node.children.back());
     if (from != nullptr && to != nullptr) {
-      into.extents.push_back(extent_of(*from, *to));
+      add_extent(*from, *to, into);
       return;
     }
     // a corner that is no reference: what the range spans cannot be read

src/odr/internal/formula/formula_parser.cpp

@@ -295,6 +295,12 @@ class Parser final : private TextCursor {
       return error_literal();
     }
     if (str::is_ascii_digit(c) || (c == '.' && str::is_ascii_digit(peek(1)))) {
+      if (m_syntax == Syntax::ooxml) {
+        const std::size_t digits = peek_while(str::is_ascii_digit).size();
+        if (digits != 0 && peek(digits) == ':') {
+          return ooxml_primary();
+        }
+      }
       return number_literal();
     }
     if (m_syntax == Syntax::opendocument) {
@@ -612,34 +618,28 @@ class Parser final : private TextCursor {
   [[nodiscard]] std::optional<std::string> ooxml_sheet() {
     const std::string_view start = rest();
     const std::optional<std::string> first = quoted('\'');
-    std::string spelled;
-    if (first.has_value()) {
-      spelled = "'" + *first + "'";
-    } else if (const std::string_view name = take_name(); !name.empty()) {
-      spelled = std::string(name);
-    } else {
+    if (!first.has_value() && take_name().empty()) {
       seek(start);
       return {};
     }
     if (peek() == '!') {
+      const std::string spelling(start.substr(0, start.size() - rest().size()));
       advance(1);
-      return first.has_value() ? *first : spelled;
+      return first.has_value() && first->find(':') == std::string::npos
+                 ? *first
+                 : spelling;
     }
     if (peek() == ':') {
       advance(1);
-      std::string span = spelled + ":";
-      if (const std::optional<std::string> second = quoted('\'');
-          second.has_value()) {
-        span += "'" + *second + "'";
-      } else if (const std::string_view name = take_name(); !name.empty()) {
-        span += std::string(name);
-      } else {
+      if (!quoted('\'').has_value() && take_name().empty()) {
         seek(start);
         return {};
       }
       if (peek() == '!') {
+        const std::string spelling(
+            start.substr(0, start.size() - rest().size()));
         advance(1);
-        return span;
+        return spelling;
       }
     }
     seek(start);

src/odr/internal/formula/formula_writer.cpp

@@ -300,7 +300,9 @@ class Writer final {
       result += "[" + *document + "]";
     }
     if (sheet.has_value()) {
-      result += (needs_quotes(*sheet) ? quote(*sheet, '\'') : *sheet) + "!";
+      const bool span = sheet->find(':') != std::string::npos;
+      result +=
+          (!span && needs_quotes(*sheet) ? quote(*sheet, '\'') : *sheet) + "!";
     } else if (document.has_value()) {
       // `[1]!Total` names the other workbook itself, with no sheet between
       result += "!";

test/src/internal/formula/formula_evaluator_test.cpp

@@ -206,6 +206,8 @@ TEST(FormulaEvaluator, a_whole_column_reaches_past_the_extent_of_its_sheet) {
   // `t` states one row, and the formula sits in row 30
   EXPECT_EQ(xlsx("=t!A:A"), number(0));
   EXPECT_EQ(xlsx("=SUM(t!A:A)"), number(5));
+  EXPECT_EQ(xlsx("=SUM(t!1:1)"), number(5));
+  EXPECT_EQ(xlsx("=SUM(4:5)"), number(2));
 }
 
 TEST(FormulaEvaluator, a_libreoffice_boolean_in_a_join_follows_its_version) {

test/src/internal/formula/formula_writer_test.cpp

@@ -5,6 +5,7 @@
 #include <gtest/gtest.h>
 
 #include <cstdint>
+#include <limits>
 #include <optional>
 #include <string>
 
@@ -93,6 +94,11 @@ TEST(FormulaWriter, an_ooxml_expression_is_written_as_it_was_read) {
   EXPECT_EQ(ooxml("#DIV/0!"), "#DIV/0!");
   EXPECT_EQ(ooxml("TRUE"), "TRUE");
   EXPECT_EQ(ooxml("A:A"), "A:A");
+  EXPECT_EQ(ooxml("SUM(1:3)"), "SUM(1:3)");
+  EXPECT_EQ(ooxml("SUM(Sheet1:Sheet3!A1)"), "SUM(Sheet1:Sheet3!A1)");
+  EXPECT_EQ(ooxml("SUM('A B':'C D'!A1)"), "SUM('A B':'C D'!A1)");
+  EXPECT_EQ(ooxml("SUM('A B:C D'!A1)"), "SUM('A B:C D'!A1)");
+  EXPECT_EQ(ooxml("SUM('A''B':'C D'!A1)"), "SUM('A''B':'C D'!A1)");
   EXPECT_EQ(ooxml("SUM((A1:A2,B1:B2))"), "SUM((A1:A2,B1:B2))");
   EXPECT_EQ(ooxml("[1]!Total"), "[1]!Total");
 }
@@ -129,6 +135,8 @@ TEST(FormulaWriter, a_relative_reference_moves_and_an_absolute_one_does_not) {
 TEST(FormulaWriter, a_reference_moved_off_the_grid_is_lost) {
   EXPECT_EQ(moved("A1+B2", 0, -1), "#REF!+B1");
   EXPECT_EQ(moved("SUM(A1:B2)", -1, 0), "SUM(#REF!)");
+  EXPECT_EQ(moved("B2", std::numeric_limits<std::int64_t>::max(), 0), "#REF!");
+  EXPECT_EQ(moved("B2", 0, std::numeric_limits<std::int64_t>::min()), "#REF!");
 }
 
 TEST(FormulaWriter, what_a_shift_does_not_name_is_left_alone) {
@@ -151,6 +159,7 @@ TEST(FormulaWriter, an_inserted_row_grows_a_range_it_falls_inside) {
 
 TEST(FormulaWriter, a_row_edit_moves_whole_rows_and_leaves_whole_columns) {
   EXPECT_EQ(inserted("SUM($3:$5)", 3, 1), "SUM($3:$6)");
+  EXPECT_EQ(inserted("SUM(3:5)", 3, 1), "SUM(3:6)");
   EXPECT_EQ(inserted("SUM(A:B)", 0, 1), "unmoved SUM(A:B)");
 }
 

test/src/sheet_dependencies_test.cpp

@@ -211,3 +211,12 @@ TEST(SheetDependencies, a_formula_after_a_merge_is_at_its_own_position) {
   EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 0, 1))),
             (std::vector<std::string>{"0!C1"}));
 }
+
+TEST(SheetDependencies, a_range_across_sheets_is_unresolved) {
+  const Document document = decode(flat_document(
+      sheet("First", row(number("1"))) + sheet("Last", row(number("2"))) +
+      sheet("Report", row(computed("of:=SUM([First.A1:Last.A1])") +
+                          computed("of:=SUM([First.A1:FIRST.A1])")))));
+  EXPECT_EQ(spelled(document.unresolved_formulas()),
+            (std::vector<std::string>{"2!A1"}));
+}