Skip to content

Commit 7e76413

Browse files
juntakiclaude
andauthored
Port LLVM's real coverage-segment semantics; complete-or-nil coverage evidence (#29)
* Port LLVM's real coverage-segment semantics; complete-or-nil coverage evidence SourceCoverageReader.executedLines tracked a single "currently open region," closing it at the next segment's line -- a hand-rolled approximation, not LLVM's own algorithm. #27 (merged) fixed one real symptom of this (a same-line region-closing drop). Investigating a planned test strengthening for that fix (using Fixtures/SwiftPackageMacOS's Pricing.swift line 26 as a single-test-exclusive attribution marker) found a second, deeper gap: line 26 was dropped entirely, not by #27's same-line shape, but because the single-open-region model has no way to represent "control re-enters an enclosing region's count after a nested region closes via a non-entry segment." Confirmed against real, live-captured coverage segments (`--enable-code-coverage`, raw codecov/Pricing.json), not assumed. ## A direct LLVM LineCoverageStats port Replaces the whole algorithm with a Swift port of LLVM's own LineCoverageIterator/LineCoverageStats (llvm/lib/ProfileData/Coverage/CoverageMapping.cpp, fetched and quoted verbatim). Per source line, carries forward the last region-entry segment (`wrapped`) across lines with no entry segment of their own; mapped/executionCount use the same variable names and control flow as upstream, so a future LLVM change is a mechanical diff to re-apply, not a re-derivation. Deliberately not a region stack -- LLVM's segments are per-position coverage state, not push/pop events. Two verified, understood behavior changes, both in the same (safe) direction -- previously under-reporting coverage, a false-noCoverage risk since MutationRunner's baseline fast path skips building/testing a mutant entirely off CoverageMap.isKnownUncovered: - A region's own closing boundary line is now correctly reported covered (previously excluded). - A line whose own fresh region entry has count 0 can still be covered when an enclosing carried-forward count is nonzero. ## Complete-or-nil coverage evidence parse/read were partial-evidence fail-open: a malformed segment in one file, or a corrupted/unreadable coverage JSON among several, silently dropped only that one file's contribution and still returned a map built from whatever else succeeded -- the same partial-map failure class #26 closed for per-test attribution, here at the baseline-coverage level. A covered line silently missing from the map reads as "known uncovered," not "unknown," which can fast-path that line's mutant straight to noCoverage. executedLines now returns nil (distinct from a legitimately-empty []) on a malformed segment. An internal ParseOutcome (.malformed / .parsed([...])) keeps genuine malformation distinguishable from a validly-parsed file that legitimately covers nothing (an untested module/target), so a directory with one real export and one legitimately-empty one still returns the real coverage, never nil. The public parse(_:projectRoot:) API is behavior-unchanged. Extended, after further review against LLVM's own exporter (llvm/tools/llvm-cov/CoverageExporterJson.cpp), to fail closed on every structural element a full/detailed export always emits but a given document does not: a top-level `type` other than "llvm.coverage.json.export"; a `filename` missing or wrong-typed for any file entry; a `segments` key missing or wrong-typed for a file entry inside projectRoot (a full export always includes this key for every in-scope file, even as [] -- its absence is the shape --summary-only output takes, meaning detailed coverage was never captured, not that the file has zero coverage); a module entry missing its own `files` key; a segment with an out-of-range line/column/count, or a (line, column) pair that regresses backwards across the segment list (LLVM's own exporter always emits segments in ascending order, and executedLines's own per-line grouping assumes it). A structurally-empty `segments` array, a file entry with no `segments` key when outside projectRoot, or a file outside projectRoot entirely remain legitimate exclusions, still skipped rather than failing the whole read. ## Tests - SourceCoverageReaderExecutedLinesTests.realBulkDiscountRateSegmentsCoverLine26: the exact raw segments captured live from a real `swift test --enable-code-coverage --filter bulkDiscountRoughly` run -- not a hand-simplified stand-in. - Existing synthetic unit tests updated to match the corrected semantics, with worked-out reasoning for each change; zeroCountSegmentsAreNotExecuted (genuinely all-zero, no preceding positive context) is unaffected and still confirms the true-negative case. - readSurvivesALegitimatelyEmptyFile / readFailsWholeOnModuleMissingFilesArray / fileWithoutFilenameFailsClosed / fileWithoutSegmentsKeyFailsClosed (+ its own read(directory:) counterpart) / wrongTypedColumnFailsClosed / outOfOrderSegmentsFailClosed: the full set of complete-or-nil regression tests, each directly exercising the exact gap it closes. - Upgraded SwiftPackageMacOSSwiftTestingSelectionAcceptanceTests .bulkDiscountRoughlyRoundTrips (the exact-attribution assertion was previously deferred -- see #25) to assert exact attribution on line 26 (bulkDiscountRoughly() alone) against the real toolchain -- confirmed passing. - Full unit suite: 2029/2029 at initial landing (one unrelated timing flake, confirmed transient by isolated rerun); targeted SourceCoverageReader suites: 34/34 after the final round of fixes above. - SwiftLint clean (split SourceCoverageReaderTests into two suites to stay under type_body_length after the new tests -- organizational only). - Three independent-review rounds, all findings fixed in this same branch, none left for a follow-up: (1) the initial complete-or-nil implementation conflated malformed with legitimately-empty; (2) a module missing its own files key fell through the same gap; (3) the same gap one level down, at filename/segments/column/ordering. Independent of the other open PRs in this train -- touches only SourceCoverageReader.swift and its own tests, plus one acceptance-test assertion upgrade. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpcUZbmC1oxNLWRr1FjYkm * Fix vertical whitespace lint violation * Fix a backwards contract statement in a test's own doc comment readSurvivesALegitimatelyEmptyFile's comment read "a genuinely malformed file must not discard the whole directory's coverage" -- the opposite of what the fix (and this test) actually establishes: a malformed sibling file must still discard the whole directory's coverage (fail-closed); only a validly-parsed, legitimately-empty one may not. The test's own assertions were already correct; only the prose describing them was backwards. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpcUZbmC1oxNLWRr1FjYkm --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 9a29098 commit 7e76413

