Skip to content

Commit 31f91f8

Browse files
committed
fix: multiline statement branches with sysmon #2070
Some cases of conditions or for loops were reported as missing branches when using sys.monitoring, which is now the default in 3.14.
1 parent d2b0a9b commit 31f91f8

File tree

4 files changed

+72
-7
lines changed

4 files changed

+72
-7
lines changed

CHANGES.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,16 @@ upgrading your version of coverage.py.
2323
Unreleased
2424
----------
2525

26+
- Fix: some multi-line case clauses or for loops (and probably other
27+
constructs) could cause incorrect claims of missing branches with the
28+
sys.monitoring core, as described in `issue 2070`_. This is now fixed.
29+
2630
- Split ``sqlite`` debugging information out of the ``sys`` :ref:`coverage
2731
debug <cmd_debug>` and :ref:`cmd_run_debug` options since it's bulky and not
2832
very useful.
2933

34+
.. _issue 2070: https://github.com/nedbat/coveragepy/issues/2070
35+
3036

3137
.. start-releases
3238

coverage/bytecode.py

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
import collections
99
import dis
1010
from types import CodeType
11-
from typing import Iterable, Optional
11+
from typing import Iterable, Mapping, Optional
1212

13-
from coverage.types import TArc, TOffset
13+
from coverage.types import TArc, TLineNo, TOffset
1414

1515

1616
def code_objects(code: CodeType) -> Iterable[CodeType]:
@@ -31,7 +31,9 @@ def op_set(*op_names: str) -> set[int]:
3131
3232
The names might not exist in this version of Python, skip those if not.
3333
"""
34-
return {op for name in op_names if (op := dis.opmap.get(name))}
34+
ops = {op for name in op_names if (op := dis.opmap.get(name))}
35+
assert ops, f"At least one opcode must exist: {op_names}"
36+
return ops
3537

3638

3739
# Opcodes that are unconditional jumps elsewhere.
@@ -99,10 +101,16 @@ def walk(
99101
TBranchTrails = dict[TOffset, TBranchTrailsOneSource]
100102

101103

102-
def branch_trails(code: CodeType) -> TBranchTrails:
104+
def branch_trails(
105+
code: CodeType,
106+
multiline_map: Mapping[TLineNo, TLineNo],
107+
) -> TBranchTrails:
103108
"""
104109
Calculate branch trails for `code`.
105110
111+
`multiline_map` maps line numbers to the first line number of a
112+
multi-line statement.
113+
106114
Instructions can have a jump_target, where they might jump to next. Some
107115
instructions with a jump_target are unconditional jumps (ALWAYS_JUMPS), so
108116
they aren't interesting to us, since they aren't the start of a branch
@@ -128,6 +136,7 @@ def branch_trails(code: CodeType) -> TBranchTrails:
128136
from_line = inst.line_number
129137
if from_line is None:
130138
continue
139+
from_line = multiline_map.get(from_line, from_line)
131140

132141
def add_one_branch_trail(
133142
trails: TBranchTrailsOneSource,
@@ -138,8 +147,11 @@ def add_one_branch_trail(
138147
to_line = None
139148
for inst2 in iwalker.walk(start_at=start_at, follow_jumps=True):
140149
inst_offsets.add(inst2.offset)
141-
if inst2.line_number and inst2.line_number != from_line:
142-
to_line = inst2.line_number
150+
l2 = inst2.line_number
151+
if l2 is not None:
152+
l2 = multiline_map.get(l2, l2)
153+
if l2 and l2 != from_line:
154+
to_line = l2
143155
break
144156
elif inst2.jump_target and (inst2.opcode not in ALWAYS_JUMPS):
145157
break

coverage/sysmon.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from coverage.bytecode import TBranchTrails, always_jumps, branch_trails
2121
from coverage.debug import short_filename, short_stack
2222
from coverage.misc import isolate_module
23+
from coverage.parser import PythonParser
2324
from coverage.types import (
2425
AnyCallable,
2526
TFileDisposition,
@@ -428,7 +429,8 @@ def sysmon_branch_either(
428429
if not code_info.branch_trails:
429430
if self.stats is not None:
430431
self.stats["branch_trails"] += 1
431-
code_info.branch_trails = branch_trails(code)
432+
multiline_map = get_multiline_map(code.co_filename)
433+
code_info.branch_trails = branch_trails(code, multiline_map=multiline_map)
432434
code_info.always_jumps = always_jumps(code)
433435
# log(f"branch_trails for {code}:\n{ppformat(code_info.branch_trails)}")
434436
added_arc = False
@@ -462,3 +464,11 @@ def sysmon_branch_either(
462464
# log(f"adding unforeseen {arc=}")
463465

464466
return DISABLE
467+
468+
469+
@functools.lru_cache(maxsize=5)
470+
def get_multiline_map(filename: str) -> dict[TLineNo, TLineNo]:
471+
"""Get a PythonParser for the given filename, cached."""
472+
parser = PythonParser(filename=filename)
473+
parser.parse_source()
474+
return parser.multiline_map

tests/test_arcs.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,19 @@ def forelse(seq):
720720
branchz_missing="26",
721721
)
722722

723+
def test_split_for(self) -> None:
724+
self.check_coverage(
725+
"""\
726+
a = 0
727+
for (i
728+
) in [1,2,3,4,5]:
729+
a += i
730+
assert a == 15
731+
""",
732+
lines=[1, 2, 4, 5],
733+
branchz="24 25",
734+
)
735+
723736
def test_while_else(self) -> None:
724737
self.check_coverage(
725738
"""\
@@ -1637,6 +1650,30 @@ def absurd(x):
16371650
)
16381651
assert self.stdout() == "also not default\n"
16391652

1653+
def test_split_match_case(self) -> None:
1654+
self.check_coverage(
1655+
"""\
1656+
def foo(x):
1657+
match x:
1658+
case (
1659+
1
1660+
| 2
1661+
):
1662+
return "output: 1 or 2"
1663+
case _:
1664+
return "output: other"
1665+
1666+
print(foo(1))
1667+
print(foo(2))
1668+
print(foo(3))
1669+
""",
1670+
lines=[1, 2, 3, 7, 8, 9, 11, 12, 13],
1671+
missing="",
1672+
branchz="37 38",
1673+
branchz_missing="",
1674+
)
1675+
assert self.stdout() == "output: 1 or 2\noutput: 1 or 2\noutput: other\n"
1676+
16401677

16411678
class OptimizedIfTest(CoverageTest):
16421679
"""Tests of if statements being optimized away."""

0 commit comments

Comments
 (0)