Python: fix PEP 758 except A, B: extraction in the default parser

[aausch]

What

except A, B: (PEP 758, Python 3.14+) is extracted by the default parser using the Python 2 reading: B becomes an alias binding (Store) instead of a use (Load). Queries that reason about whether a name is used then misfire — py/unused-import reports the import of B as unused.

Why

blib2to3/Grammar.txt shares one rule between both readings:

except_clause: 'except' [test [(',' | 'as') test]]

and visit_except_clause never looked at the separator, so a fourth child was always bound as an alias:

if len(node.children) > 3:
    name = self.visit(node.children[3], STORE)

This patch decides from the separator token and the version being extracted:

clause	Python 3	Python 2
except A as e:	alias binding	alias binding
except A, B:	tuple of types, Load, no alias	alias binding

So as behaves as before everywhere, and the comma form only changes meaning when we are not extracting Python 2. See Python 2 below.

Only the two-type form was affected. except A, B, C: already extracted correctly, and parenthesized forms were never affected.

Verification

• Matches the existing expectation. With this change the default parser reproduces the checked-in python/extractor/tests/parser/exceptions_new.expected — the tree-sitter parser's expected AST for this exact syntax — byte for byte, locations and contexts included.
• No collateral change. Dumping the default parser's AST for all 37 files in tests/parser/ before and after, only the two files containing PEP 758 syntax differ.
• New parser test. tests/parser/exceptions_relaxed.py is unsuffixed, so the harness asserts the two parsers produce identical ASTs. It fails on main and passes with this change. pytest tests/test_parser.py → 37 passed.
• New Python 2 extractor test. python/ql/test/2/extractor-tests/relaxed_except extracts with --lang=2 and pins, per handler, the types and whether the bound name is a definition — the property a query sees, not the shape of the AST. Removing the version gate makes it fail.
• End to end. Patching this file into the codeql-bundle-v2.26.3 extractor and re-running Imports/UnusedImport.ql over the reproduction removes the false positive, while a genuinely unused import in the same file is still reported.
• Whole suites, against a real CLI. With this branch's extractor patched into codeql 2.26.3: python/ql/test/query-tests/Imports all 17 passed, python/ql/test/2/extractor-tests 10 passed, and python/extractor pytest 116 passed. 2/extractor-tests/hidden fails, identically against the unpatched 2.26.3 extractor, so it does not come from this branch.
• New query tests. python/ql/test/query-tests/Imports/unused/ gains coverage for two-, three- and four-type chains through the real extraction path. Reverting the extractor fix in a 2.26.3 bundle makes relaxed_except.py report Import of 'Beta' is not used. and the test fail.

Chains longer than two

Worth recording, because the two cases behave differently and the difference is not obvious.

clause	default parser	result today
except A, B:	parses (as Python 2)	wrong AST reaches the queries
except A, B, C:	SyntaxError	Module.py_ast falls back to tree-sitter, AST is correct
except A, B, C, D:	SyntaxError	same, correct

except_clause: 'except' [test [(',' | 'as') test]] admits exactly one trailing
test, so three or more types cannot parse at all. The fallback in
semmle/python/modules.py then rescues them. Only the two-type form is silently
wrong, precisely because it is the only one the default parser accepts.

This has a consequence for testing that cost me a first attempt: the two cases
cannot share a file. Any three-type clause fails the default parser, sends the
whole file to tree-sitter, and masks the two-type behaviour completely. So the
query tests are split into relaxed_except.py and relaxed_except_long.py,
with a comment in each explaining why. For the same reason each name is used in
exactly one clause — a name that also appears in a parenthesized clause is a use
regardless, and hides the defect.

I have deliberately not changed the grammar to accept longer chains. The
fallback already yields correct results, and a Grammar.txt change is a much
larger and riskier diff than the defect warrants. It may still be worth doing:
the fallback costs a failed parse per affected file, which measured at roughly
541ms versus 9ms for a neighbouring file in the same run. Happy to open that
separately if you would like it.

Python 2

except ValueError, e: is the canonical Python 2 idiom and it parses under the Python 3 grammar too, so the input is genuinely ambiguous and the grammar rule cannot separate the two readings. The default parser is version-agnostic and is tried first, which means it also handles Python 2 extraction (CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION=2, --lang=2).

The first version of this branch read the comma form as a tuple unconditionally, which broke that case: e flipped from a Store to a Load of an undefined name and the exception stopped being bound at all. Thanks to @redsun82 for catching it. visit_except_clause now consults get_analysis_major_version(), so Python 2 extraction keeps the alias binding it had before this branch, byte for byte, and only Python 3 extraction gets the PEP 758 reading.

Testing that through extraction needed one fix first. populator.main honours --lang by calling update_analysis_version, which rebinds a global in the process that parses the options — but extraction runs in an ExtractorPool, and on macOS those workers are spawned rather than forked, so they re-read CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION and saw the default of 3. --lang=2 therefore meant Python 2 on Linux and Python 3 on macOS. The flag now sets the environment variable as well, so it means the same thing on both. Real Python 2 extraction was never affected: the CodeQL action sets that variable itself and worker processes inherit it.