3 files changed

Lines changed: 652 additions & 152 deletions

File tree

Sources/AppleBuildAdapters/SourceCoverageReader.swift

Lines changed: 256 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,47 @@ public enum SourceCoverageReader {
1616
/// Parses every coverage JSON file in `directory`, normalised against
1717
/// `projectRoot`.
1818
///
19-
/// - Returns: `nil` when no coverage files were found or every file failed
20-
/// to parse. The caller treats `nil` as "no coverage information", which
21-
/// is different from "the project has zero coverage": a missing file is
22-
/// missing data, not a measurement.
19+
/// Complete-or-nil against genuine malformation, never against a file
20+
/// that legitimately parsed to zero lines: if any file in `directory`
21+
/// cannot be read, does not parse as a coverage document at all, or
22+
/// contains a segment that fails `executedLines`, the whole result is
23+
/// `nil` — never a map unioned only from the files that happened to
24+
/// succeed. A directory containing a genuine coverage export alongside
25+
/// an unrelated or corrupted file has no honest partial answer — this
26+
/// directory is `SourceCoverageReader`'s own dedicated input (SwiftPM
27+
/// writes nothing else to `codecov/`), so every `*.json` found here is
28+
/// expected to be a real export, and one that isn't is evidence
29+
/// something is wrong, not evidence to quietly skip.
30+
///
31+
/// That is a different fact from "this one file's own coverage happened
32+
/// to be empty" — a real export for an untested target/module, where
33+
/// nothing under `projectRoot` was executed, is not malformed at all
34+
/// (codex review: an earlier version of this method conflated the two,
35+
/// via `parse`'s own nil-means-either-one public contract, and one
36+
/// legitimately-empty export among several would silently discard every
37+
/// other file's real coverage too). `parseOutcome` keeps that
38+
/// distinction available internally; only `.malformed` fails the whole
39+
/// read, `.parsed` (even an empty one) always contributes.
40+
///
41+
/// - Returns: `nil` when no coverage files were found, every file's
42+
/// coverage was legitimately empty, or any one file was unreadable or
43+
/// malformed. The caller treats `nil` as "no coverage information",
44+
/// which is different from "the project has zero coverage": missing or
45+
/// corrupted data is missing data, not a measurement.
2346
public static func read(directory: URL, projectRoot: URL) -> CoverageMap? {
2447
let files = codecovFiles(in: directory)
2548
guard !files.isEmpty else { return nil }
2649

2750
var executed: [String: Set<Int>] = [:]
2851
for file in files {
29-
guard let data = try? Data(contentsOf: file),
30-
let parsed = parse(data, projectRoot: projectRoot) else { continue }
31-
for (path, lines) in parsed {
32-
executed[path, default: []].formUnion(lines)
52+
guard let data = try? Data(contentsOf: file) else { return nil }
53+
switch parseOutcome(data, projectRoot: projectRoot) {
54+
case .malformed:
55+
return nil
56+
case let .parsed(parsed):
57+
for (path, lines) in parsed {
58+
executed[path, default: []].formUnion(lines)
59+
}
3360
}
3461
}
3562

@@ -39,91 +66,257 @@ public enum SourceCoverageReader {
3966

4067
/// Parses a single JSON document. Exposed for tests so a fixture file can
4168
/// drive the reader without a directory walk.
69+
///
70+
/// A thin wrapper over `parseOutcome` that collapses `.malformed` and a
71+
/// legitimately-empty `.parsed([:])` into the same `nil` — the public
72+
/// contract this method has always had ("no coverage information",
73+
/// either because there was none to find or because reading it failed).
74+
/// `read(directory:)` needs the finer distinction `parseOutcome` keeps,
75+
/// specifically so a legitimately-empty file among several doesn't
76+
/// discard the others' real coverage; a single standalone document has
77+
/// no "others" to protect, so collapsing the two here is the same
78+
/// behavior this method has always documented.
4279
public static func parse(_ data: Data, projectRoot: URL) -> [String: Set<Int>]? {
80+
switch parseOutcome(data, projectRoot: projectRoot) {
81+
case .malformed:
82+
return nil
83+
case let .parsed(executed):
84+
return executed.isEmpty ? nil : executed
85+
}
86+
}
87+
88+
/// One coverage JSON document's own parse result, distinguishing
89+
/// genuine malformation from a validly-parsed (possibly empty) result —
90+
/// see `read(directory:)`'s own doc comment for why that distinction
91+
/// matters to a multi-file merge, and `parse`'s for why the public API
92+
/// collapses it.
93+
enum ParseOutcome: Equatable {
94+
case malformed
95+
case parsed([String: Set<Int>])
96+
}
97+
98+
/// Complete-or-nil (well, complete-or-`.malformed`) within one document:
99+
/// any structural element an LLVM detailed export always emits, but this
100+
/// document does not, invalidates the whole document — not just that one
101+
/// element's own entry. A mutation runner that silently dropped one
102+
/// covered file's real lines because its structure happened to be
103+
/// malformed would misreport those lines as uncovered
104+
/// (`CoverageMap.isKnownUncovered` treats "file present, line absent" as
105+
/// known-uncovered), which can fast-path a mutant on that line straight
106+
/// to `noCoverage` without ever building or testing it — the same
107+
/// false-negative class P12-B Finding D closed for per-test attribution.
108+
///
109+
/// Fails closed on (all per independent review, verified against LLVM's
110+
/// own exporter, `llvm/tools/llvm-cov/CoverageExporterJson.cpp`):
111+
/// - a top-level `type` other than `"llvm.coverage.json.export"`;
112+
/// - a `filename` missing or wrong-typed for any file entry;
113+
/// - a `segments` key missing or wrong-typed for a file entry *inside*
114+
/// `projectRoot` — a full/detailed export always includes this key for
115+
/// every in-scope file (even as `[]`, when the file genuinely has no
116+
/// regions); its absence is the shape `--summary-only` output takes,
117+
/// meaning detailed coverage was never captured at all, not that this
118+
/// file has zero coverage. A file entry *outside* `projectRoot` is
119+
/// unaffected either way — its own `segments` shape is never inspected,
120+
/// since `relativePath` already excludes it before this check runs,
121+
/// the same intentional exclusion as always (unrelated to evidence
122+
/// corruption).
123+
/// - a segment with a malformed field, including out-of-range `line`/
124+
/// `column`/`count` or `(line, column)` pairs that regress backwards
125+
/// across the segment list — LLVM's own exporter always emits segments
126+
/// in ascending `(line, column)` order per file, and `executedLines`'s
127+
/// own per-line grouping assumes that order (see `executedLines`'s
128+
/// own doc comment).
129+
///
130+
/// An empty `segments: []` array remains legitimate — a file genuinely
131+
/// contributing no regions, not corrupted data — and, like a file
132+
/// outside `projectRoot`, is skipped rather than failing the document.
133+
static func parseOutcome(_ data: Data, projectRoot: URL) -> ParseOutcome {
43134
guard let document = try? JSONSerialization.jsonObject(with: data) as? [String: Any],
44-
let export = document["data"] as? [[String: Any]] else { return nil }
135+
document["type"] as? String == "llvm.coverage.json.export",
136+
let export = document["data"] as? [[String: Any]] else { return .malformed }
45137

46138
let rootPath = projectRoot.standardizedFileURL.path
47139
var executed: [String: Set<Int>] = [:]
48140

49141
for module in export {
50-
guard let files = module["files"] as? [[String: Any]] else { continue }
142+
// A well-formed llvm-cov export always has a `files` array for
143+
// every module entry (even an empty one, `[]`) -- a module
144+
// missing this key entirely, or with the wrong type, is not "a
145+
// module with nothing to report," it is a malformed document
146+
// (codex review: `{"data":[{}]}` previously read as a valid,
147+
// empty export and could silently drop a sibling module's real
148+
// coverage for a file this malformed one also covered).
149+
guard let files = module["files"] as? [[String: Any]] else { return .malformed }
51150
for file in files {
52-
guard let absolutePath = file["filename"] as? String else { continue }
151+
// A well-formed export always names every file entry --
152+
// missing/wrong-typed is malformed, not "nothing to
153+
// report for an unnamed file" (there is no honest path
154+
// string to even consider skipping by).
155+
guard let absolutePath = file["filename"] as? String else { return .malformed }
53156
guard let relativePath = Self.relativePath(
54157
from: absolutePath, droppingPrefix: rootPath
55158
) else { continue }
56-
guard let segments = file["segments"] as? [[Any]] else { continue }
159+
// Only reached for a file *inside* projectRoot -- see this
160+
// method's own doc comment for why a missing/wrong-typed
161+
// `segments` here (unlike a merely-empty `[]`) is malformed.
162+
guard let segments = file["segments"] as? [[Any]] else { return .malformed }
57163

58-
let lines = Self.executedLines(from: segments)
164+
guard let lines = Self.executedLines(from: segments) else { return .malformed }
59165
guard !lines.isEmpty else { continue }
60166
executed[relativePath, default: []].formUnion(lines)
61167
}
62168
}
63169

64-
return executed.isEmpty ? nil : executed
170+
return .parsed(executed)
171+
}
172+
173+
/// One `[line, column, count, hasCount, isRegionEntry, isGapRegion]`
174+
/// segment from an LLVM source-based coverage export — see
175+
/// https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h,
176+
/// `struct CoverageSegment`. `isGapRegion` is absent in older export
177+
/// versions (the 5-element variant); defaults to `false`, matching LLVM's
178+
/// own `IsGapRegion` default. `column` is stored (not just parsed and
179+
/// discarded) purely so `executedLines` can validate real `(line,
180+
/// column)` ordering across the segment list — LLVM's own algorithm
181+
/// never uses it within one line's own stats computation.
182+
struct CoverageSegmentRecord {
183+
let line: Int
184+
let column: Int
185+
let count: Int
186+
let hasCount: Bool
187+
let isRegionEntry: Bool
188+
let isGapRegion: Bool
65189
}
66190

67191
/// Translates coverage segments into the set of lines that were executed.
68192
///
69-
/// A segment is `[line, column, count, hasCount, isRegionEntry, isGap]` in
70-
/// the LLVM Source-based Code Coverage export. A *region* begins at a
71-
/// segment with `isRegionEntry == true` and `isGap == false`, and extends
72-
/// to the position of the next segment (or end-of-file for the last one).
73-
/// A region whose `count > 0` was executed, so every line within its span
74-
/// is marked covered.
193+
/// A direct Swift port of LLVM's own `LineCoverageIterator`/
194+
/// `LineCoverageStats` (`llvm/lib/ProfileData/Coverage/CoverageMapping.cpp`)
195+
/// — not an independently-derived approximation. An earlier version of
196+
/// this reader tracked a single "currently open region" and closed it at
197+
/// the next segment's line, which drops any line whose own coverage comes
198+
/// not from a region *entry* but from an still-active region *carried
199+
/// forward* across a non-entry boundary segment (confirmed live: a
200+
/// same-line early return, and separately, the line immediately after a
201+
/// nested `if`-branch closes and control returns to the enclosing
202+
/// region's own count — both real, both silently misclassified as
203+
/// uncovered by the old single-open-region model). LLVM's real algorithm
204+
/// instead evaluates coverage per source line, carrying forward the last
205+
/// region-entry segment (`WrappedSegment`) across lines with no entry
206+
/// segment of their own, exactly reproduced below.
75207
///
76-
/// The 5-element variant (without `isGap`) is also handled: the sixth
77-
/// element is absent in older export versions.
78-
static func executedLines(from segments: [[Any]]) -> Set<Int> {
79-
// Walk segments in order. When a region entry with count > 0 appears,
80-
// all lines from its line to the next segment's line are covered. The
81-
// next segment can be anything — another entry, a gap, or a plain
82-
// boundary marker with `isRegionEntry == false`.
83-
var lines = Set<Int>()
84-
var open: (line: Int, count: Int)?
85-
208+
/// - Returns: `nil` when a segment array element is missing a required
209+
/// field, has the wrong type for one that is present, has an
210+
/// out-of-range `line`/`column`/`count` (LLVM's own coordinates are
211+
/// 1-based; a non-negative count only), or regresses the `(line,
212+
/// column)` ordering LLVM's own exporter always emits segments in —
213+
/// genuinely malformed data, distinct from an empty `segments` array
214+
/// (which is a legitimate "nothing here" and yields `[]`, not `nil`).
215+
static func executedLines(from segments: [[Any]]) -> Set<Int>? {
216+
var records: [CoverageSegmentRecord] = []
217+
records.reserveCapacity(segments.count)
86218
for segment in segments {
87-
guard let line = segment[safe: 0] as? Int else { continue }
88-
let count = segment[safe: 2] as? Int ?? 0
89-
let isRegionEntry = segment[safe: 4] as? Bool ?? false
90-
91-
// Close the currently-open region at this segment's line. The
92-
// region's own opening line always counts, even when the
93-
// closing segment sits on that identical line (a single-line
94-
// region — an early return or a one-statement branch body is
95-
// exactly this shape): the closing segment marks a later
96-
// *column* on the same line, which this line-granular reader
97-
// does not track, so `start.line ..< line` alone would collapse
98-
// to an empty range and silently drop the region's only line.
99-
// `max(line, start.line + 1)` guarantees at least `start.line`
100-
// itself is included, and is a no-op for the ordinary
101-
// multi-line case where `line` is already past `start.line`.
102-
if let start = open {
103-
if start.count > 0 {
104-
for l in start.line ..< max(line, start.line + 1) { lines.insert(l) }
105-
}
106-
open = nil
219+
guard let line = segment[safe: 0] as? Int,
220+
let column = segment[safe: 1] as? Int,
221+
let count = segment[safe: 2] as? Int,
222+
let hasCount = segment[safe: 3] as? Bool,
223+
let isRegionEntry = segment[safe: 4] as? Bool,
224+
line >= 1, column >= 1, count >= 0 else { return nil }
225+
// `isGapRegion` (index 5) is the one field genuinely absent in
226+
// older, still-supported exports, not merely malformed when
227+
// missing — defaults to `false` only when the element itself is
228+
// absent; a present-but-wrong-typed value still fails closed.
229+
let isGapRegion: Bool
230+
if segment.indices.contains(5) {
231+
guard let value = segment[safe: 5] as? Bool else { return nil }
232+
isGapRegion = value
233+
} else {
234+
isGapRegion = false
235+
}
236+
// LLVM's exporter always emits one file's segments in ascending
237+
// `(line, column)` order; `LineCoverageIterator`'s own per-line
238+
// grouping (below) assumes it. A regression means either a
239+
// corrupted export or an input this reader's ordering
240+
// assumption does not hold for -- fail closed rather than
241+
// silently mis-groups segments into the wrong line.
242+
if let previous = records.last,
243+
(line, column) < (previous.line, previous.column) {
244+
return nil
107245
}
246+
records.append(CoverageSegmentRecord(
247+
line: line, column: column, count: count, hasCount: hasCount,
248+
isRegionEntry: isRegionEntry, isGapRegion: isGapRegion
249+
))
250+
}
251+
guard let firstLine = records.first?.line, let lastLine = records.last?.line else { return [] }
108252

109-
// Open a new region only when this segment marks the entry.
110-
// Gap regions (isGap == true) are never opened — they have no
111-
// code to cover.
112-
if isRegionEntry {
113-
let isGap = segment[safe: 5] as? Bool ?? false
114-
if !isGap {
115-
open = (line, count)
116-
}
253+
var executed = Set<Int>()
254+
var wrapped: CoverageSegmentRecord?
255+
var index = 0
256+
var line = firstLine
257+
while line <= lastLine {
258+
var lineSegments: [CoverageSegmentRecord] = []
259+
while index < records.count, records[index].line == line {
260+
lineSegments.append(records[index])
261+
index += 1
262+
}
263+
264+
let stats = Self.lineCoverageStats(lineSegments: lineSegments, wrapped: wrapped)
265+
if stats.mapped, stats.executionCount > 0 {
266+
executed.insert(line)
117267
}
268+
269+
if let last = lineSegments.last {
270+
wrapped = last
271+
}
272+
line += 1
118273
}
119274

120-
// The last open region extends to its own line only — there is no
121-
// next segment to define its end.
122-
if let start = open, start.count > 0 {
123-
lines.insert(start.line)
275+
return executed
276+
}
277+
278+
/// `!isGapRegion && hasCount && isRegionEntry` — LLVM's own
279+
/// `isStartOfRegion` lambda, verbatim.
280+
private static func isStartOfRegion(_ segment: CoverageSegmentRecord) -> Bool {
281+
!segment.isGapRegion && segment.hasCount && segment.isRegionEntry
282+
}
283+
284+
private struct LineStats {
285+
let mapped: Bool
286+
let executionCount: Int
287+
}
288+
289+
/// A direct port of `LineCoverageStats::LineCoverageStats` — same
290+
/// variable names, same order, same early exits, so a future upstream
291+
/// change is a mechanical diff to re-apply rather than a re-derivation.
292+
private static func lineCoverageStats(
293+
lineSegments: [CoverageSegmentRecord],
294+
wrapped: CoverageSegmentRecord?
295+
) -> LineStats {
296+
// Find the minimum number of regions which start on this line.
297+
var minRegionCount = 0
298+
for segment in lineSegments {
299+
guard minRegionCount < 2 else { break }
300+
if isStartOfRegion(segment) { minRegionCount += 1 }
124301
}
125302

126-
return lines
303+
let startOfSkippedRegion = lineSegments.first.map { !$0.hasCount && $0.isRegionEntry } ?? false
304+
305+
var mapped = !startOfSkippedRegion && ((wrapped?.hasCount ?? false) || minRegionCount > 0)
306+
// If there is any starting segment at this line with a counter, it
307+
// must be mapped.
308+
mapped = mapped || lineSegments.contains { $0.isRegionEntry && $0.hasCount }
309+
310+
guard mapped else { return LineStats(mapped: false, executionCount: 0) }
311+
312+
// Pick the max count from the non-gap, region entry segments and the
313+
// wrapped count.
314+
var executionCount = wrapped?.count ?? 0
315+
guard minRegionCount > 0 else { return LineStats(mapped: true, executionCount: executionCount) }
316+
for segment in lineSegments where isStartOfRegion(segment) {
317+
executionCount = max(executionCount, segment.count)
318+
}
319+
return LineStats(mapped: true, executionCount: executionCount)
127320
}
128321

129322
/// Walks `directory` for coverage JSON files.

0 commit comments

Comments
 (0)