Skip to content

Commit 1ec44f9

Browse files
authored
Merge pull request #22386 from aausch/aausch/python-pep758-legacy-parser
Python: fix PEP 758 `except A, B:` extraction in the default parser
2 parents d0aa8f8 + 7f8d03d commit 1ec44f9

21 files changed

Lines changed: 169 additions & 2 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/python/parser/ast.py

Lines changed: 15 additions & 1 deletion
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,7 +982,20 @@ 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-
name = self.visit(node.children[3], STORE)
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:
994+
name = self.visit(node.children[3], STORE)
995+
else:
996+
elts = [type, self.visit(node.children[3], LOAD)]
997+
type = ast.Tuple(elts, LOAD)
998+
set_location(type, node.children[1].start, node.children[3].end)
985999
return type, name
9861000

9871001
def visit_del_stmt(self, node):

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
try:
2+
a
3+
except b, c:
4+
d
5+
except (e, f):
6+
g
7+
except h as i:
8+
j
9+
except k:
10+
l
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
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.
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
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
| relaxed_except.py:2:1:2:43 | Import | Import of 'Beta' is not used. |

0 commit comments

Comments
 (0)