diff --git a/AGENTS.md b/AGENTS.md index 0ae78e667..1cc5120d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,7 +84,7 @@ producer's layout recorded — odf's `text:soft-page-break` — are not parsed. |------|------| | `src/odr/*.hpp` | **Public API**: `file`, `document`, `document_element`, `html`, `style`, `quantity` (`Measure`), `odr`. | | `src/odr/internal/abstract/` | Core interfaces: `File`/`DecodedFile`, `Document` + `ElementAdapter`, `Filesystem`, `Archive`, `HtmlService`. | -| `src/odr/internal/common/` | Reusable impls: `Path`/`AbsPath`, base `Document`, the shared `ElementRegistry` + `ElementAdapter`, filesystem, `style`, table cursor/range, `TextCursor`, temp files. | +| `src/odr/internal/common/` | Reusable impls: `Path`/`AbsPath`, base `Document`, the shared `ElementRegistry` + `ElementAdapter`, filesystem, `style`, table cursor/range, `TextCursor`, `SheetDependencies`, temp files. | | `src/odr/internal/util/` | Helpers: `byte_stream_util`, `string_util`, `stream_util`, `document_util`. | | `src/odr/internal/magic.*`, `open_strategy.*` | File-type detection + open/dispatch. | | `src/odr/internal/file_type_table.*` | **The** per-`FileType` table: extensions, MIME types, category, document type, `FileTypeCapabilities`. Every public lookup in `odr.hpp` is a thin forward into it — extend the table, not the lookups. | diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f0dac0b4..2738481c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- `Document::dependents(position)` answers which cells' formulas read a + position, directly or through another, and `unresolved_formulas()` those + whose references could not all be read. A position is the new + `SheetPosition`: the sheet by its place among the document's sheets. + - 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. diff --git a/CMakeLists.txt b/CMakeLists.txt index 609af5c99..6e8b48815 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -149,6 +149,7 @@ set(ODR_SOURCE_FILES "src/odr/logger.cpp" "src/odr/odr.cpp" "src/odr/quantity.cpp" + "src/odr/sheet_position.cpp" "src/odr/style.cpp" "src/odr/table_dimension.cpp" "src/odr/table_position.cpp" @@ -176,6 +177,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/common/media_file.cpp" "src/odr/internal/common/path.cpp" "src/odr/internal/common/random.cpp" + "src/odr/internal/common/sheet_dependencies.cpp" "src/odr/internal/common/style.cpp" "src/odr/internal/common/table_cursor.cpp" "src/odr/internal/common/table_range.cpp" @@ -189,6 +191,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/csv/csv_util.cpp" "src/odr/internal/formula/formula_ast.cpp" + "src/odr/internal/formula/formula_dependencies.cpp" "src/odr/internal/formula/formula_parser.cpp" "src/odr/internal/formula/formula_writer.cpp" diff --git a/docs/design/spreadsheet-editing.md b/docs/design/spreadsheet-editing.md index 3b7059b51..ba909bb25 100644 --- a/docs/design/spreadsheet-editing.md +++ b/docs/design/spreadsheet-editing.md @@ -473,8 +473,22 @@ Each step ships on its own. "Both" means `.ods` and `.xlsx`. 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". +2. **Landed.** Reference extraction → `internal::SheetDependencies`: every + sheet walked, every formula parsed, each reference resolved to the + rectangle of a sheet it reads. `Document::dependents(position)` answers + which cells read it, directly or through another formula. The graph is + built once off the decoded document and kept, because writing a formula is + refused until step 4. + + A formula that names something no position can be read out of is in + `Document::unresolved_formulas()` instead: it may read anything, and the + graph cannot say what. A reference into another document is neither — no + edit here reaches it. + + The walk goes through `SheetAdapter::sheet_visit_formulas`, which hands out + the cells the file *spells* rather than the positions they cover: a + repeated ODS row of 1024 columns over 1048576 rows is a handful of nodes + and a billion positions, and only the first is walked. 3. View: a commit marks dependents stale (a class, the host is told); the locked formula cell exposes its text (`data-odr-formula`, formula cells only) so a formula bar or a tooltip can show it. diff --git a/src/odr/document.cpp b/src/odr/document.cpp index 05e81a4ef..d387edddb 100644 --- a/src/odr/document.cpp +++ b/src/odr/document.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -17,6 +18,7 @@ #include #include #include +#include #include @@ -375,6 +377,20 @@ Paragraph Document::insert_paragraph_after(const Paragraph ¶graph) const { return {adapter, identifier, adapter->paragraph_adapter(identifier)}; } +std::vector +Document::dependents(const SheetPosition &position) const { + return dependents(std::vector{position}); +} + +std::vector +Document::dependents(const std::vector &positions) const { + return m_impl->sheet_dependencies().dependents(positions); +} + +std::vector Document::unresolved_formulas() const { + return m_impl->sheet_dependencies().unresolved(); +} + Filesystem Document::as_filesystem() const { if (std::shared_ptr files = m_impl->as_filesystem()) { diff --git a/src/odr/document.hpp b/src/odr/document.hpp index 90c895013..63a3e7d64 100644 --- a/src/odr/document.hpp +++ b/src/odr/document.hpp @@ -3,10 +3,13 @@ #include #include +#include + #include #include #include #include +#include namespace odr::internal::abstract { class Document; @@ -104,6 +107,25 @@ class Document final { /// @} + /// @name Formulas + /// The graph is built the first time one of these is asked and kept. + /// @{ + + /// The cells whose formula reads @p position, directly or through another + /// formula. Sorted by sheet and then in reading order, each named once. + [[nodiscard]] std::vector + dependents(const SheetPosition &position) const; + /// The same for a whole batch of edited positions, which costs one walk + /// rather than one per position. + [[nodiscard]] std::vector + dependents(const std::vector &positions) const; + + /// The cells holding a formula whose references could not all be read: it + /// may read any position, so a caller that must be right assumes it does. + [[nodiscard]] std::vector unresolved_formulas() const; + + /// @} + /// The files the document is packaged from; empty for a document that is /// one file. [[nodiscard]] Filesystem as_filesystem() const; diff --git a/src/odr/internal/abstract/document.hpp b/src/odr/internal/abstract/document.hpp index c3ec31d95..91dc00f8d 100644 --- a/src/odr/internal/abstract/document.hpp +++ b/src/odr/internal/abstract/document.hpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -30,6 +31,10 @@ struct ParagraphStyle; struct GraphicStyle; } // namespace odr +namespace odr::internal { +class SheetDependencies; +} // namespace odr::internal + namespace odr::internal::abstract { class ReadableFilesystem; class ElementAdapter; @@ -76,6 +81,10 @@ class Document { [[nodiscard]] virtual ElementIdentifier root_element() const = 0; [[nodiscard]] virtual const ElementAdapter *element_adapter() const = 0; + + /// Built on the first question and kept: writing a formula is refused, so + /// nothing a write does changes it. + [[nodiscard]] virtual const SheetDependencies &sheet_dependencies() const = 0; }; class ElementAdapter { @@ -245,6 +254,11 @@ class PageAdapter { page_name(ElementIdentifier element_id) const = 0; }; +/// What a formula cell states: the position the file states it at, and the +/// expression in the engine's own syntax. +using SheetFormulaVisitor = std::function; + class SheetAdapter { public: virtual ~SheetAdapter() = default; @@ -268,6 +282,13 @@ class SheetAdapter { [[nodiscard]] virtual ElementIdentifier sheet_first_shape(ElementIdentifier element_id) const = 0; + /// Calls @p visitor for every cell of the sheet stating a formula, once per + /// cell the file spells — a repeated run at its first position, so the grid + /// a repeat stands for is never walked. Visits none by default. + virtual void sheet_visit_formulas( + [[maybe_unused]] const ElementIdentifier element_id, + [[maybe_unused]] const SheetFormulaVisitor &visitor) const {} + /// Writes @p value into the cell at (@p column, @p row). A value stating /// nothing clears it. /// @throws UnsupportedOperation where the engine cannot write, the cell is diff --git a/src/odr/internal/common/document.cpp b/src/odr/internal/common/document.cpp index b7ee6c672..c058c6c6d 100644 --- a/src/odr/internal/common/document.cpp +++ b/src/odr/internal/common/document.cpp @@ -3,6 +3,7 @@ #include #include +#include namespace odr::internal { @@ -45,6 +46,14 @@ const abstract::ElementAdapter *Document::element_adapter() const { return m_element_adapter.get(); } +const SheetDependencies &Document::sheet_dependencies() const { + if (!m_sheet_dependencies) { + m_sheet_dependencies = + std::make_unique(SheetDependencies::of(*this)); + } + return *m_sheet_dependencies; +} + bool Document::is_decrypted() const noexcept { return m_encryption_state == EncryptionState::decrypted; } diff --git a/src/odr/internal/common/document.hpp b/src/odr/internal/common/document.hpp index 94300d35d..89d9f3655 100644 --- a/src/odr/internal/common/document.hpp +++ b/src/odr/internal/common/document.hpp @@ -13,6 +13,8 @@ class ReadableFilesystem; namespace odr::internal { +class SheetDependencies; + class Document : public abstract::Document { public: Document(FileType file_type, DocumentType document_type, @@ -37,6 +39,8 @@ class Document : public abstract::Document { [[nodiscard]] const abstract::ElementAdapter * element_adapter() const override; + [[nodiscard]] const SheetDependencies &sheet_dependencies() const final; + /// Decoded from a package that was password-encrypted. `save` has no /// encryption to put back, so a savable engine refuses one. [[nodiscard]] bool is_decrypted() const noexcept; @@ -50,6 +54,9 @@ class Document : public abstract::Document { ElementIdentifier m_root_element{null_element_id}; std::unique_ptr m_element_adapter; + +private: + mutable std::unique_ptr m_sheet_dependencies; }; } // namespace odr::internal diff --git a/src/odr/internal/common/sheet_dependencies.cpp b/src/odr/internal/common/sheet_dependencies.cpp new file mode 100644 index 000000000..ed75cf845 --- /dev/null +++ b/src/odr/internal/common/sheet_dependencies.cpp @@ -0,0 +1,165 @@ +#include + +#include + +#include +#include +#include +#include + +#include +#include +#include + +namespace odr::internal { + +namespace { + +/// The syntax the engine behind @p file_type writes a formula in. Nothing +/// where it states none at all. +std::optional syntax_of(const FileType file_type) { + switch (file_type) { + case FileType::opendocument_spreadsheet: + return formula::Syntax::opendocument; + case FileType::office_open_xml_workbook: + return formula::Syntax::ooxml; + default: + return {}; + } +} + +} // namespace + +bool SheetDependencies::Read::contains(const SheetPosition &position) const { + return position.sheet == sheet && range.contains(position.cell); +} + +SheetDependencies SheetDependencies::of(const abstract::Document &document) { + SheetDependencies result; + + const std::optional syntax = syntax_of(document.file_type()); + const abstract::ElementAdapter *adapter = document.element_adapter(); + if (!syntax.has_value() || adapter == nullptr) { + return result; + } + + std::vector sheets; + std::unordered_map by_name; + for (ElementIdentifier id = + adapter->element_first_child(document.root_element()); + id != null_element_id; id = adapter->element_next_sibling(id)) { + if (adapter->element_type(id) != ElementType::sheet) { + continue; + } + // a formula names a sheet without case, as the applications writing one do + by_name.emplace( + util::string::to_lower(adapter->sheet_adapter(id)->sheet_name(id)), + static_cast(sheets.size())); + sheets.push_back(id); + } + + for (std::uint32_t index = 0; index < sheets.size(); ++index) { + const ElementIdentifier sheet_id = sheets[index]; + adapter->sheet_adapter(sheet_id)->sheet_visit_formulas( + sheet_id, [&](const std::uint32_t column, const std::uint32_t row, + const std::string &formula) { + if (!formula.empty()) { + result.add_formula_(SheetPosition(index, column, row), formula, + *syntax, by_name); + } + }); + } + + // a sheet hands its cells out in whatever order it holds them + std::ranges::sort(result.m_unresolved); + return result; +} + +void SheetDependencies::add_formula_( + const SheetPosition &cell, const std::string &expression, + const formula::Syntax syntax, + const std::unordered_map &by_name) { + const std::optional node = formula::parse(expression, syntax); + if (!node.has_value()) { + m_unresolved.push_back(cell); + return; + } + + const formula::References references = formula::references(*node); + Entry entry; + entry.cell = cell; + bool complete = references.complete; + for (const formula::Extent &extent : references.extents) { + if (extent.document.has_value()) { + continue; // another file, which nothing here opens and no edit reaches + } + std::uint32_t sheet = cell.sheet; + if (extent.sheet.has_value()) { + const auto named = by_name.find(util::string::to_lower(*extent.sheet)); + if (named == by_name.end()) { + complete = false; // a sheet this document has none of, or a span + continue; + } + sheet = named->second; + } + entry.reads.push_back(Read{sheet, extent.range}); + } + + if (!complete) { + m_unresolved.push_back(cell); + } + if (entry.reads.empty()) { + return; + } + + const std::size_t at = m_entries.size(); + for (const Read &read : entry.reads) { + std::vector &bucket = m_by_sheet[read.sheet]; + if (bucket.empty() || bucket.back() != at) { + bucket.push_back(at); + } + } + m_entries.push_back(std::move(entry)); +} + +std::vector SheetDependencies::dependents( + const std::vector &positions) const { + std::vector reported(m_entries.size(), false); + std::vector frontier = positions; + std::vector result; + + while (!frontier.empty()) { + std::vector next; + for (const SheetPosition &position : frontier) { + const auto bucket = m_by_sheet.find(position.sheet); + if (bucket == m_by_sheet.end()) { + continue; + } + for (const std::size_t at : bucket->second) { + if (reported[at]) { + continue; + } + const Entry &entry = m_entries[at]; + if (!std::ranges::any_of(entry.reads, [&](const Read &read) { + return read.contains(position); + })) { + continue; + } + reported[at] = true; + result.push_back(entry.cell); + next.push_back(entry.cell); + } + } + frontier = std::move(next); + } + + std::ranges::sort(result); + return result; +} + +const std::vector & +SheetDependencies::unresolved() const noexcept { + return m_unresolved; +} + +} // namespace odr::internal diff --git a/src/odr/internal/common/sheet_dependencies.hpp b/src/odr/internal/common/sheet_dependencies.hpp new file mode 100644 index 000000000..60ee6eab1 --- /dev/null +++ b/src/odr/internal/common/sheet_dependencies.hpp @@ -0,0 +1,67 @@ +#pragma once + +#include + +#include +#include + +#include +#include +#include +#include +#include + +namespace odr::internal::abstract { +class Document; +} // namespace odr::internal::abstract + +namespace odr::internal { + +/// Which cells a formula reads, as the reverse map an edit asks: the cells +/// that go stale when a position changes. Built once off a decoded document. +class SheetDependencies final { +public: + SheetDependencies() = default; + + /// Walks every sheet of @p document and parses every formula it states. + /// Empty where the engine states none (`.xls`, `.numbers`, a text file). + [[nodiscard]] static SheetDependencies of(const abstract::Document &document); + + /// The cells whose formula reads one of @p positions, directly or through + /// another formula. Sorted, and each named once. + [[nodiscard]] std::vector + dependents(const std::vector &positions) const; + + /// The cells holding a formula whose references could not all be read — one + /// that does not parse, names a name, or reaches over several sheets. + [[nodiscard]] const std::vector &unresolved() const noexcept; + +private: + /// The rectangle of one sheet a formula reads. + struct Read final { + std::uint32_t sheet{0}; + TableRange range{}; + + [[nodiscard]] bool contains(const SheetPosition &position) const; + }; + + struct Entry final { + SheetPosition cell; + std::vector reads; + }; + + /// Reads @p expression and records what it names, resolving a sheet name + /// through @p by_name. + void + add_formula_(const SheetPosition &cell, const std::string &expression, + formula::Syntax syntax, + const std::unordered_map &by_name); + + std::vector m_entries; + /// The entries reading a sheet, by that sheet — so a position is asked of + /// the formulas that could name it rather than of all of them. + std::unordered_map> m_by_sheet; + std::vector m_unresolved; +}; + +} // namespace odr::internal diff --git a/src/odr/internal/common/table_range.hpp b/src/odr/internal/common/table_range.hpp index 13d95faca..8f3d5e968 100644 --- a/src/odr/internal/common/table_range.hpp +++ b/src/odr/internal/common/table_range.hpp @@ -19,6 +19,8 @@ class TableRange final { /// Closed: @ref to is the last position of the range, and it is contained. [[nodiscard]] bool contains(const TablePosition &position) const noexcept; + friend bool operator==(const TableRange &, const TableRange &) = default; + private: TablePosition m_from; TablePosition m_to; diff --git a/src/odr/internal/formula/formula_dependencies.cpp b/src/odr/internal/formula/formula_dependencies.cpp new file mode 100644 index 000000000..b3b2fcdf2 --- /dev/null +++ b/src/odr/internal/formula/formula_dependencies.cpp @@ -0,0 +1,78 @@ +#include + +#include +#include +#include + +namespace odr::internal::formula { + +namespace { + +constexpr std::uint32_t index_limit = std::numeric_limits::max(); + +/// The rectangle two corners span. An axis neither corner states reaches the +/// whole sheet; the second corner's unstated sheet is the first's. +Extent extent_of(const CellReference &from, const CellReference &to) { + const auto index = [](const std::optional &axis, + const std::uint32_t unstated) { + return axis.has_value() ? axis->index : unstated; + }; + const std::uint32_t from_column = index(from.column, 0); + const std::uint32_t to_column = index(to.column, index_limit); + const std::uint32_t from_row = index(from.row, 0); + const std::uint32_t to_row = index(to.row, index_limit); + return Extent{ + from.document, from.sheet, + TableRange{ + {std::min(from_column, to_column), std::min(from_row, to_row)}, + {std::max(from_column, to_column), std::max(from_row, to_row)}}}; +} + +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)); + return; + } + if (const auto *range = std::get_if(&node.content)) { + into.extents.push_back(extent_of(range->from, range->to)); + return; + } + if (node.holds()) { + into.complete = false; + return; + } + // a range the file spelled as two nodes around `:` rather than as one token + if (const auto *binary = std::get_if(&node.content); + binary != nullptr && binary->op == BinaryOperator::range && + node.children.size() == 2) { + 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)); + return; + } + // a corner that is no reference: what the range spans cannot be read + into.complete = false; + } + for (const Node &child : node.children) { + collect(child, into); + } +} + +} // namespace + +} // namespace odr::internal::formula + +namespace odr::internal { + +formula::References formula::references(const Node &node) { + References result; + collect(node, result); + return result; +} + +} // namespace odr::internal diff --git a/src/odr/internal/formula/formula_dependencies.hpp b/src/odr/internal/formula/formula_dependencies.hpp new file mode 100644 index 000000000..39085a56b --- /dev/null +++ b/src/odr/internal/formula/formula_dependencies.hpp @@ -0,0 +1,34 @@ +#pragma once + +#include +#include + +#include +#include +#include + +namespace odr::internal::formula { + +/// The rectangle one reference names. An axis the reference leaves out +/// reaches the whole sheet, which is what `A:A` and `1:1` mean. +struct Extent final { + std::optional document{}; + std::optional sheet{}; + TableRange range{}; + + friend bool operator==(const Extent &, const Extent &) = default; +}; + +/// What a formula reads. +struct References final { + std::vector extents; + /// False where the formula names something no position can be read out of — + /// a name, or a reference over several sheets. It may then read anything. + bool complete{true}; +}; + +/// The cells @p node reads. A reference into another document is stated with +/// its document, and nothing here opens one. +[[nodiscard]] References references(const Node &node); + +} // namespace odr::internal::formula diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index 6e484d6ce..1c85a320d 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -417,6 +417,26 @@ class ElementAdapter final : public AdapterBase { sheet_first_shape(const ElementIdentifier element_id) const override { return m_registry->sheet_element_at(element_id).first_shape_id; } + /// The runs the parser indexed, not the grid they stand for: a repeat is + /// one entry however many positions it covers. + void sheet_visit_formulas( + const ElementIdentifier element_id, + const abstract::SheetFormulaVisitor &visitor) const override { + const ElementRegistry::Sheet &sheet = + m_registry->sheet_element_at(element_id); + std::uint32_t row = 0; + for (const ElementRegistry::Sheet::Row &run : sheet.rows) { + std::uint32_t column = 0; + for (const ElementRegistry::Sheet::Cell &cell : sheet.row_cells(run)) { + if (const pugi::xml_attribute formula = + cell.node.attribute("table:formula")) { + visitor(column, row, formula.value()); + } + column = cell.end; + } + row = run.end; + } + } /// [ODF 1.2] 19.385: the value is an attribute and the `text:p` under the /// cell shows it, so both are written or the file contradicts itself. void sheet_set_cell(const ElementIdentifier element_id, diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp index 556523132..0296dadd8 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp @@ -217,6 +217,19 @@ class ElementAdapter final : public AdapterBase { sheet_first_shape(const ElementIdentifier element_id) const override { return m_registry->sheet_element_at(element_id).first_shape_id; } + /// The cell map, not the grid `dimension` claims: a sheet states a `c` for + /// every cell it holds. + void sheet_visit_formulas( + const ElementIdentifier element_id, + const abstract::SheetFormulaVisitor &visitor) const override { + for (const auto &[position, cell] : + m_registry->sheet_element_at(element_id).cells) { + if (const pugi::xml_node formula = cell.node.child("f")) { + visitor(position.column, position.row, + formula_expression(cell.element_id, formula)); + } + } + } /// ECMA-376 18.3.1.4: a cell states its value as `v`, or as the text under /// `is` with `t="inlineStr"`. A written string goes inline - rewriting the /// shared entry would rewrite every other cell indexing it. diff --git a/src/odr/sheet_position.cpp b/src/odr/sheet_position.cpp new file mode 100644 index 000000000..1e6eacf27 --- /dev/null +++ b/src/odr/sheet_position.cpp @@ -0,0 +1,39 @@ +#include + +#include + +namespace odr { + +bool SheetPosition::operator==(const SheetPosition &rhs) const { + return sheet == rhs.sheet && cell == rhs.cell; +} + +std::strong_ordering +SheetPosition::operator<=>(const SheetPosition &rhs) const { + if (const std::strong_ordering order = sheet <=> rhs.sheet; + order != std::strong_ordering::equal) { + return order; + } + if (const std::strong_ordering order = cell.row <=> rhs.cell.row; + order != std::strong_ordering::equal) { + return order; + } + return cell.column <=> rhs.cell.column; +} + +std::string SheetPosition::to_string() const noexcept { + return std::to_string(sheet) + "!" + cell.to_string(); +} + +std::size_t SheetPosition::hash() const noexcept { + std::size_t result = 0; + internal::util::hash::hash_combine(result, sheet, cell.column, cell.row); + return result; +} + +} // namespace odr + +std::size_t std::hash::operator()( + const odr::SheetPosition &k) const noexcept { + return k.hash(); +} diff --git a/src/odr/sheet_position.hpp b/src/odr/sheet_position.hpp new file mode 100644 index 000000000..31891082a --- /dev/null +++ b/src/odr/sheet_position.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include + +#include +#include +#include +#include + +namespace odr { + +/// A cell of one of a document's sheets: the sheet by its place among them, +/// which is how an edit operation names one, and the cell by position. +struct SheetPosition final { + std::uint32_t sheet{0}; + TablePosition cell; + + constexpr SheetPosition() noexcept = default; + constexpr SheetPosition(const std::uint32_t sheet_, + const TablePosition &cell_) noexcept + : sheet{sheet_}, cell{cell_} {} + constexpr SheetPosition(const std::uint32_t sheet_, + const std::uint32_t column, + const std::uint32_t row) noexcept + : sheet{sheet_}, cell{column, row} {} + + bool operator==(const SheetPosition &rhs) const; + /// The sheet first, then the cell in reading order. + std::strong_ordering operator<=>(const SheetPosition &rhs) const; + + /// `2!B3`: the sheet's ordinal, then the cell. + [[nodiscard]] std::string to_string() const noexcept; + [[nodiscard]] std::size_t hash() const noexcept; +}; + +} // namespace odr + +template <> struct std::hash { + std::size_t operator()(const odr::SheetPosition &k) const noexcept; +}; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 5657df2b4..e9b53d91b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -40,6 +40,7 @@ add_executable(odr_test "src/odr_test.cpp" "src/pdf_annotate_test.cpp" "src/quantity_test.cpp" + "src/sheet_dependencies_test.cpp" "src/table_position_test.cpp" "src/internal/html/common_test.cpp" diff --git a/test/src/sheet_dependencies_test.cpp b/test/src/sheet_dependencies_test.cpp new file mode 100644 index 000000000..9be24c834 --- /dev/null +++ b/test/src/sheet_dependencies_test.cpp @@ -0,0 +1,201 @@ +#include +#include +#include +#include +#include + +#include +#include +#include + +#include + +#include + +#include +#include +#include + +using namespace odr; +using namespace odr::internal; + +namespace { + +/// A flat spreadsheet of @p sheets, each written as a `table:table`. +std::string flat_document(const std::string &sheets) { + return R"()" + R"()" + R"()" + + sheets + R"()"; +} + +std::string sheet(const std::string &name, const std::string &rows) { + return R"()" + rows + + R"()"; +} + +std::string row(const std::string &cells) { + return R"()" + cells + R"()"; +} + +/// A cell computing @p formula, whose cached result nothing here reads. +std::string computed(const std::string &formula) { + return R"()" + R"(0)"; +} + +std::string number(const std::string &text) { + return R"()" + text + R"()"; +} + +Document decode(const std::string &xml) { + return DecodedFile(open_strategy::open_file(std::make_shared(xml), + {}, Logger::null())) + .as_document_file() + .document(); +} + +std::vector spelled(const std::vector &positions) { + std::vector result; + result.reserve(positions.size()); + for (const SheetPosition &position : positions) { + result.push_back(position.to_string()); + } + return result; +} + +} // namespace + +TEST(SheetDependencies, a_formula_reading_a_cell_depends_on_it) { + const Document document = decode( + flat_document(sheet("s", row(number("1") + computed("of:=[.A1]*2"))))); + + EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 0, 0))), + (std::vector{"0!B1"})); + EXPECT_TRUE(document.dependents(SheetPosition(0, 1, 0)).empty()); +} + +TEST(SheetDependencies, a_dependent_of_a_dependent_is_named_too) { + const Document document = decode(flat_document(sheet( + "s", row(number("1") + computed("of:=[.A1]") + computed("of:=[.B1]"))))); + + EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 0, 0))), + (std::vector{"0!B1", "0!C1"})); +} + +TEST(SheetDependencies, a_range_covers_every_position_inside_it) { + const Document document = decode( + flat_document(sheet("s", row(number("1") + number("2") + number("3") + + computed("of:=SUM([.A1:.C1])"))))); + + EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 1, 0))), + (std::vector{"0!D1"})); + EXPECT_TRUE(document.dependents(SheetPosition(0, 4, 0)).empty()); +} + +TEST(SheetDependencies, a_formula_on_another_sheet_names_the_sheet_it_reads) { + const Document document = + decode(flat_document(sheet("Data", row(number("1"))) + + sheet("Report", row(computed("of:=[Data.A1]*2"))))); + + EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 0, 0))), + (std::vector{"1!A1"})); + EXPECT_TRUE(document.dependents(SheetPosition(1, 0, 0)).empty()); +} + +TEST(SheetDependencies, a_sheet_name_is_matched_without_case) { + const Document document = + decode(flat_document(sheet("Data", row(number("1"))) + + sheet("Report", row(computed("of:=[DATA.A1]"))))); + + EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 0, 0))), + (std::vector{"1!A1"})); +} + +TEST(SheetDependencies, a_batch_names_each_dependent_once) { + const Document document = decode(flat_document(sheet( + "s", row(number("1") + number("2") + computed("of:=[.A1]+[.B1]"))))); + + EXPECT_EQ(spelled(document.dependents( + {SheetPosition(0, 0, 0), SheetPosition(0, 1, 0)})), + (std::vector{"0!C1"})); +} + +TEST(SheetDependencies, a_formula_naming_no_position_reads_nothing) { + const Document document = + decode(flat_document(sheet("s", row(computed("of:=TODAY()"))))); + + EXPECT_TRUE(document.dependents(SheetPosition(0, 0, 0)).empty()); + EXPECT_TRUE(document.unresolved_formulas().empty()); +} + +TEST(SheetDependencies, a_formula_naming_a_named_expression_is_unresolved) { + const Document document = + decode(flat_document(sheet("s", row(computed("of:=SUM($$Sales)"))))); + + EXPECT_EQ(spelled(document.unresolved_formulas()), + (std::vector{"0!A1"})); +} + +TEST(SheetDependencies, a_formula_that_does_not_parse_is_unresolved) { + const Document document = + decode(flat_document(sheet("s", row(computed("of:=[.A1] +"))))); + + EXPECT_EQ(spelled(document.unresolved_formulas()), + (std::vector{"0!A1"})); +} + +/// An edit here reaches no other file, so the cell is not unresolved either. +TEST(SheetDependencies, a_reference_into_another_document_is_left_alone) { + const Document document = decode(flat_document( + sheet("s", row(computed("of:=['file:///x.ods'#$Sheet1.A1]"))))); + + EXPECT_TRUE(document.unresolved_formulas().empty()); + EXPECT_TRUE(document.dependents(SheetPosition(0, 0, 0)).empty()); +} + +TEST(SheetDependencies, a_document_holding_no_sheet_answers_nothing) { + const Document document = + decode(R"()" + R"()" + R"(x)" + R"()"); + + EXPECT_TRUE(document.dependents(SheetPosition(0, 0, 0)).empty()); + EXPECT_TRUE(document.unresolved_formulas().empty()); +} + +TEST(SheetDependencies, an_xlsx_formula_is_read_in_its_own_syntax) { + const Document document = odr::test::ooxml::decode(odr::test::ooxml::workbook( + R"(1)" + R"(SUM(A1:A3)1)")); + + EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 0, 2))), + (std::vector{"0!B1"})); +} + +/// `A:A` is every row of the column, so a cell far down it is still read. +TEST(SheetDependencies, a_whole_column_is_read_to_its_end) { + const Document document = odr::test::ooxml::decode(odr::test::ooxml::workbook( + R"(SUM(A:A)1)")); + + EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 0, 999))), + (std::vector{"0!B1"})); + EXPECT_TRUE(document.dependents(SheetPosition(0, 2, 0)).empty()); +} + +/// Each member reads the master's expression moved, so its own row, not A1. +TEST(SheetDependencies, every_member_of_a_shared_group_reads_its_own_row) { + const Document document = odr::test::ooxml::decode(odr::test::ooxml::workbook( + R"(1)" + R"(A11)" + R"(2)" + R"(2)")); + + EXPECT_EQ(spelled(document.dependents(SheetPosition(0, 0, 1))), + (std::vector{"0!C2"})); +}