Chains of three or more need no gate. They are not valid Python 2, and the default grammar rejects them regardless of version, so Module.py_ast falls back to tree-sitter — see Chains longer than two.

Fixes #22387

---

Investigated with assistance from Claude Code. Every result quoted above was executed against the real 2.26.3 bundle and the repo's own test harness, not inferred.

[Copilot]

Pull request overview

Fixes default Python parser extraction of PEP 758 exception lists, aligning it with the tree-sitter parser.

Changes:

• Distinguishes as aliases from comma-separated exception types.
• Adds parser parity regression coverage.
• Documents the corrected extraction behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
python/extractor/semmle/python/parser/ast.py	Extracts except A, B: as a load-context tuple.
python/extractor/tests/parser/exceptions_relaxed.py	Tests exception-list and alias variants across parsers.
python/ql/lib/change-notes/2026-08-19-legacy-parser-relaxed-except.md	Records the parser fix and query impact.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[redsun82]

I think this might actually be a problem for Python 2, which is still officially supported 😅

The default (blib2to3) parser is the primary one (modules.py tries it first, tree-sitter is only the fallback), and this branch is version-agnostic, so it also kicks in when we extract in Python 2 mode (CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION=2). There except Exception, e: really is the alias binding, so with this change e flips from a Store to a Load of an undefined name, and the exception stops being bound. That's the canonical py2 idiom, so it's not exactly a rare construct.

Could we gate the tuple reading on the version? Something like keeping the old alias branch when get_analysis_major_version() == 2, and only building the tuple otherwise. The 3+ types case isn't affected (three unparenthesized types aren't valid py2 anyway, so the tree-sitter fallback is fine there).

[aausch]

Good catch — and it was worse than a mislabelled AST: in Python 2 mode the binding disappeared entirely, so e became a Load of an undefined name.

Gated as you suggested in 117fc6b. as binds an alias in every version; a comma binds an alias when get_analysis_major_version() == 2 and builds the tuple otherwise. Three or more types are unaffected — not valid py2, and the default grammar rejects them regardless of version, so the tree-sitter fallback covers them.

The file-driven parser tests can't express this, since they run at the default analysis version with no per-fixture override. So python/extractor/tests/test_except_clause.py drives parser.parse directly with the version flipped and pins all four combinations: comma and as, py2 and py3, plus the parenthesized form that must bind no alias in either. Removing the gate fails the py2 case, and pytest tests/test_parser.py still passes 37.

Happy to add a --lang=2 query test under Imports/unused/ as well if you would rather see it through the real extraction path — I left it out because the unit test pins the exact decision site.

I have also rewritten the "Trade-off" section of the description, which described the behaviour this replaces.

[tausbn]

A more appropriate solution would be to add an extractor test in python/ql/test/2/extractor-tests. I think that's preferable to a bespoke unit test.

[aausch]

Agreed — done in 7d9ae21, and the unit test is gone. python/ql/test/2/extractor-tests/relaxed_except extracts with --lang=2 and pins, per handler, the types and whether the bound name is a definition, so it asserts what a query actually sees rather than the AST shape. Removing the version gate makes it fail.

Getting there turned up something you may want to know independently of this PR: --lang did not reach the worker processes. populator.main honours it by calling update_analysis_version, but that rebinds a global in the process that parses the options, while extraction happens in an ExtractorPool — and on macOS those workers are spawned, not forked, so they re-read CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION and saw the default of 3. So --lang=2 meant Python 2 on Linux and Python 3 on macOS. My first version of this test passed under CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION=2 and failed under --lang=2 on the same machine, which is what gave it away. The commit also sets the environment variable alongside the global, so the flag means the same thing on both platforms. Real Python 2 extraction was never affected — the action sets that variable itself and children inherit it. Happy to split that into its own PR if you would rather it not ride along.

I also bumped the extractor version, which the first commit should have done.

Verified with codeql 2.26.3, this branch's extractor patched into it:

• python/ql/test/2/extractor-tests — 10 passed. hidden/test.ql fails, but it fails identically against the unpatched 2.26.3 extractor (extra | .hidden/inner | and | folder | rows), so it is not from this branch.
• python/ql/test/query-tests/Imports — all 17 passed, which also re-confirms the relaxed_except*.py query tests from the earlier commit.
• python/extractor pytest — 116 passed, including the 37 parser tests.

[tausbn]

This looks good to me. I think the explicit --lang=2 in the extractor options is not needed in this case, since the tests in that directory are already run with that flag, but it's harmless, so there's no need to change that. Good catch on the macOS spawn issue!

I'll run some more internal checks before merging, but otherwise I think this should be good to go. Thanks for the contribution! ❤️