diff --git a/CHANGELOG.md b/CHANGELOG.md index b213c935..6abbd044 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixed +- Emptying an array of tables under a table that a later header declares (e.g. `[[a.b.c]]` before `[a]` or `[a.b]`), or that a header in an earlier fragment of an out-of-order table declares (e.g. `[a.b]` before `[a]` and `[[a.b.c]]`), no longer renders invalid TOML; the `key = []` fallback is now written under that header. ([#690](https://github.com/python-poetry/tomlkit/issues/690)) - Arithmetic and `replace()` results for `Integer`, `Float`, `String`, `DateTime`, `Date` and `Time` no longer share the original item's `Trivia`, so changing the result's comment or indentation no longer rewrites the item still stored in the document. ## [0.15.2] - 2026-10-16 diff --git a/tests/test_toml_document.py b/tests/test_toml_document.py index 84b114b9..49481fa5 100644 --- a/tests/test_toml_document.py +++ b/tests/test_toml_document.py @@ -1,6 +1,7 @@ import copy import json import pickle +import sys from collections.abc import Callable from datetime import datetime @@ -1804,3 +1805,83 @@ def test_emptied_array_of_tables_is_relative_to_the_nearest_header() -> None: doc["t"]["u"]["a"].pop() assert doc.as_string() == "[t]\nu.a = []\n" + + +@pytest.mark.parametrize( + "content", + [ + # The super table is declared later by a header. + "[[a.b.c]]\nx = 1\n\n[a]\ny = 1\n", + # ... which extends the implicit parent with a dotted key. + "[[a.b.c]]\nx = 1\n[a]\nb.z = 1\n", + # The implicit parent is declared later by a header. + "[[a.b.c]]\nx = 1\n[a.b]\ny = 1\n", + # Both are declared later. + "[[a.b.c]]\nx = 1\n[a]\ny = 1\n[a.b]\nw = 1\n", + # A sibling table header comes before the super table. + "[[a.b.c]]\nx = 1\n[a.b.d]\nz = 1\n[a]\ny = 1\n", + ], +) +def test_emptied_array_of_tables_before_out_of_order_header(content) -> None: + # https://github.com/python-poetry/tomlkit/issues/690 + # The fallback belongs to the header that declares its parent, even when + # that header is in a later fragment. Writing `a.b.c = []` at the top + # declared `a` before `[a]` did. + doc = parse(content) + doc["a"]["b"]["c"].pop() + out = doc.as_string() + + assert parse(out).unwrap() == doc.unwrap() + if sys.version_info >= (3, 11): + # tomlkit's own parser accepts some of the broken output. + import tomllib + + assert tomllib.loads(out) == doc.unwrap() + + +def test_emptied_array_of_tables_is_written_under_the_out_of_order_header() -> None: + doc = parse("[[a.b.c]]\nx = 1\n\n[a]\ny = 1\n") + doc["a"]["b"]["c"].pop() + + assert doc.as_string() == "[a]\nb.c = []\ny = 1\n" + + +@pytest.mark.parametrize( + "content, path", + [ + # The parent is declared by a header in an earlier fragment of `a`. + ("[a.b]\nz = 1\n[a]\ny = 1\n[[a.b.c]]\nx = 1\n", ["a", "b", "c"]), + # ... a level deeper, under a super table in both fragments. + ("[a.b.d]\nz = 1\n[a]\ny = 1\n[[a.b.d.e]]\nx = 1\n", ["a", "b", "d", "e"]), + # ... with a header in between in the later fragment. + ( + "[a.b.d]\nz = 1\n[a]\ny = 1\n[a.b]\nw = 1\n[[a.b.d.e]]\nx = 1\n", + ["a", "b", "d", "e"], + ), + ], +) +def test_emptied_array_of_tables_after_out_of_order_header( + content: str, path: list[str] +) -> None: + # The header declaring the parent comes first, in another fragment of the + # out-of-order table, so the fallback has to be handed to it before it is + # rendered. + doc = parse(content) + aot: Any = doc + for key in path: + aot = aot[key] + aot.pop() + out = doc.as_string() + + assert parse(out).unwrap() == doc.unwrap() + if sys.version_info >= (3, 11): + import tomllib + + assert tomllib.loads(out) == doc.unwrap() + + +def test_emptied_array_of_tables_is_written_under_the_earlier_fragment() -> None: + doc = parse("[a.b]\nz = 1\n[a]\ny = 1\n[[a.b.c]]\nx = 1\n") + doc["a"]["b"]["c"].pop() + + assert doc.as_string() == "[a.b]\nc = []\nz = 1\n[a]\ny = 1\n" diff --git a/tomlkit/container.py b/tomlkit/container.py index 9265c740..eb6c09ad 100644 --- a/tomlkit/container.py +++ b/tomlkit/container.py @@ -3,6 +3,7 @@ import copy import math +from collections import Counter from collections.abc import Iterator from typing import TYPE_CHECKING from typing import Any @@ -64,8 +65,8 @@ def _emits_header(key: Key, table: Table) -> bool: def _header_less_aots( - body: list[tuple[Key | None, Item]], prefix: str = "" -) -> list[tuple[str, AoT]]: + body: list[tuple[Key | None, Item]], prefix: tuple[Key, ...] = () +) -> list[tuple[tuple[Key, ...], AoT]]: """Every emptied array of tables reachable without crossing a header. An emptied array of tables has no ``[[key]]`` header left to render, so it @@ -75,14 +76,14 @@ def _header_less_aots( with a key relative to it, and that includes the ones sitting inside super tables, which emit no header to separate them. - Returns ``(dotted key, aot)`` pairs, keyed relative to the scope ``body`` + Returns ``(key path, aot)`` pairs, keyed relative to the scope ``body`` belongs to. """ - found: list[tuple[str, AoT]] = [] + found: list[tuple[tuple[Key, ...], AoT]] = [] for k, v in body: if k is None: continue - path = f"{prefix}.{k.as_string()}" if prefix else k.as_string() + path = (*prefix, k) if _is_empty_aot(v): found.append((path, v)) elif isinstance(v, Table) and not _emits_header(k, v): @@ -90,6 +91,113 @@ def _header_less_aots( return found +def _declaring_table( + body: list[tuple[Key | None, Item]], path: tuple[Key, ...] +) -> tuple[Table, tuple[Key, ...]] | None: + """The deepest table whose header declares a parent of ``path``. + + An out-of-order table is split into fragments, so that header may be in a + different fragment than the array of tables itself. A fallback written + with a dotted key above it would declare the table a second time. + + Returns the table and ``path`` relative to it, or ``None`` if no header + in ``body`` declares a parent of ``path``. + """ + if len(path) < 2: + return None + found = None + for k, v in body: + if k != path[0] or not isinstance(v, Table): + continue + candidate = _declaring_table(v.value.body, path[1:]) + if candidate is None and _emits_header(k, v): + candidate = (v, path[1:]) + if candidate is not None and ( + found is None or len(candidate[1]) < len(found[1]) + ): + found = candidate + return found + + +def _fragment_aots( + body: list[tuple[Key | None, Item]], prefix: tuple[Key, ...] = () +) -> list[tuple[tuple[Key, ...], AoT, Table]]: + """Every emptied array of tables under a header of an out-of-order table. + + The header declaring its parent may be in an earlier fragment, which is + rendered before the header the array of tables sits under, so only the + scope holding all the fragments can hand the fallback over in time. + + Returns ``(key path, aot, table)`` triples, keyed relative to the scope + ``body`` belongs to, where ``table`` is the closest header above ``aot``. + """ + found: list[tuple[tuple[Key, ...], AoT, Table]] = [] + counts = Counter(k for k, _ in body if k is not None) + for k, v in body: + if k is None or not isinstance(v, Table): + continue + path = (*prefix, k) + if not _emits_header(k, v): + found.extend(_fragment_aots(v.value.body, path)) + elif counts[k] > 1: + found.extend(_aots_below(v, path, v)) + return found + + +def _aots_below( + table: Table, prefix: tuple[Key, ...], header: Table +) -> list[tuple[tuple[Key, ...], AoT, Table]]: + """Every emptied array of tables below ``table``, across headers. + + Returns ``(key path, aot, table)`` triples like ``_fragment_aots()``, + starting from ``prefix`` and ``header``. + """ + found: list[tuple[tuple[Key, ...], AoT, Table]] = [] + for k, v in table.value.body: + if k is None: + continue + path = (*prefix, k) + if _is_empty_aot(v): + found.append((path, v, header)) + elif isinstance(v, Table): + found.extend(_aots_below(v, path, v if _emits_header(k, v) else header)) + return found + + +def _scope_aots( + body: list[tuple[Key | None, Item]], + inherited: list[tuple[tuple[Key, ...], AoT]], + delegated: dict[int, list[tuple[tuple[Key, ...], AoT]]], +) -> list[tuple[str, AoT]]: + """The fallbacks to write at the top of the scope ``body`` belongs to. + + Those whose parent is declared by a header within the scope are left in + ``delegated``, keyed by the ``id()`` of that table, for it to write. One + handed to a header in another fragment is also keyed by its own ``id()``, + so the scope it sits in does not write it a second time. + + Returns ``(dotted key, aot)`` pairs, keyed relative to the scope. + """ + for path, aot, header in _fragment_aots(body): + declaring = _declaring_table(body, path) + if id(aot) not in delegated and declaring and declaring[0] is not header: + table, rest = declaring + delegated.setdefault(id(table), []).append((rest, aot)) + delegated[id(aot)] = [] + found: list[tuple[str, AoT]] = [] + for path, aot in [ + *inherited, + *((p, a) for p, a in _header_less_aots(body) if id(a) not in delegated), + ]: + declaring = _declaring_table(body, path) + if declaring is None: + found.append((".".join(k.as_string() for k in path), aot)) + else: + table, rest = declaring + delegated.setdefault(id(table), []).append((rest, aot)) + return found + + def _render_empty_aot(key: str, aot: AoT) -> str: """Render an emptied array of tables as an inline empty array.""" return ( @@ -709,7 +817,8 @@ def last_item(self) -> Item | None: def as_string(self) -> str: """Render as TOML string.""" s = "" - for path, aot in _header_less_aots(self._body): + delegated: dict[int, list[tuple[tuple[Key, ...], AoT]]] = {} + for path, aot in _scope_aots(self._body, [], delegated): s += _render_empty_aot(path, aot) for k, v in self._body: if k is not None: @@ -722,7 +831,7 @@ def as_string(self) -> str: and "\n" not in v.trivia.indent ): s += "\n" - s += self._render_table(k, v) + s += self._render_table(k, v, delegated=delegated) elif isinstance(v, AoT): if ( s.strip(" ") @@ -738,8 +847,16 @@ def as_string(self) -> str: return s - def _render_table(self, key: Key, table: Table, prefix: str | None = None) -> str: + def _render_table( + self, + key: Key, + table: Table, + prefix: str | None = None, + delegated: dict[int, list[tuple[tuple[Key, ...], AoT]]] | None = None, + ) -> str: cur = "" + if delegated is None: + delegated = {} if table.display_name is not None: _key = table.display_name @@ -774,7 +891,8 @@ def _render_table(self, key: Key, table: Table, prefix: str | None = None) -> st if header_emitted: # A super table opens no scope, so its fallbacks belong to -- and # were already written by -- the nearest enclosing header instead. - for path, aot in _header_less_aots(table.value.body): + inherited = delegated.pop(id(table), []) + for path, aot in _scope_aots(table.value.body, inherited, delegated): cur += _render_empty_aot(path, aot) for k, v in table.value.body: if k is not None and _is_empty_aot(v): @@ -790,11 +908,13 @@ def _render_table(self, key: Key, table: Table, prefix: str | None = None) -> st if v.is_super_table(): if k.is_dotted() and not key.is_dotted(): # Dotted key inside table - cur += self._render_table(k, v) + cur += self._render_table(k, v, delegated=delegated) else: - cur += self._render_table(k, v, prefix=_key) + cur += self._render_table( + k, v, prefix=_key, delegated=delegated + ) else: - cur += self._render_table(k, v, prefix=_key) + cur += self._render_table(k, v, prefix=_key, delegated=delegated) elif isinstance(v, AoT): if ( cur.strip(" ") @@ -838,7 +958,8 @@ def _render_aot_table(self, table: Table, prefix: str | None = None) -> str: f"{table.trivia.trail}" ) - for path, aot in _header_less_aots(table.value.body): + delegated: dict[int, list[tuple[tuple[Key, ...], AoT]]] = {} + for path, aot in _scope_aots(table.value.body, [], delegated): cur += _render_empty_aot(path, aot) for k, v in table.value.body: if k is not None and _is_empty_aot(v): @@ -848,11 +969,13 @@ def _render_aot_table(self, table: Table, prefix: str | None = None) -> str: if v.is_super_table(): if k.is_dotted(): # Dotted key inside table - cur += self._render_table(k, v) + cur += self._render_table(k, v, delegated=delegated) else: - cur += self._render_table(k, v, prefix=_key) + cur += self._render_table( + k, v, prefix=_key, delegated=delegated + ) else: - cur += self._render_table(k, v, prefix=_key) + cur += self._render_table(k, v, prefix=_key, delegated=delegated) elif isinstance(v, AoT): assert k is not None cur += self._render_aot(k, v, prefix=_key)