fix: keep source-context echo out of diagnostics - #87
Merged
Conversation
A `file:line:col: error:/warning:/note:` header is followed by the offending source line, indented, and a caret line. The echoed source can carry `: error: ` inside a string literal or a comment. The parser read those bytes as a build error, so a successful build got the `failed` verdict. Track the echo block as parser state. The block opens on a diagnostic header that carries a location. It closes on the caret line, or on the next line without indentation. Error and warning parsing is skipped only inside the block. Indentation alone must never suppress a diagnostic. Indented tool output such as `swiftgen: error: template not found` and an indented `Command PhaseScriptExecution failed with a nonzero exit code` stay reportable. Fixes #78.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #78.
Problem
The compiler prints the offending source line, indented, under every
file:line:col: error:/warning:/note:header, then a caret line. The echoed source can carry: error:inside a string literal or a comment.parseErrorread those bytes as a compile error, so a successful build got thefailedverdict.The repro in the issue does not reproduce, on 1.4.2 or on the 1.3.1 code. The parser searches for
": error: ", andpublic init(_ error: Error) {holds_ error:— no colon before the word. The fast-path marker is wider ("error:"), so the line does reach the error path, butparseErrorrejects it. The reported failure needs the echoed line to hold the full": error: "byte sequence.Real
swift buildoutput that reproduces it:Log messages with a prefix such as
"upload: error: "are common, so a large codebase hits this often.print("...: error: \(x)")escapes the bug only by accident:isJSONLikeLineneeds both a backslash and a quote on the line.Why not an indentation test
The issue proposes a guard that drops an indented line unless it holds
": error: ". That guard is a no-op for the failure above, because the echoed line does hold": error: ". It also drops real errors:Command PhaseScriptExecution failed with a nonzero exit code/p/File.swift:3: Fatal error: found nilBoth regressions report a broken build as a successful one, which #73 closed. Indentation cannot decide this question: indented tool output stands on its own.
Change
Track the echo block as parser state.
sourceContextOpenopens on a diagnostic header that carries a location, and closes on the caret line or on the next line without indentation. Error and warning parsing is skipped only inside the block.error.line != nil/warning.line != nil), so the hot path never scans a line twice.note:opens a block too. No other parser reads those lines, so the marker gets its own candidate bit and one byte-exact search.LineCandidates.rawValuewidens toUInt16for bit 8.isCaretLineruns only while a block is open, and rejects on the last byte first.swiftgen: error: …carries no location, so it never opens a block and never hides the next line.: error:under awarning:header: error:under anote:header: warning:under anerror:headerswiftgen: error: template not foundsourcery: error: could not parseCommand PhaseScriptExecution failed with a nonzero exit code/p/File.swift:3: Fatal error: found nil/p/File.swift:10:5: error: cannot find xAn indented line inside the block is never a diagnostic. Only interleaved output from parallel targets could place a real error there, and
xcodebuildbuffers output per task. The earlier failure mode was worse: a phantom error and a wrong verdict.The new gutter diagnostic style of Swift 5.8+ was never affected, because every context line holds
|. The caret style is whatxcodebuildemits — the 2.7 MBbuild.txtfixture holds 30 caret lines and no gutter lines.Verification
swift test— 452 tests, 0 failures (444 before).Tests/XCSiftCoreTests/SourceContextEchoTests.swift— 8 tests. Five fail onmasterand cover the bug. Three pass onmasterand guard the indented tool output, so they fail on the proposed indentation guard.swift format lint --strict --recursive .— clean.Benchmarks/large-log.sh 100, medians of three to five runs:fast-reject0.26 s → 0.26 s,fixture-mixed0.24 s → 0.24 s,warning-duplicate1.11 s → 1.11 s,video-go-shaped1.43 s → 1.41 s.swift buildlog above now reportssuccesswith four warnings and no errors.