Skip to content

Commit 117fc6b

Browse files
aauschclaude
andcommitted
Python: keep the Python 2 reading of except A, e:
The previous commit read a comma-separated fourth child as a tuple of exception types unconditionally. That is right for Python 3, but the default parser is version-agnostic and also runs when extracting Python 2 (`CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION=2`, `--lang=2`), where `except Exception, e:` really is the alias binding and the canonical idiom. In that mode the change flipped `e` from a `Store` to a `Load` of an undefined name and dropped the binding altogether. So the separator token alone is not enough to decide: `as` binds an alias in every version, a comma binds an alias under Python 2 and builds a tuple otherwise. Chains of three or more are unaffected either way -- they are not valid Python 2, and the default grammar rejects them, so `Module.py_ast` falls back to tree-sitter. The file-driven parser tests cannot express this; they run at the default analysis version and there is no per-fixture way to change it. So `tests/test_except_clause.py` drives `parser.parse` directly with the version flipped, and pins all four combinations -- comma and `as`, Python 2 and 3, plus the parenthesized form that must bind no alias in either. Removing the version gate fails the Python 2 case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 896d8d7 commit 117fc6b

3 files changed

Lines changed: 82 additions & 6 deletions

File tree

python/extractor/semmle/python/parser/ast.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from blib2to3.pgen2 import token
22
from ast import literal_eval
33
from semmle.python import ast
4+
from semmle.util import get_analysis_major_version
45
from blib2to3.pgen2.parse import ParseError
56
import sys
67

@@ -981,13 +982,17 @@ def visit_except_clause(self, node):
981982
if len(node.children) > 1:
982983
type = self.visit(node.children[1], LOAD)
983984
if len(node.children) > 3:
984-
if is_token(node.children[2], "as"):
985+
# The grammar rule `'except' [test [(',' | 'as') test]]` is shared
986+
# between two incompatible readings of a fourth child, so the
987+
# separator token and the analysis version together decide:
988+
# `except A as e:` binds an alias, in every version;
989+
# `except A, e:` binds an alias when extracting Python 2, where
990+
# that is the canonical idiom;
991+
# `except A, B:` is an unparenthesized tuple of exception types
992+
# otherwise -- PEP 758, Python 3.14+.
993+
if is_token(node.children[2], "as") or get_analysis_major_version() == 2:
985994
name = self.visit(node.children[3], STORE)
986995
else:
987-
# PEP 758 (Python 3.14+): `except A, B:` is an unparenthesized
988-
# tuple of exception types, not a Python 2 alias binding. The
989-
# grammar rule `'except' [test [(',' | 'as') test]]` is shared
990-
# between both readings, so the separator token decides.
991996
elts = [type, self.visit(node.children[3], LOAD)]
992997
type = ast.Tuple(elts, LOAD)
993998
set_location(type, node.children[1].start, node.children[3].end)
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import unittest
2+
from contextlib import contextmanager
3+
4+
from semmle import util
5+
from semmle.python import ast
6+
from semmle.python import parser
7+
from semmle.python.parser.dump_ast import StdoutLogger
8+
from semmle.python.parser.tokenizer import Tokenizer
9+
10+
11+
@contextmanager
12+
def analysis_version(version):
13+
'Extract as if `CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION` were `version`.'
14+
previous = util.get_analysis_version()
15+
util.update_analysis_version(version)
16+
try:
17+
yield
18+
finally:
19+
util.update_analysis_version(previous)
20+
21+
22+
class ExceptClauseTest(unittest.TestCase):
23+
'''`except_clause: 'except' [test [(',' | 'as') test]]` is one grammar rule
24+
covering two incompatible readings of `except A, B:` -- a Python 2 alias
25+
binding and a PEP 758 tuple of exception types. Which one the default parser
26+
picks depends on the version being extracted, so these tests pin both.
27+
'''
28+
29+
def handler(self, source):
30+
'The first `except` handler of the first statement of `source`.'
31+
with StdoutLogger() as logger:
32+
module = parser.parse(Tokenizer(source).tokens(), logger)
33+
return module.body[0].handlers[0]
34+
35+
def test_comma_is_a_tuple_of_types_in_python_3(self):
36+
with analysis_version("3.11"):
37+
handler = self.handler("try:\n a\nexcept b, c:\n d\n")
38+
self.assertIsNone(handler.name)
39+
self.assertIsInstance(handler.type, ast.Tuple)
40+
self.assertIsInstance(handler.type.ctx, ast.Load)
41+
self.assertEqual(["b", "c"], [elt.id for elt in handler.type.elts])
42+
for elt in handler.type.elts:
43+
self.assertIsInstance(elt.ctx, ast.Load)
44+
45+
def test_comma_is_an_alias_binding_in_python_2(self):
46+
with analysis_version("2.7.18"):
47+
handler = self.handler("try:\n a\nexcept b, c:\n d\n")
48+
self.assertIsInstance(handler.type, ast.Name)
49+
self.assertEqual("b", handler.type.id)
50+
self.assertIsInstance(handler.type.ctx, ast.Load)
51+
self.assertIsInstance(handler.name, ast.Name)
52+
self.assertEqual("c", handler.name.id)
53+
self.assertIsInstance(handler.name.ctx, ast.Store)
54+
55+
def test_as_is_an_alias_binding_in_both_versions(self):
56+
for version in ("3.11", "2.7.18"):
57+
with analysis_version(version):
58+
handler = self.handler("try:\n a\nexcept b as c:\n d\n")
59+
self.assertIsInstance(handler.type, ast.Name, version)
60+
self.assertEqual("b", handler.type.id, version)
61+
self.assertIsInstance(handler.name, ast.Name, version)
62+
self.assertEqual("c", handler.name.id, version)
63+
self.assertIsInstance(handler.name.ctx, ast.Store, version)
64+
65+
def test_parenthesised_types_bind_no_alias_in_either_version(self):
66+
for version in ("3.11", "2.7.18"):
67+
with analysis_version(version):
68+
handler = self.handler("try:\n a\nexcept (b, c):\n d\n")
69+
self.assertIsNone(handler.name, version)
70+
self.assertIsInstance(handler.type, ast.Tuple, version)
71+
self.assertEqual(["b", "c"], [elt.id for elt in handler.type.elts], version)
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: fix
33
---
4-
* Fixed the extraction of PEP 758 `except A, B:` clauses by the default (non-tree-sitter) Python parser. Previously the second exception type was extracted as a Python 2 style alias binding, so it was recorded as a `Store` rather than a use. This caused false positives from queries that reason about whether a name is used, such as `py/unused-import`.
4+
* Fixed the extraction of PEP 758 `except A, B:` clauses by the default (non-tree-sitter) Python parser. Previously the second exception type was extracted as a Python 2 style alias binding, so it was recorded as a `Store` rather than a use. This caused false positives from queries that reason about whether a name is used, such as `py/unused-import`. When extracting Python 2 (`--lang=2`), `except A, e:` continues to bind `e` as an alias, since that is what the syntax means in that version.

0 commit comments

Comments
 (0)