Fix a regression rejecting a valid out-of-order table extension - #596
Fix a regression rejecting a valid out-of-order table extension#596afonsojanu wants to merge 2 commits into
Conversation
_validate_table_candidate() flags a type mismatch whenever one side of a key is a Table/AoT and the other isn't, to catch a genuine table-vs-scalar redefinition. But once an out-of-order table gets merged, the existing value under that key is an OutOfOrderTableProxy, not a Table or AoT instance, even though it's still very much a table. A later fragment extending that same key (itself a plain Table) then looks like a type conflict and gets rejected with KeyAlreadyPresent, even though tomllib and earlier tomlkit releases both accept the document fine. Reported in python-poetry#571 with a minimal repro and the root cause already narrowed down to this exact check, introduced by the concrete+super validation in python-poetry#530. Treating OutOfOrderTableProxy as table-like here fixes the regression without touching the concrete/super logic below it. A related report, python-poetry#577, hits a different KeyAlreadyPresent inside OutOfOrderTableProxy's own AoT-extension merge path rather than this validation check, so it's a separate bug and stays open.
for more information, see https://pre-commit.ci
|
Good question, and looking at it side by side I don't think mine adds anything #592 doesn't already cover. Both land on the same core fix (counting OutOfOrderTableProxy as table-like in the candidate check), #592 was opened a good while before mine, and it also keeps the deeper duplicate-detection recursion intact by walking the proxy's own fragments, which mine skips. I tested a case with a genuine nested duplicate inside a proxied table and both still catch it correctly, so functionally they land in the same place, but #592 is the more complete of the two. #572 takes a different route entirely (validating against every stored index for a key rather than special-casing the proxy type), so that one's worth judging on its own merits separately from these two. Closing this one in favor of #592. |
2 participants
Fixes #571.
Reproduced the exact case from the issue: an out-of-order table (tool.ruff.lint) gets a further sibling table after an unrelated header comes in between. Once merged, the existing entry under that key is an OutOfOrderTableProxy rather than a bare Table or AoT, and _validate_table_candidate's type check treats that as a table-vs-non-table mismatch, raising KeyAlreadyPresent on input tomllib and earlier tomlkit releases both accept.
The fix just widens that check to count OutOfOrderTableProxy as table-like too, same as Table and AoT already are. The concrete/super merge logic right below stays untouched.
Added a regression test using the issue's own minimized repro, checked it fails on unpatched master (git stash confirms) and passes with the fix. Full suite is green (1053 passed, including the toml-test compliance suite).
I also looked at #577 since it sounded related, but that one's actually a different bug in OutOfOrderTableProxy's own AoT-extension merge path, not this validation check, so I left it alone rather than trying to fold in a second fix here.