From 716edd47d2448a26e51355300703a5febffa2bd0 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 01:03:00 +0200 Subject: [PATCH 1/2] fix: preserve formula references across parsing and edits --- CHANGELOG.md | 4 +++ src/odr/internal/formula/formula_ast.cpp | 6 ++-- .../internal/formula/formula_dependencies.cpp | 20 +++++++++++-- src/odr/internal/formula/formula_parser.cpp | 30 +++++++++---------- src/odr/internal/formula/formula_writer.cpp | 4 ++- .../formula/formula_evaluator_test.cpp | 2 ++ .../internal/formula/formula_writer_test.cpp | 9 ++++++ test/src/sheet_dependencies_test.cpp | 9 ++++++ 8 files changed, 62 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0689638c0..ea81665e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Spreadsheet formulas preserve quoted sheet spans when rewritten, recognize + unprefixed whole-row ranges, and detect cross-sheet dependencies as unresolved. + Extreme reference shifts now yield `#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. diff --git a/src/odr/internal/formula/formula_ast.cpp b/src/odr/internal/formula/formula_ast.cpp index b55397ab6..e04c3b9e2 100644 --- a/src/odr/internal/formula/formula_ast.cpp +++ b/src/odr/internal/formula/formula_ast.cpp @@ -17,11 +17,11 @@ constexpr std::int64_t index_limit = std::numeric_limits::max(); if (!coordinate.has_value() || coordinate->absolute) { return true; } - const std::int64_t moved = static_cast(coordinate->index) + by; - if (moved < 0 || moved > index_limit) { + const auto index = static_cast(coordinate->index); + if (by < -index || by > index_limit - index) { return false; } - coordinate->index = static_cast(moved); + coordinate->index = static_cast(index + by); return true; } diff --git a/src/odr/internal/formula/formula_dependencies.cpp b/src/odr/internal/formula/formula_dependencies.cpp index b3b2fcdf2..ed7141d5a 100644 --- a/src/odr/internal/formula/formula_dependencies.cpp +++ b/src/odr/internal/formula/formula_dependencies.cpp @@ -1,5 +1,7 @@ #include +#include + #include #include #include @@ -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(&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(&node.content)) { - into.extents.push_back(extent_of(range->from, range->to)); + add_extent(range->from, range->to, into); return; } if (node.holds()) { @@ -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 diff --git a/src/odr/internal/formula/formula_parser.cpp b/src/odr/internal/formula/formula_parser.cpp index 9d6fd4be6..6ec2ecd65 100644 --- a/src/odr/internal/formula/formula_parser.cpp +++ b/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 ooxml_sheet() { const std::string_view start = rest(); const std::optional 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 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); diff --git a/src/odr/internal/formula/formula_writer.cpp b/src/odr/internal/formula/formula_writer.cpp index 994ea01a5..214e5331e 100644 --- a/src/odr/internal/formula/formula_writer.cpp +++ b/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 += "!"; diff --git a/test/src/internal/formula/formula_evaluator_test.cpp b/test/src/internal/formula/formula_evaluator_test.cpp index 148d694a3..1de29bd78 100644 --- a/test/src/internal/formula/formula_evaluator_test.cpp +++ b/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) { diff --git a/test/src/internal/formula/formula_writer_test.cpp b/test/src/internal/formula/formula_writer_test.cpp index 4896f6a7d..007ecd1b4 100644 --- a/test/src/internal/formula/formula_writer_test.cpp +++ b/test/src/internal/formula/formula_writer_test.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -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::max(), 0), "#REF!"); + EXPECT_EQ(moved("B2", 0, std::numeric_limits::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)"); } diff --git a/test/src/sheet_dependencies_test.cpp b/test/src/sheet_dependencies_test.cpp index 2a497649d..c7799ba76 100644 --- a/test/src/sheet_dependencies_test.cpp +++ 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{"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{"2!A1"})); +} From 599d21fa8d24d748260095582baf081378f2285a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 16:30:54 +0200 Subject: [PATCH 2/2] docs: wrap the formula reference entry at 80 columns Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea81665e4..2a84a9aad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- Spreadsheet formulas preserve quoted sheet spans when rewritten, recognize - unprefixed whole-row ranges, and detect cross-sheet dependencies as unresolved. - Extreme reference shifts now yield `#REF!` without signed overflow. +- 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