Skip to content

Copy trivia in _new so arithmetic results cannot mutate the original item - #683

Merged
frostming merged 3 commits into
python-poetry:masterfrom
feiiiiii5:d166-new-trivia-copy
Oct 27, 2026
Merged

frostming merged 3 commits into
python-poetry:masterfrom
feiiiiii5:d166-new-trivia-copy

Conversation

@feiiiiii5

Copy link
Copy Markdown
Contributor

Summary

The _new constructors of Integer, Float, String, DateTime, Date and Time pass the original item's Trivia object to the result, so the result aliases the original's comment/indentation state. Editing the result then rewrites the line still stored in the document:

doc = tomlkit.parse('x = 1  # keep\ny = 2\n')
z = doc["x"] + 1
z.trivia.comment = "# MUTATED"
tomlkit.dumps(doc)
# 'x = 1  # MUTATED\ny = 2\n'   <- the document changed via the result

Table.__copy__ and InlineTable.__copy__ in the same file already pass self._trivia.copy() — Trivia.copy() exists for exactly this — and the container's _replace_at copies trivia explicitly, so only the six arithmetic/replace() paths leak. This adds the same .copy() there; nothing changes in what gets rendered at creation time, only the aliasing is cut.

New parametrized test test_arithmetic_result_does_not_share_trivia (int/float/string/datetime/date/time): asserts result.trivia is not original.trivia and that mutating the result's comment leaves the document byte-identical. All 6 cases fail on current master and pass here. Full suite: pytest tests/ --ignore=tests/test_toml_tests.py — 529 passed.

Agent Drafting Metadata

  • Agent: ZCode coding agent
  • Model: GLM-5.3
  • Notes: Reproduction, six-line fix and regression test verified in an isolated worktree against origin/master (eddd27e); full offline test suite run before and after.

feiiiiii5 and others added 3 commits October 25, 2026 11:25
Integer/Float/String/DateTime/Date/Time _new passed the original
item's Trivia object to the result, so editing the result's comment or
indentation rewrote the line still stored in the document. Table and
InlineTable __copy__ already copy trivia; do the same on the six
arithmetic paths.
pre-commit's ruff hook flags list(doc)[0] as SIM118. The test only wants
the first key, so next(iter(doc)) is the same without building a list.
@feiiiiii5

Copy link
Copy Markdown
Contributor Author

The ruff (legacy alias) failure was list(doc)[0] (SIM118) in the new test; fixed in b95d40e with next(iter(doc)). The targeted tests still pass (6 passed) and ruff check tests/test_items.py is clean.

The ubuntu-latest / 3.9 job is not a test failure: setup-python could not find Python 3.9 for Ubuntu 26.04 and stopped before running anything (The version '3.9' with architecture 'x64' was not found for Ubuntu 26.04). The same check passed on the base commit from an earlier runner image.

@feiiiiii5

Copy link
Copy Markdown
Contributor Author

The only red job is ubuntu-latest / 3.9, and it fails in the Set up Python 3.9 step before checkout, dependency installation or tests; the other 24 jobs pass, including Python 3.9 on macOS and Windows. That looks like a setup-python runner failure rather than this patch. The failing job: https://github.com/python-poetry/tomlkit/actions/runs/40259867291/job/121369693175

@frostming
frostming merged commit e781ee0 into python-poetry:master Oct 27, 2026
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants