Skip to content

Fix list of keys before modifying that same dictionary#1666

Merged
rolandwalker merged 1 commit intomainfrom
RW/fix-dict-iteration-warnings-styles
Mar 2, 2026
Merged

Fix list of keys before modifying that same dictionary#1666
rolandwalker merged 1 commit intomainfrom
RW/fix-dict-iteration-warnings-styles

Conversation

@rolandwalker
Copy link
Contributor

Description

Fixes a bug in #1653.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

@rolandwalker rolandwalker self-assigned this Mar 1, 2026
@github-actions
Copy link

github-actions bot commented Mar 1, 2026

  1. Missing regression test for the exact bug fixed (medium)
    The PR changes iteration to for warning_token in list(style.keys()) in mycli/clistyle.py:189, which prevents mutating a dict during iteration. There is no corresponding test to lock this behavior.
    Action: add a unit test for style_factory_helpers(..., warnings=True) that asserts it does not raise and that Token.Output.* entries are created from Token.Warnings.*.

  2. Current style tests don’t protect this path (medium)
    Existing tests in test/test_clistyle.py are skipped, so this fix is currently unguarded by active tests.
    Action: add/enable a non-skipped test for style_factory_helpers (not only style_factory_toolkit) covering warnings-style token copying.

Security concerns found in this PR: none.

I couldn’t execute tests in this environment (pytest and required deps are unavailable), so runtime verification here is limited.

@rolandwalker rolandwalker merged commit 301568a into main Mar 2, 2026
10 checks passed
@rolandwalker rolandwalker deleted the RW/fix-dict-iteration-warnings-styles branch March 2, 2026 10:15
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.

1 participant