Skip to content

Honour 'pragma: no mutate block' on else/except/finally arms - #559

Merged
nicklafleur merged 2 commits into
boxed:mainfrom
dylanpulver:fix/pragma-block-on-compound-arms
Sep 5, 2026
Merged

Honour 'pragma: no mutate block' on else/except/finally arms#559
nicklafleur merged 2 commits into
boxed:mainfrom
dylanpulver:fix/pragma-block-on-compound-arms

Conversation

@dylanpulver

@dylanpulver dylanpulver commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

_visit_compound_header is only reachable from visit_If / For / While / With / Try / FunctionDef / ClassDef / Match. In libcst, else, except, except* and finally are separate nodes owning their own suite, so a # pragma: no mutate block on one of those header lines reaches no visitor and is dropped — silently, which is the bad part: you mark an error branch as skipped and it gets mutated anyway.

Measured on main, ignore_node_lines for a pragma on each header:

if       [2,3]   works        else (if/for/while/try)   []   ignored
elif     [4,5]   works        except                    []   ignored
while    [2,3]   works        except*                   []   ignored
with     [2,3]   works        finally                   []   ignored
try      [2,3]   works        try  (of a try/except*)   []   ignored

elif works because it parses as a nested If; the else one line below it does not. The last row is the same source line behaving differently depending on whether a later handler writes except or except*try/except* parses as TryStar, and there is no visit_TryStar.

README:291 says the block pragma "works on any compound statement -- functions, classes, if/elif/else, loops, context managers, etc."; README:345 extends it to for/else, while/else and try/except/finally.

Five visitors added, in the existing style. Match/MatchCase deliberately untouched#554 is fixing that, and I did not want to duplicate it. I cherry-picked this commit onto pull/554/head: it auto-merges with zero conflicts and the combined tree passes 50/50 in test_pragma_handling.py. Land them in either order.

Checked by reverting: with pragma_handling.py back at main and the tests kept, exactly the 8 new tests fail and the other 36 pass. Adding only visit_Else still fails 4 of them (except, except*, finally, TryStar), so each visitor is pinned rather than carried by its neighbours. A sibling-leak case asserts the statement after the else block stays mutable.

uv run pytest gives 387 passed / 2 failed against 379 passed / 2 failed on main, same venv after uv sync (three consecutive runs each). The two failures are the test_e2e_type_checking snapshot tests and are identical on both trees. test_safe_setproctitle is flaky here on macOS — it failed in one earlier run of each tree and in none of the three above. ruff check and ruff format clean at the pinned 0.15.7; mypy reports 19 errors, all in __main__.py, none in the files touched, identical to main. I did not run ./scripts/run_tests.sh in the Linux container.

Found by enumerating the visit_* methods against the libcst node types that own a suite, then probing each. AI assistance: this change and its tests were drafted with Claude Opus 5 (claude-opus-5). The commands and numbers above were run locally.

_visit_compound_header is only reachable from visit_If/For/While/With/
Try/FunctionDef/ClassDef/Match. In libcst, else, except, except* and
finally are separate nodes owning their own suite, so a pragma on their
header line reached no visitor and was silently dropped -- while elif,
which parses as a nested If, worked. try/except* parses as TryStar, so
the pragma on its try line was dropped too.

README documents the block pragma as working on 'any compound statement
-- functions, classes, if/elif/else, loops, context managers, etc.'

Match/MatchCase is left alone; boxed#554 is fixing that.

@nicklafleur nicklafleur left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one very minor nit otherwise lgtm

Comment thread tests/mutation/test_pragma_handling.py
The parametrized cases were escaped one-line strings while every other
test in the file writes its source as a triple-quoted block with real
newlines and indentation. Match the surrounding style so the pragma
placement is readable at a glance. No behaviour change.
@nicklafleur
nicklafleur merged commit 2f39c84 into boxed:main Sep 5, 2026
6 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