Fix crash when emitted diagnostics have incorrect source ranges #840
Conversation
rdar://122345738
| @@ -270,7 +270,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase { | |||
| let bundle = try XCTUnwrap(fs.bundles().first) | |||
| let baseURL = bundle.baseURL | |||
| let source = try XCTUnwrap(bundle.markupURLs.first) | |||
| let range = SourceLocation(line: 3, column: 18, source: source)..<SourceLocation(line: 3, column: 32, source: source) | |||
| let range = SourceLocation(line: 3, column: 18, source: source)..<SourceLocation(line: 3, column: 36, source: source) | |||
There was a problem hiding this comment.
I changed this test because it the range was specifying a byte in the middle of the emoji, which I don't consider to be a correct diagnostic range.
| return result | ||
| assert(highlightStart <= sourceLineUTF8.count, { | ||
| """ | ||
| Received diagnostic with incorrect source range; (\(range.lowerBound.column) ..< \(range.upperBound.column)) extends beyond the text on line \(lineNumber) (\(sourceLineUTF8.count) characters) |
There was a problem hiding this comment.
It's interesting when I tried the example from the Swift Book, this line number seems to be incorrect:
warning: '`' doesn't exist at '/The-Swift-Programming-Language/LexicalStructure'
--> TSPL.docc/ReferenceManual/LexicalStructure.md:142:145-142:146
140 | > Grammar of an identifier:
141 | >
142 + > *identifier* → *identifier-head* *identifier-characters*_?_ \
143 | > *identifier* → **`` ` ``** *identifier-head* *identifier-characters*_?_ **`` ` ``** \
144 | > *identifier* → *implicit-parameter-name* \
The stray backtick is on line 143, not 142. I wonder if this discrepancy has anything to do with the crash we were seeing before? Now that we have this assert here, we should take some time to find the root cause of the invalid diagnostics.
There was a problem hiding this comment.
Yes, I saw that as well.
The other diagnostics are on the correct line so it doesn't look like a systematic off-by-one error. We'll continue to investigate that separately.
|
@swift-ci please test |
Bug/issue #, if applicable: rdar://122345738
Summary
This updates the default diagnostics formatter to skip highlighting the line if the diagnostic range is incorrect.
Currently the default diagnostics formatter assumes that all the diagnostic ranges are correct and crashes—instead of printing any diagnostics—if they are not.
With these changes we'll also assert in debug builds if the diagnostic range is incorrect with extra information for us to investigate where the diagnostic with incorrect ranges originated from to help us fix the root cause of the incorrect diagnostic range.
Because of this assertion we can't add a test that the diagnostic isn't highlighted when the range is incorrect. I feel that the assertion—helping us find and fix the root cause of these issues—is more valuable than that test.
Dependencies
None.
Testing
This issue currently reproduces with the main branch of the Swift Programming Language documentation catalog.
git@github.com:apple/swift-book.gitswift run docc convert /path/to/swift-book/TSPL.doccThis should fail with an assertion, since it's a debug build.
swift run --configuration release docc convert /path/to/swift-book/TSPL.doccThis should succeed and print a few diagnostics, where not all of the diagnostics will highlight the range in the displayed lines of markup.
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded