Skip to content

Commit 7d9ae21

Browse files
aauschclaude
andcommitted
Python: test the Python 2 except reading through extraction, not a unit test
Replaces `tests/test_except_clause.py` with an extractor test, as suggested in review. `python/ql/test/2/extractor-tests/relaxed_except` extracts a Python 2 file with `--lang=2` and pins, per handler, the types and whether the bound name is a definition -- so it asserts the consequence a query sees, not the shape of the AST. Removing the version gate from `visit_except_clause` makes it fail. Doing it that way needed one extractor fix first. `populator.main` honours `--lang` by calling `update_analysis_version`, but that only rebinds a global in the process that parses the options; the extraction itself runs in an `ExtractorPool`, and on macOS those workers are spawned rather than forked, so they re-read `CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION` from the environment and saw the default of 3. `--lang=2` therefore meant Python 2 on Linux and Python 3 on macOS. Setting the variable as well as the global makes the flag mean the same thing on both, which is what lets the new test pin the Python 2 reading anywhere. Real Python 2 extraction was never affected: the CodeQL action sets that variable itself, and children inherit it. Bumps the extractor version, which the fix in the first commit should have done. Verified with codeql 2.26.3 and this branch's extractor patched into it: `python/ql/test/2/extractor-tests` 10 passed (`hidden` fails identically on the unpatched extractor, so it is not from this branch), and the py3 side is unchanged -- `python/ql/test/query-tests/Imports` all 17 passed, which also confirms the `relaxed_except*.py` query tests added earlier. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 117fc6b commit 7d9ae21

7 files changed

Lines changed: 55 additions & 72 deletions

File tree

python/extractor/semmle/populator.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ def main(sys_path = sys.path[:]):
6565
if options.language_version:
6666
last_version = options.language_version[-1]
6767
update_analysis_version(last_version)
68+
# Worker processes are spawned rather than forked on macOS, so they do
69+
# not inherit the value set above; they re-read it from the environment
70+
# as this module did on import. Set it there too, or `--lang` would take
71+
# effect in this process only, and on one platform only.
72+
os.environ["CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION"] = last_version
6873

6974
found_py2 = False
7075
if get_analysis_major_version() == 2 and options.extract_stdlib:

python/extractor/semmle/util.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
#Semantic version of extractor.
1212
#Update this if any changes are made
13-
VERSION = "7.1.8"
13+
VERSION = "7.1.9"
1414

1515
PY_EXTENSIONS = ".py", ".pyw"
1616

python/extractor/tests/test_except_clause.py

Lines changed: 0 additions & 71 deletions
This file was deleted.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
semmle-extractor-options: --lang=2
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
| 6 | ValueError | err (definition) |
2+
| 12 | ValueError | other (definition) |
3+
| 18 | ValueError, TypeError | none |
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* The types of each `except` clause, and the name it binds. In Python 2 the
3+
* comma form binds a name and has a single type; reading it as a PEP 758 tuple
4+
* instead would give two types and no name.
5+
*/
6+
7+
import python
8+
9+
from ExceptStmt handler, string types, string name
10+
where
11+
types =
12+
concat(Expr type |
13+
type = handler.getType()
14+
|
15+
type.toString(), ", " order by type.getLocation().getStartColumn()
16+
) and
17+
(
18+
exists(Name bound | bound = handler.getName() |
19+
bound.isDefinition() and name = bound.getId() + " (definition)"
20+
or
21+
not bound.isDefinition() and name = bound.getId() + " (use)"
22+
)
23+
or
24+
not exists(handler.getName()) and name = "none"
25+
)
26+
select handler.getLocation().getStartLine(), types, name
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# When extracting Python 2, `except A, e:` binds `e`. It is not a PEP 758
2+
# unparenthesized tuple of exception types, which is what the same syntax means
3+
# from Python 3.14 on.
4+
try:
5+
unlikely()
6+
except ValueError, err:
7+
print err
8+
9+
# `as` means the same thing in every version.
10+
try:
11+
unlikely()
12+
except ValueError as other:
13+
print other
14+
15+
# A parenthesized tuple is several types, and binds nothing.
16+
try:
17+
unlikely()
18+
except (ValueError, TypeError):
19+
pass

0 commit comments

Comments
 (0)