Handle various out-of-bounds diagnostic ranges in default diagnostics formatter (#947) (#948)

Author: d-ronnqvist

Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticConsoleWriter.swift

@@ -343,19 +343,18 @@ extension DefaultDiagnosticConsoleFormatter {
 
         let sourceLines = readSourceLines(url)
 
-        guard !sourceLines.isEmpty
-        else {
-            return "\n--> \(formattedSourcePath(url)):\(diagnosticRange.lowerBound.line):\(diagnosticRange.lowerBound.column)-\(diagnosticRange.upperBound.line):\(diagnosticRange.upperBound.column)"
+        guard sourceLines.indices.contains(diagnosticRange.lowerBound.line - 1), sourceLines.indices.contains(diagnosticRange.upperBound.line - 1) else {
+            return "\n--> \(formattedSourcePath(url)):\(max(1, diagnosticRange.lowerBound.line)):\(max(1, diagnosticRange.lowerBound.column))-\(max(1, diagnosticRange.upperBound.line)):\(max(1, diagnosticRange.upperBound.column))"
         }
 
         // A range containing the source lines and some surrounding context.
-        let sourceRange = Range(
+        let sourceLinesToDisplay = Range(
             uncheckedBounds: (
-                lower: max(1, diagnosticRange.lowerBound.line - Self.contextSize) - 1,
-                upper: min(sourceLines.count, diagnosticRange.upperBound.line + Self.contextSize)
+                lower: diagnosticRange.lowerBound.line - Self.contextSize - 1,
+                upper: diagnosticRange.upperBound.line + Self.contextSize
             )
-        )
-        let maxLinePrefixWidth = String(sourceRange.upperBound).count
+        ).clamped(to: sourceLines.indices)
+        let maxLinePrefixWidth = String(sourceLinesToDisplay.upperBound).count
 
         var suggestionsPerLocation = [SourceLocation: [String]]()
         for solution in solutions {
@@ -377,11 +376,10 @@ extension DefaultDiagnosticConsoleFormatter {
         // Example:
         //   --> /path/to/file.md:1:10-2:20
         result.append("\n\(String(repeating: " ", count: maxLinePrefixWidth))--> ")
-        result.append(        "\(formattedSourcePath(url)):\(diagnosticRange.lowerBound.line):\(diagnosticRange.lowerBound.column)-\(diagnosticRange.upperBound.line):\(diagnosticRange.upperBound.column)"
-        )
+        result.append(        "\(formattedSourcePath(url)):\(max(1, diagnosticRange.lowerBound.line)):\(max(1, diagnosticRange.lowerBound.column))-\(max(1, diagnosticRange.upperBound.line)):\(max(1, diagnosticRange.upperBound.column))")
 
-        for (sourceLineIndex, sourceLine) in sourceLines[sourceRange].enumerated() {
-            let lineNumber = sourceLineIndex + sourceRange.lowerBound + 1
+        for (sourceLineIndex, sourceLine) in sourceLines[sourceLinesToDisplay].enumerated() {
+            let lineNumber = sourceLineIndex + sourceLinesToDisplay.lowerBound + 1
             let linePrefix = "\(lineNumber)".padding(toLength: maxLinePrefixWidth, withPad: " ", startingAt: 0)
 
             let highlightedSource = highlightSource(
@@ -483,7 +481,7 @@ extension DefaultDiagnosticConsoleFormatter {
 
         let sourceLineUTF8 = sourceLine.utf8
 
-        let highlightStart = range.lowerBound.column - 1
+        let highlightStart = max(0, range.lowerBound.column - 1)
         let highlightEnd = range.upperBound.column - 1
 
         assert(highlightStart <= sourceLineUTF8.count, {

Tests/SwiftDocCTests/Diagnostics/DiagnosticConsoleWriterDefaultFormattingTest.swift

@@ -390,6 +390,108 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
         }
     }
 
+    func testClampsDiagnosticRangeToSourceRange() throws {
+        let fs = try TestFileSystem(folders: [
+            Folder(name: "Something.docc", content: [
+                TextFile(name: "Article.md", utf8Content: """
+                # Title
+                
+                A very short article with only an abstract.
+                """)
+            ])
+        ])
+        
+        let summary = "Test diagnostic summary"
+        let explanation = "Test diagnostic explanation."
+        
+        let bundle = try XCTUnwrap(fs.bundles().first)
+        let baseURL = bundle.baseURL
+        let source = try XCTUnwrap(bundle.markupURLs.first)
+        
+        typealias Location = (line: Int, column: Int)
+        func logMessageFor(start: Location, end: Location) throws -> String {
+            let range = SourceLocation(line: start.line, column: start.column, source: source)..<SourceLocation(line: end.line, column: end.column, source: source)
+            
+            let logStorage = LogHandle.LogStorage()
+            let consumer = DiagnosticConsoleWriter(LogHandle.memory(logStorage), baseURL: baseURL, highlight: true, fileManager: fs)
+            
+            let diagnostic = Diagnostic(source: source, severity: .warning, range: range, identifier: "org.swift.docc.test-identifier", summary: summary, explanation: explanation)
+            consumer.receive([Problem(diagnostic: diagnostic, possibleSolutions: [])])
+            try consumer.flush()
+            
+            // There are no lines before line 1
+            return logStorage.text
+        }
+        
+        // Highlight the "Title" word on line 1
+        XCTAssertEqual(try logMessageFor(start: (line: 1, column: 3), end: (line: 1, column: 8)), """
+            \u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
+            \(explanation)
+             --> Something.docc/Article.md:1:3-1:8
+            1 + # \u{001B}[1;32mTitle\u{001B}[0;0m
+            2 |
+            3 | A very short article with only an abstract.
+            """)
+                       
+        // Highlight the "short" word on line 3
+        XCTAssertEqual(try logMessageFor(start: (line: 3, column: 8), end: (line: 3, column: 13)), """
+            \u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
+            \(explanation)
+             --> Something.docc/Article.md:3:8-3:13
+            1 | # Title
+            2 |
+            3 + A very \u{001B}[1;32mshort\u{001B}[0;0m article with only an abstract.
+            """)
+        
+        // Extend the highlight beyond the end of that line
+        XCTAssertEqual(try logMessageFor(start: (line: 3, column: 8), end: (line: 3, column: 100)), """
+            \u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
+            \(explanation)
+             --> Something.docc/Article.md:3:8-3:100
+            1 | # Title
+            2 |
+            3 + A very \u{001B}[1;32mshort article with only an abstract.\u{001B}[0;0m
+            """)
+        
+        // Extend the highlight beyond the start of that line
+        XCTAssertEqual(try logMessageFor(start: (line: 3, column: -4), end: (line: 3, column: 13)), """
+            \u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
+            \(explanation)
+             --> Something.docc/Article.md:3:1-3:13
+            1 | # Title
+            2 |
+            3 + \u{001B}[1;32mA very short\u{001B}[0;0m article with only an abstract.
+            """)
+        
+        // Highlight a line before the start of the file
+        XCTAssertEqual(try logMessageFor(start: (line: -4, column: 1), end: (line: -4, column: 5)), """
+            \u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
+            \(explanation)
+            --> Something.docc/Article.md:1:1-1:5
+            """)
+        
+        // Highlight a line after the end of the file
+        XCTAssertEqual(try logMessageFor(start: (line: 100, column: 1), end: (line: 100, column: 5)), """
+            \u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
+            \(explanation)
+            --> Something.docc/Article.md:100:1-100:5
+            """)
+        
+        // Extended the highlighted lines before the start of the file
+        XCTAssertEqual(try logMessageFor(start: (line: -4, column: 1), end: (line: 1, column: 5)), """
+            \u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
+            \(explanation)
+            --> Something.docc/Article.md:1:1-1:5
+            """)
+        
+        // Extended the highlighted lines after the end of the file
+        XCTAssertEqual(try logMessageFor(start: (line: 1, column: 1), end: (line: 100, column: 5)), """
+            \u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
+            \(explanation)
+            --> Something.docc/Article.md:1:1-100:5
+            """)
+    }
+    
     func testEmitAdditionReplacementSolution() throws {
         func problemsLoggerOutput(possibleSolutions: [Solution]) -> String {
             let logger = Logger()
@@ -399,8 +501,8 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
             try? consumer.flush()
             return logger.output
         }
-        let sourcelocation = SourceLocation(line: 1, column: 1, source: nil)
-        let range = sourcelocation..<sourcelocation
+        let sourceLocation = SourceLocation(line: 1, column: 1, source: nil)
+        let range = sourceLocation..<sourceLocation
         XCTAssertEqual(
             problemsLoggerOutput(possibleSolutions: [
                 Solution(summary: "Create a sloth.", replacements: [