From f2855af87f9456afe0a892d69d28c76537af5643 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 11 Sep 2026 23:28:45 +0200 Subject: [PATCH] feat(spreadsheet): read the formula of every cell of a shared group [ECMA-376] 18.3.1.40 spells a shared formula on the group's master alone, and a member states its `si` and nothing else. A member reported an empty formula, so a formula bar had nothing to show and a dependency has nothing to read. The parser collects the masters per sheet, and `sheet_cell_value` reads a member through the one its `si` names: parse the master's expression, move every relative reference by the offset between the two cells, and write it again. An absolute axis does not move, and a reference moved off the grid becomes `#REF!`, as a sheet makes it. That takes two pieces beside the parser, both in `internal/formula`: `shift` over the tree, and a writer that spells a tree back in either syntax. The writer drops a parenthesis the precedence already states, so what it writes is the tree rather than the producer's own text. A number that overflows to infinity no longer parses, because no formula spells one. A master the parser cannot read is handed out as it stands, and a member whose `si` names no master stays set and empty: it computes, and nothing here can spell what. Step 3.1 of `docs/design/spreadsheet-editing.md`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VVmjmddv2Ui17Nptc1ggue --- CHANGELOG.md | 4 + CMakeLists.txt | 2 + docs/design/spreadsheet-editing.md | 6 + src/odr/internal/formula/formula_ast.cpp | 54 +++ src/odr/internal/formula/formula_ast.hpp | 4 + src/odr/internal/formula/formula_parser.cpp | 4 +- src/odr/internal/formula/formula_writer.cpp | 359 ++++++++++++++++++ src/odr/internal/formula/formula_writer.hpp | 14 + src/odr/internal/ooxml/spreadsheet/AGENTS.md | 14 +- .../ooxml_spreadsheet_document.cpp | 44 ++- .../ooxml_spreadsheet_element_registry.hpp | 8 + .../spreadsheet/ooxml_spreadsheet_parser.cpp | 9 + test/CMakeLists.txt | 1 + .../internal/formula/formula_parser_test.cpp | 1 + .../internal/formula/formula_writer_test.cpp | 94 +++++ .../ooxml/ooxml_spreadsheet_value_test.cpp | 40 +- 16 files changed, 651 insertions(+), 7 deletions(-) create mode 100644 src/odr/internal/formula/formula_ast.cpp create mode 100644 src/odr/internal/formula/formula_writer.cpp create mode 100644 src/odr/internal/formula/formula_writer.hpp create mode 100644 test/src/internal/formula/formula_writer_test.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index e013f4f40..1f0dac0b4 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 +- A shared formula in an `.xlsx` is read for every cell of its group, not only + the master that spells it: `SheetCell::value().formula()` answers a member + with the expression moved to it, and `#REF!` where it moved off the grid. + - `TablePosition::try_to_column_num` / `try_to_row_num` read a column or row spelling as an optional instead of throwing, and without case. **Fix**: `to_column_num` wrapped silently past the index range instead of refusing. diff --git a/CMakeLists.txt b/CMakeLists.txt index cbcc97432..609af5c99 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -188,7 +188,9 @@ set(ODR_SOURCE_FILES "src/odr/internal/csv/csv_file.cpp" "src/odr/internal/csv/csv_util.cpp" + "src/odr/internal/formula/formula_ast.cpp" "src/odr/internal/formula/formula_parser.cpp" + "src/odr/internal/formula/formula_writer.cpp" "src/odr/internal/html/common.cpp" "src/odr/internal/html/document.cpp" diff --git a/docs/design/spreadsheet-editing.md b/docs/design/spreadsheet-editing.md index ec82b709f..3b7059b51 100644 --- a/docs/design/spreadsheet-editing.md +++ b/docs/design/spreadsheet-editing.md @@ -467,6 +467,12 @@ Each step ships on its own. "Both" means `.ods` and `.xlsx`. where the two part. A named expression, a reference over several sheets and a spelling past the grid (`A0`) stay opaque names; a formula that does not parse answers nothing. + + A writer and a `shift` over the tree come with it, which is what makes an + ooxml shared formula readable: a group spells its expression on the master + alone ([ECMA-376] 18.3.1.40), so a member now reads it moved by the offset + between the two cells, `#REF!` where that leaves the grid. An array + formula's members carry no `` at all and still report none. 2. Reference extraction → dependency graph per document; `Document` answers "which cells depend on this position". 3. View: a commit marks dependents stale (a class, the host is told); the diff --git a/src/odr/internal/formula/formula_ast.cpp b/src/odr/internal/formula/formula_ast.cpp new file mode 100644 index 000000000..e83c9bc44 --- /dev/null +++ b/src/odr/internal/formula/formula_ast.cpp @@ -0,0 +1,54 @@ +#include + +#include +#include + +namespace odr::internal::formula { + +namespace { + +constexpr std::int64_t index_limit = std::numeric_limits::max(); + +[[nodiscard]] bool shift_coordinate(std::optional &coordinate, + const std::int64_t by) { + if (!coordinate.has_value() || coordinate->absolute) { + return true; + } + const std::int64_t moved = static_cast(coordinate->index) + by; + if (moved < 0 || moved > index_limit) { + return false; + } + coordinate->index = static_cast(moved); + return true; +} + +[[nodiscard]] bool shift_cell(CellReference &cell, const std::int64_t columns, + const std::int64_t rows) { + return shift_coordinate(cell.column, columns) && + shift_coordinate(cell.row, rows); +} + +} // namespace + +} // namespace odr::internal::formula + +namespace odr::internal { + +void formula::shift(Node &node, const std::int64_t columns, + const std::int64_t rows) { + if (auto *cell = std::get_if(&node.content)) { + if (!shift_cell(*cell, columns, rows)) { + node.content = ErrorLiteral{ErrorType::reference}; + } + } else if (auto *range = std::get_if(&node.content)) { + if (!shift_cell(range->from, columns, rows) || + !shift_cell(range->to, columns, rows)) { + node.content = ErrorLiteral{ErrorType::reference}; + } + } + for (Node &child : node.children) { + shift(child, columns, rows); + } +} + +} // namespace odr::internal diff --git a/src/odr/internal/formula/formula_ast.hpp b/src/odr/internal/formula/formula_ast.hpp index 619f40e70..6c025bb8d 100644 --- a/src/odr/internal/formula/formula_ast.hpp +++ b/src/odr/internal/formula/formula_ast.hpp @@ -168,4 +168,8 @@ struct Node final { } }; +/// Moves every relative reference in @p node by (@p columns, @p rows). An +/// absolute (`$`) axis stays, and one moved off the grid becomes `#REF!`. +void shift(Node &node, std::int64_t columns, std::int64_t rows); + } // namespace odr::internal::formula diff --git a/src/odr/internal/formula/formula_parser.cpp b/src/odr/internal/formula/formula_parser.cpp index f02e5d399..07ab53a8c 100644 --- a/src/odr/internal/formula/formula_parser.cpp +++ b/src/odr/internal/formula/formula_parser.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -410,7 +411,8 @@ class Parser final : private TextCursor { const std::string text(rest().substr(0, length)); char *end = nullptr; const double value = std::strtod(text.c_str(), &end); - if (end != text.c_str() + text.size()) { + // an overflow answers infinity, which no formula spells + if (end != text.c_str() + text.size() || !std::isfinite(value)) { return {}; } advance(length); diff --git a/src/odr/internal/formula/formula_writer.cpp b/src/odr/internal/formula/formula_writer.cpp new file mode 100644 index 000000000..cb552f310 --- /dev/null +++ b/src/odr/internal/formula/formula_writer.cpp @@ -0,0 +1,359 @@ +#include + +#include + +#include + +#include +#include +#include +#include +#include +#include + +#include + +namespace odr::internal::formula { + +namespace str = util::string; + +namespace { + +/// How tightly an operator binds, lowest first. A child binding less tightly +/// than its parent needs the parenthesis the tree no longer carries. +enum Precedence : std::uint8_t { + comparison, + concatenation, + additive, + multiplicative, + exponentiation, + sign, + percent, + reference, + primary, +}; + +Precedence precedence_of(const BinaryOperator op) { + switch (op) { + case BinaryOperator::equal: + case BinaryOperator::not_equal: + case BinaryOperator::less: + case BinaryOperator::less_equal: + case BinaryOperator::greater: + case BinaryOperator::greater_equal: + return Precedence::comparison; + case BinaryOperator::concat: + return Precedence::concatenation; + case BinaryOperator::add: + case BinaryOperator::subtract: + return Precedence::additive; + case BinaryOperator::multiply: + case BinaryOperator::divide: + return Precedence::multiplicative; + case BinaryOperator::power: + return Precedence::exponentiation; + case BinaryOperator::range: + case BinaryOperator::intersect: + case BinaryOperator::unite: + return Precedence::reference; + } + return Precedence::primary; +} + +Precedence precedence_of(const Node &node) { + if (node.holds()) { + return precedence_of(node.get().op); + } + if (node.holds()) { + return node.get().op == UnaryOperator::percent + ? Precedence::percent + : Precedence::sign; + } + return Precedence::primary; +} + +std::string_view spelling_of(const BinaryOperator op) { + switch (op) { + case BinaryOperator::add: + return "+"; + case BinaryOperator::subtract: + return "-"; + case BinaryOperator::multiply: + return "*"; + case BinaryOperator::divide: + return "/"; + case BinaryOperator::power: + return "^"; + case BinaryOperator::concat: + return "&"; + case BinaryOperator::equal: + return "="; + case BinaryOperator::not_equal: + return "<>"; + case BinaryOperator::less: + return "<"; + case BinaryOperator::less_equal: + return "<="; + case BinaryOperator::greater: + return ">"; + case BinaryOperator::greater_equal: + return ">="; + case BinaryOperator::range: + return ":"; + case BinaryOperator::intersect: + return "!"; + case BinaryOperator::unite: + return "~"; + } + return "+"; +} + +std::string_view spelling_of(const ErrorType type) { + switch (type) { + case ErrorType::null: + return "#NULL!"; + case ErrorType::division: + return "#DIV/0!"; + case ErrorType::value: + return "#VALUE!"; + case ErrorType::reference: + return "#REF!"; + case ErrorType::name: + return "#NAME?"; + case ErrorType::number: + return "#NUM!"; + case ErrorType::not_available: + return "#N/A"; + } + return "#NULL!"; +} + +/// A quote inside a quoted run is written twice, which is how both syntaxes +/// escape one. +std::string quote(const std::string_view text, const char mark) { + std::string result(1, mark); + for (const char c : text) { + if (c == mark) { + result += mark; + } + result += c; + } + result += mark; + return result; +} + +/// A sheet name keeps its quotes only where it needs them: one holding +/// anything but a letter, a digit or `_`, or opening with a digit, is quoted. +bool needs_quotes(const std::string_view name) { + return name.empty() || str::is_ascii_digit(name.front()) || + !std::ranges::all_of(name, [](const char c) { + return str::is_ascii_letter_or_digit(c) || c == '_'; + }); +} + +std::string spell_coordinate(const std::optional &coordinate, + const bool column) { + if (!coordinate.has_value()) { + return ""; + } + return (coordinate->absolute ? "$" : "") + + (column ? TablePosition::to_column_string(coordinate->index) + : TablePosition::to_row_string(coordinate->index)); +} + +class Writer final { +public: + explicit Writer(const Syntax syntax) : m_syntax{syntax} {} + + [[nodiscard]] std::string write(const Node &node) const { + return std::visit( + [&](const auto &content) { return write_content(content, node); }, + node.content); + } + +private: + Syntax m_syntax{Syntax::ooxml}; + + [[nodiscard]] char separator() const { + return m_syntax == Syntax::opendocument ? ';' : ','; + } + + /// The child, parenthesised where its operator binds less tightly than the + /// one above — or equally, on the right: `1-(2-3)` is not `1-2-3`. + [[nodiscard]] std::string nested(const Node &child, const Precedence parent, + const bool right) const { + const Precedence own = precedence_of(child); + if (own < parent || (right && own == parent)) { + return "(" + write(child) + ")"; + } + return write(child); + } + + [[nodiscard]] std::string write_content(const NumberLiteral &content, + const Node &) const { + return fmt::format("{}", content.value); + } + + [[nodiscard]] std::string write_content(const StringLiteral &content, + const Node &) const { + return quote(content.value, '"'); + } + + [[nodiscard]] std::string write_content(const BooleanLiteral &content, + const Node &) const { + if (m_syntax == Syntax::opendocument) { + return content.value ? "TRUE()" : "FALSE()"; + } + return content.value ? "TRUE" : "FALSE"; + } + + [[nodiscard]] std::string write_content(const ErrorLiteral &content, + const Node &) const { + return std::string(spelling_of(content.type)); + } + + [[nodiscard]] std::string write_content(const Missing &, const Node &) const { + return ""; + } + + [[nodiscard]] std::string write_content(const CellReference &content, + const Node &) const { + if (m_syntax == Syntax::opendocument) { + return "[" + locator(content, true) + "]"; + } + return locator(content, true); + } + + [[nodiscard]] std::string write_content(const RangeReference &content, + const Node &) const { + if (m_syntax == Syntax::opendocument) { + return "[" + locator(content.from, true) + ":" + + locator(content.to, true) + "]"; + } + return locator(content.from, true) + ":" + locator(content.to, false); + } + + [[nodiscard]] std::string write_content(const NameReference &content, + const Node &) const { + if (m_syntax == Syntax::opendocument) { + return "$$" + (needs_quotes(content.name) ? quote(content.name, '\'') + : content.name); + } + return qualifier(content.document, content.sheet, false) + content.name; + } + + [[nodiscard]] std::string write_content(const FunctionCall &content, + const Node &node) const { + std::string result = content.name + "("; + for (std::size_t i = 0; i < node.children.size(); ++i) { + if (i > 0) { + result += separator(); + } + result += write(node.children[i]); + } + return result + ")"; + } + + [[nodiscard]] std::string write_content(const UnaryOperation &content, + const Node &node) const { + const Node &operand = node.children.front(); + if (content.op == UnaryOperator::percent) { + return nested(operand, Precedence::percent, false) + "%"; + } + return (content.op == UnaryOperator::minus ? "-" : "+") + + nested(operand, Precedence::sign, false); + } + + [[nodiscard]] std::string write_content(const BinaryOperation &content, + const Node &node) const { + const Precedence own = precedence_of(content.op); + // ooxml spells a union with the comma it also separates arguments with, + // so the parenthesis is what tells the two apart + if (m_syntax == Syntax::ooxml && content.op == BinaryOperator::unite) { + return "(" + write(node.children.front()) + "," + + write(node.children.back()) + ")"; + } + const std::string_view spelling = + m_syntax == Syntax::ooxml && content.op == BinaryOperator::intersect + ? " " + : spelling_of(content.op); + return nested(node.children.front(), own, false) + std::string(spelling) + + nested(node.children.back(), own, true); + } + + [[nodiscard]] std::string write_content(const ArrayLiteral &content, + const Node &node) const { + const char row_separator = m_syntax == Syntax::opendocument ? '|' : ';'; + std::string result = "{"; + for (std::size_t i = 0; i < node.children.size(); ++i) { + if (i > 0) { + result += content.columns != 0 && i % content.columns == 0 + ? row_separator + : separator(); + } + result += write(node.children[i]); + } + return result + "}"; + } + + /// The document and the sheet in front of a reference or a name. + [[nodiscard]] std::string + qualifier(const std::optional &document, + const std::optional &sheet, + const bool sheet_absolute) const { + if (m_syntax == Syntax::opendocument) { + std::string result; + if (document.has_value()) { + result += quote(*document, '\'') + "#"; + } + if (sheet_absolute) { + result += "$"; + } + if (sheet.has_value()) { + result += needs_quotes(*sheet) ? quote(*sheet, '\'') : *sheet; + } + return result; + } + std::string result; + if (document.has_value()) { + result += "[" + *document + "]"; + } + if (sheet.has_value()) { + result += (needs_quotes(*sheet) ? quote(*sheet, '\'') : *sheet) + "!"; + } else if (document.has_value()) { + // `[1]!Total` names the other workbook itself, with no sheet between + result += "!"; + } + return result; + } + + /// One corner of a reference. The second corner of an ooxml range drops a + /// qualifier the first already states. + [[nodiscard]] std::string locator(const CellReference &cell, + const bool qualified) const { + std::string coordinates = + spell_coordinate(cell.column, true) + spell_coordinate(cell.row, false); + if (m_syntax == Syntax::opendocument) { + return qualifier(cell.document, cell.sheet, cell.sheet_absolute) + "." + + coordinates; + } + if (!qualified) { + return coordinates; + } + return qualifier(cell.document, cell.sheet, cell.sheet_absolute) + + coordinates; + } +}; + +} // namespace + +} // namespace odr::internal::formula + +namespace odr::internal { + +std::string formula::to_string(const formula::Node &node, + const formula::Syntax syntax) { + return formula::Writer(syntax).write(node); +} + +} // namespace odr::internal diff --git a/src/odr/internal/formula/formula_writer.hpp b/src/odr/internal/formula/formula_writer.hpp new file mode 100644 index 000000000..059da982b --- /dev/null +++ b/src/odr/internal/formula/formula_writer.hpp @@ -0,0 +1,14 @@ +#pragma once + +#include +#include + +#include + +namespace odr::internal::formula { + +/// Writes @p node back in @p syntax, without the `of:=` prefix or the leading +/// `=`. A parenthesis the precedence already states is dropped. +[[nodiscard]] std::string to_string(const Node &node, Syntax syntax); + +} // namespace odr::internal::formula diff --git a/src/odr/internal/ooxml/spreadsheet/AGENTS.md b/src/odr/internal/ooxml/spreadsheet/AGENTS.md index befc12e6c..c0e04dda2 100644 --- a/src/odr/internal/ooxml/spreadsheet/AGENTS.md +++ b/src/odr/internal/ooxml/spreadsheet/AGENTS.md @@ -34,9 +34,14 @@ never evaluated**. `sheet_cell_value_type` derives number-vs-string from `c/@t` (default "n" → `float_number` when a `` exists; dates/booleans/errors report `string`). `sheet_cell_value` adds what that leaves out — `` parsed as a number where the type is one, and `` as its own string. A shared -formula writes its expression on the group's master, so a member's formula is -**set and empty** rather than absent. Merged ranges from `mergeCells` land in -the `SheetCell` side map as anchor `span` + `is_covered` flags at parse time. +formula (18.3.1.40) writes its expression on the group's master alone, so a +member is read **through the master its `si` names** — the parser collects +them per sheet, and `sheet_cell_value` moves the master's expression by the +offset between the two cells (`internal/formula`), `#REF!` where that leaves +the grid. A master that does not parse is handed out as it stands; a member +whose `si` names none stays **set and empty**. Merged ranges from `mergeCells` +land in the `SheetCell` side map as anchor `span` + `is_covered` flags at parse +time. **Style resolution: styles.xml index vectors.** `StyleRegistry` loads positional `fonts`/`fills`/`borders`/`cellStyleXfs`/`cellXfs`. A cell's `s` attribute @@ -87,7 +92,8 @@ reader is asked to. Coverage is in [`README.md`](README.md). Foundational gaps, roughly by value: 1. **Formulas & rich value types.** `` is never evaluated (the cached `` - shows); dates, booleans, and errors are typed as plain strings. + shows); dates, booleans, and errors are typed as plain strings. An array + formula's members (`t="array"`) carry no `` at all, so they report none. 2. **Content-range detection.** `sheet_content` ignores the requested range and returns the full `` — no trim to the populated range. 3. **No named/master cell-style inheritance** (`cellStyleXfs` loaded but unused); diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp index 7bfc930d3..556523132 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp @@ -8,6 +8,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -15,6 +18,8 @@ #include #include +#include +#include #include #include #include @@ -363,11 +368,48 @@ class ElementAdapter final : public AdapterBase { } } if (const pugi::xml_node formula = node.child("f")) { - result = result.with_formula(formula.text().get()); + result = result.with_formula(formula_expression(element_id, formula)); } return result; } + /// [ECMA-376] 18.3.1.40: a member of a shared group states its `si` alone, + /// and reads the master's expression moved by the offset between the two. + [[nodiscard]] std::string + formula_expression(const ElementIdentifier element_id, + const pugi::xml_node formula) const { + std::string text = formula.text().get(); + if (!text.empty() || + std::string_view(formula.attribute("t").value()) != "shared") { + return text; + } + const ElementIdentifier sheet_id = + m_registry->element_at(element_id).parent_id; + if (sheet_id == null_element_id) { + return text; + } + const ElementRegistry::Sheet &sheet = + m_registry->sheet_element_at(sheet_id); + const auto master = + sheet.shared_formulas.find(formula.attribute("si").value()); + if (master == sheet.shared_formulas.end()) { + return text; + } + std::optional node = + formula::parse(master->second.expression, formula::Syntax::ooxml); + if (!node.has_value()) { + return master->second.expression; + } + const TablePosition position = + m_registry->sheet_cell_element_at(element_id).position; + formula::shift(*node, + static_cast(position.column) - + master->second.position.column, + static_cast(position.row) - + master->second.position.row); + return formula::to_string(*node, formula::Syntax::ooxml); + } + [[nodiscard]] TextStyle line_break_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).text_style; diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_element_registry.hpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_element_registry.hpp index 7126b2737..5833d5a2b 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_element_registry.hpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_element_registry.hpp @@ -49,6 +49,12 @@ class ElementRegistry final ElementIdentifier element_id{null_element_id}; }; + /// The master of a shared group, which every member of it reads. + struct SharedFormula final { + TablePosition position; + std::string expression; + }; + /// From the workbook's ``; the worksheet part carries none. std::string name; @@ -58,6 +64,8 @@ class ElementRegistry final std::unordered_map rows; std::unordered_map cells; + std::unordered_map shared_formulas; + ElementIdentifier first_shape_id{null_element_id}; ElementIdentifier last_shape_id{null_element_id}; diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.cpp index 9bc1bcac1..58cb52dc6 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.cpp @@ -135,6 +135,15 @@ parse_sheet_element(ElementRegistry ®istry, const ParseContext &context, sheet.register_cell(position.column, position.row, cell_node, cell_id); parse_sheet_cell_children(registry, context, cell_id, cell_node); + // [ECMA-376] 18.3.1.40: only the master spells the expression + if (const pugi::xml_node formula_node = cell_node.child("f"); + std::string_view(formula_node.attribute("t").value()) == "shared" && + !std::string_view(formula_node.text().get()).empty()) { + sheet.shared_formulas.emplace(formula_node.attribute("si").value(), + ElementRegistry::Sheet::SharedFormula{ + position, formula_node.text().get()}); + } + used.rows = std::max(used.rows, position.row + 1); used.columns = std::max(used.columns, position.column + 1); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index b9f61c053..5657df2b4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -62,6 +62,7 @@ add_executable(odr_test "src/internal/csv/csv_file_test.cpp" "src/internal/encoding/text_encoding_test.cpp" "src/internal/formula/formula_parser_test.cpp" + "src/internal/formula/formula_writer_test.cpp" "src/internal/svg/svg_file_test.cpp" "src/internal/svg/svg_writer_test.cpp" "src/internal/xml/xml_file_test.cpp" diff --git a/test/src/internal/formula/formula_parser_test.cpp b/test/src/internal/formula/formula_parser_test.cpp index faaeeb365..7dd1e81fb 100644 --- a/test/src/internal/formula/formula_parser_test.cpp +++ b/test/src/internal/formula/formula_parser_test.cpp @@ -288,6 +288,7 @@ TEST(FormulaParser, what_does_not_parse_is_nothing) { EXPECT_FALSE(ooxml("\"open").has_value()); EXPECT_FALSE(ooxml("").has_value()); EXPECT_FALSE(ooxml("A1 B1").has_value()); + EXPECT_FALSE(ooxml("1e999").has_value()); } /// Rows are 1-based, and both axes stop at what an index holds. diff --git a/test/src/internal/formula/formula_writer_test.cpp b/test/src/internal/formula/formula_writer_test.cpp new file mode 100644 index 000000000..94d172728 --- /dev/null +++ b/test/src/internal/formula/formula_writer_test.cpp @@ -0,0 +1,94 @@ +#include +#include +#include + +#include + +#include +#include +#include + +using namespace odr::internal::formula; + +namespace { + +std::string round_trip(const std::string &text, const Syntax syntax) { + const std::optional node = parse(text, syntax); + return node.has_value() ? to_string(*node, syntax) : ""; +} + +std::string odf(const std::string &text) { + return round_trip(text, Syntax::opendocument); +} + +std::string ooxml(const std::string &text) { + return round_trip(text, Syntax::ooxml); +} + +std::string moved(const std::string &text, const std::int64_t columns, + const std::int64_t rows) { + std::optional node = parse(text, Syntax::ooxml); + if (!node.has_value()) { + return ""; + } + shift(*node, columns, rows); + return to_string(*node, Syntax::ooxml); +} + +} // namespace + +TEST(FormulaWriter, an_ooxml_expression_is_written_as_it_was_read) { + EXPECT_EQ(ooxml("SUM(A1:B2)"), "SUM(A1:B2)"); + EXPECT_EQ(ooxml("IF(A1>0,\"a\",\"b\")"), "IF(A1>0,\"a\",\"b\")"); + EXPECT_EQ(ooxml("$A$1+1"), "$A$1+1"); + EXPECT_EQ(ooxml("'My Sheet'!A1:B2"), "'My Sheet'!A1:B2"); + EXPECT_EQ(ooxml("[1]Sheet1!A1"), "[1]Sheet1!A1"); + EXPECT_EQ(ooxml("A1&\"x\""), "A1&\"x\""); + EXPECT_EQ(ooxml("-3%"), "-3%"); + EXPECT_EQ(ooxml("{1,2;3,4}"), "{1,2;3,4}"); + EXPECT_EQ(ooxml("IF(A1,,B1)"), "IF(A1,,B1)"); + EXPECT_EQ(ooxml("#DIV/0!"), "#DIV/0!"); + EXPECT_EQ(ooxml("TRUE"), "TRUE"); + EXPECT_EQ(ooxml("A:A"), "A:A"); + EXPECT_EQ(ooxml("SUM((A1:A2,B1:B2))"), "SUM((A1:A2,B1:B2))"); + EXPECT_EQ(ooxml("[1]!Total"), "[1]!Total"); +} + +TEST(FormulaWriter, an_opendocument_expression_is_written_as_it_was_read) { + EXPECT_EQ(odf("of:=SUM([.A1:.B2])"), "SUM([.A1:.B2])"); + EXPECT_EQ(odf("of:=[$'My Sheet'.$A$1]"), "[$'My Sheet'.$A$1]"); + EXPECT_EQ(odf("of:=IF([.A1]>0;1;2)"), "IF([.A1]>0;1;2)"); + EXPECT_EQ(odf("of:={1;2|3;4}"), "{1;2|3;4}"); + EXPECT_EQ(odf("of:=$$'Total Sales'"), "$$'Total Sales'"); + EXPECT_EQ(odf("of:=[.A1:Sheet2.B2]"), "[.A1:Sheet2.B2]"); + EXPECT_EQ(odf("of:=SUM([.A1:.A2]~[.B1:.B2])"), "SUM([.A1:.A2]~[.B1:.B2])"); + EXPECT_EQ(odf("of:=['file:///x.ods'#$Sheet1.A1]"), + "['file:///x.ods'#$Sheet1.A1]"); + EXPECT_EQ(odf("of:=TRUE"), "TRUE()"); +} + +TEST(FormulaWriter, a_parenthesis_is_written_only_where_it_is_needed) { + EXPECT_EQ(ooxml("(1+2)*3"), "(1+2)*3"); + EXPECT_EQ(ooxml("1+(2*3)"), "1+2*3"); + EXPECT_EQ(ooxml("1-(2-3)"), "1-(2-3)"); + EXPECT_EQ(ooxml("(1-2)-3"), "1-2-3"); + EXPECT_EQ(ooxml("-(1+2)"), "-(1+2)"); +} + +TEST(FormulaWriter, a_relative_reference_moves_and_an_absolute_one_does_not) { + EXPECT_EQ(moved("A1+B1", 0, 1), "A2+B2"); + EXPECT_EQ(moved("$A$1+B1", 0, 1), "$A$1+B2"); + EXPECT_EQ(moved("A$1+$A1", 1, 1), "B$1+$A2"); + EXPECT_EQ(moved("SUM(A1:A5)", 2, 0), "SUM(C1:C5)"); + EXPECT_EQ(moved("Sheet2!A1", 0, 3), "Sheet2!A4"); +} + +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!)"); +} + +TEST(FormulaWriter, what_a_shift_does_not_name_is_left_alone) { + EXPECT_EQ(moved("SUM(Total)+1", 3, 3), "SUM(Total)+1"); + EXPECT_EQ(moved("\"A1\"", 3, 3), "\"A1\""); +} diff --git a/test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp b/test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp index 51976588f..6e30867e7 100644 --- a/test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp +++ b/test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp @@ -5,6 +5,7 @@ #include +#include #include using namespace odr; @@ -21,6 +22,12 @@ std::string text_of(const std::string &sheet_data) { return value_of(sheet_data).text(); } +std::string formula_at(const std::string &sheet_data, + const std::uint32_t column, const std::uint32_t row) { + const Document document = decode(workbook(sheet_data)); + return first_sheet(document).cell(column, row).value().formula(); +} + } // namespace /// ECMA-376 18.3.1.4: an inline string holds its text under `is`, one level @@ -70,7 +77,7 @@ TEST(OoxmlSpreadsheetValue, a_formula_cell_states_both_formula_and_result) { /// Set-and-empty says the member computes; unset would claim it does not. TEST(OoxmlSpreadsheetValue, - a_shared_formula_member_holds_a_formula_it_cannot_spell) { + a_shared_formula_member_without_its_master_spells_nothing) { const CellValue value = value_of( R"(8)"); @@ -78,6 +85,37 @@ TEST(OoxmlSpreadsheetValue, EXPECT_TRUE(value.formula().empty()); } +/// ECMA-376 18.3.1.40: the master spells the expression for the whole group. +TEST(OoxmlSpreadsheetValue, a_shared_formula_member_reads_its_master_moved) { + const std::string data = + R"(A1+$B$1)" + R"(3)" + R"(7)" + R"(9)"; + + EXPECT_EQ(formula_at(data, 2, 0), "A1+$B$1"); + EXPECT_EQ(formula_at(data, 2, 1), "A2+$B$1"); + EXPECT_EQ(formula_at(data, 2, 2), "A3+$B$1"); +} + +TEST(OoxmlSpreadsheetValue, a_shared_formula_member_can_lose_its_reference) { + const std::string data = + R"(A1)" + R"(3)" + R"(7)"; + + EXPECT_EQ(formula_at(data, 2, 0), "#REF!"); +} + +TEST(OoxmlSpreadsheetValue, a_shared_formula_that_does_not_parse_is_kept) { + const std::string data = + R"(A1 +)" + R"(3)" + R"(7)"; + + EXPECT_EQ(formula_at(data, 2, 1), "A1 +"); +} + /// `` holds `1`, not a quantity, and `c/@t="b"` types the cell a string. TEST(OoxmlSpreadsheetValue, a_boolean_cell_states_no_number) { const CellValue value =