Fix validation of nested out-of-order tables - #572
Conversation
davidpavlovschi
left a comment
There was a problem hiding this comment.
Reviewed the validation path against the reported nested out-of-order case and the existing duplicate/redefinition checks. Iterating the raw fragments avoids treating the proxy as a non-table while still validating every fragment against the candidate. I also ran the complete suite locally on this head: 1,051 passed, 1 skipped. The focused regression preserves both the unwrapped structure and exact serialization.
Add regression coverage for a valid multi-fragment extension and rejection of duplicate parent/child tables and scalar-to-table redefinitions.
|
Hi, following up on this PR after reading the discussion in #596. I’ve updated the branch against current master and added regression tests for collisions across nested out-of-order fragments. All 1,058 tests pass, and lint and formatting checks pass. Compared with #592, this approach applies the same type-conflict, concrete-table redefinition, and recursive checks to every stored fragment. This keeps validation in one path, avoiding proxy construction and a separate proxy-specific branch that would need to stay consistent with those checks. Both implementations pass the focused tests; I prefer this approach because it keeps the validation logic simpler. Would you be able to take another look? Happy to address any concerns with this approach. |
Summary
OutOfOrderTableProxyduring concrete/super-table validationunrelated array-of-tables
The proxy was incorrectly treated as a non-table, causing valid TOML to raise
KeyAlreadyPresent. Validating the underlyingTablefragments preserves theexisting collision checks without special-casing the proxy.
Closes #571.
Checks
python -m pytest -q— 1052 passedruff check tomlkit tests --exclude tests/toml-testruff format --check tomlkit tests --exclude tests/toml-testThe repository's strict mypy run still reports five pre-existing errors in
untouched
source.py,items.py, andapi.py.Agent Drafting Metadata
tests, and prepared the issue and PR text.