Repository: apple/swift-docc (swift)

Issue:
Duplicated diagnostic causing crash on DefaultDiagnosticConsoleFormatter
### Description

See detail on https://github.com/apple/swift-docc/issues/678#issuecomment-1748170163

### Checklist

- [X] If possible, I've reproduced the issue using the `main` branch of this package.
- [X] This issue hasn't been addressed in an [existing GitHub issue](https://github.com/apple/swift-docc/issues).

### Expected Behavior

Only emit one diagnostic for the actual location - the Extension file.

### Actual behavior

Emit 2 duplicated diagnostic: the Extension Markdown file and the source Swift file

### Steps To Reproduce

Change `eat(_:quantity:)` to `eat(_:quantity:extraLog:)` and build documentation for SlothCreator.

### Swift-DocC Version Information

5.9.0

### Swift Compiler Version Information

```shell
swift-driver version: 1.87.1 Apple Swift version 5.9 (swiftlang-5.9.0.128.108 clang-1500.0.40.1)
Target: arm64-apple-macosx14.0
```

Pull request:
Bug/issue #, if applicable: #729

## Summary

This improved the input validation of the default diagnostic formatter so that incorrectly created diagnostics with out-of-bounds range information won't cause a crash.

## Dependencies

None.

## Testing

Since this needs an incorrectly created diagnostic, it requires a bug that results in out-of-bounds diagnostics to test. 

[This comment](https://github.com/apple/swift-docc/issues/729#issuecomment-2119101213) includes a package that reproduces on such bug. Building it should no longer crash (although it will output a duplicate diagnostic, because _that_ issue is not fixed in this PR).

## Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

- [x] Added tests
- [x] Ran the `./bin/test` script and it succeeded
- [x] ~Updated documentation if necessary~ Not applicable

Patch:
diff --git a/Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticConsoleWriter.swift b/Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticConsoleWriter.swift
index 64da2ab85b..b4d069fc62 100644
--- a/Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticConsoleWriter.swift
+++ b/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, {


Test patch:
diff --git a/Tests/SwiftDocCTests/Diagnostics/DiagnosticConsoleWriterDefaultFormattingTest.swift b/Tests/SwiftDocCTests/Diagnostics/DiagnosticConsoleWriterDefaultFormattingTest.swift
index 5c44ed29c3..4e69eebaa1 100644
--- a/Tests/SwiftDocCTests/Diagnostics/DiagnosticConsoleWriterDefaultFormattingTest.swift
+++ b/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: [