From 918c05c538bb139f22e6a3f1f89aaef5bd314d0f Mon Sep 17 00:00:00 2001 From: Taus Date: Tue, 11 Feb 2025 12:58:52 +0000 Subject: [PATCH 1/3] Python: Don't prune any `MatchLiteralPattern`s Extends the mechanism introduced in https://github.com/github/codeql/pull/18030 to behave the same for _all_ `MatchLiteralPattern`s, not just the ones that happen to be the constant `True` or `False`. Co-authored-by: yoff --- python/extractor/semmle/python/passes/pruner.py | 6 +++--- .../Statements/unreachable/UnreachableCode.expected | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/python/extractor/semmle/python/passes/pruner.py b/python/extractor/semmle/python/passes/pruner.py index fe3b03d453cd..5b69ec25bf3d 100644 --- a/python/extractor/semmle/python/passes/pruner.py +++ b/python/extractor/semmle/python/passes/pruner.py @@ -203,7 +203,8 @@ def __init__(self): self.nodes = set() def visit_MatchLiteralPattern(self, node): - # MatchLiteralPatterns _look_ like boolean tests, but are not. + # MatchLiteralPatterns _look_ like boolean tests in that they have both a true ("matched") + # and false ("didn't match") successor, but are not. # Thus, without this check, we would interpret # # match x: @@ -212,8 +213,7 @@ def visit_MatchLiteralPattern(self, node): # # (and similarly for True) as if it was a boolean test. This would cause the true edge # (leading to pass) to be pruned later on. - if isinstance(node.literal, ast.Name) and node.literal.id in ('True', 'False'): - self.nodes.add(node.literal) + self.nodes.add(node.literal) class NonlocalVisitor(ASTVisitor): def __init__(self): diff --git a/python/ql/test/query-tests/Statements/unreachable/UnreachableCode.expected b/python/ql/test/query-tests/Statements/unreachable/UnreachableCode.expected index f5e74fab8d49..2417041f472d 100644 --- a/python/ql/test/query-tests/Statements/unreachable/UnreachableCode.expected +++ b/python/ql/test/query-tests/Statements/unreachable/UnreachableCode.expected @@ -4,6 +4,3 @@ | test.py:21:5:21:38 | For | This statement is unreachable. | | test.py:28:9:28:21 | ExprStmt | This statement is unreachable. | | test.py:84:5:84:21 | ExceptStmt | This statement is unreachable. | -| test.py:158:9:159:16 | Case | This statement is unreachable. | -| test.py:162:13:162:16 | Pass | This statement is unreachable. | -| test.py:167:13:167:16 | Pass | This statement is unreachable. | From a69e3f523666f4dffa06f6c845ca68364f320e76 Mon Sep 17 00:00:00 2001 From: Taus Date: Tue, 11 Feb 2025 13:02:09 +0000 Subject: [PATCH 2/3] Python: Add change note Co-authored-by: yoff --- .../lib/change-notes/2025-02-11-fix-match-literal-pruning.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 python/ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md diff --git a/python/ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md b/python/ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md new file mode 100644 index 000000000000..79b2d4b11fec --- /dev/null +++ b/python/ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md @@ -0,0 +1,5 @@ +--- +category: fix +--- + +- `MatchLiteralPattern`s are now never pruned, as this could lead to code being wrongly identified as unreachable. From f246ef764a8be8ff148d97732a3ee0acb4d6abfd Mon Sep 17 00:00:00 2001 From: Taus Date: Tue, 4 Mar 2025 18:09:54 +0100 Subject: [PATCH 3/3] Python: Update change note Co-authored-by: Aditya Sharad <6874315+adityasharad@users.noreply.github.com> --- .../ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md b/python/ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md index 79b2d4b11fec..957f2a4ca99c 100644 --- a/python/ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md +++ b/python/ql/lib/change-notes/2025-02-11-fix-match-literal-pruning.md @@ -2,4 +2,4 @@ category: fix --- -- `MatchLiteralPattern`s are now never pruned, as this could lead to code being wrongly identified as unreachable. +- `MatchLiteralPattern`s such as `case None: ...` are now never pruned from the extracted source code. This fixes some situations where code was wrongly identified as unreachable.