Unify logging of errors during position conversions

Instead of logging errors in position translation ad-hoc at the caller’s side (and ofter forgetting to do so), log these errors in `LineTable`. To be able to debug where the position conversion error is coming from, also log the file name and line number of the caller.

rdar://125545620

Author: ahoppen

Sources/SKSupport/LineTable.swift

@@ -10,6 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+import LSPLogging
+
 public struct LineTable: Hashable {
   @usableFromInline
   var impl: [String.Index]
@@ -123,113 +125,242 @@ extension LineTable {
   }
 }
 
-extension LineTable {
+// MARK: - Position translation
 
-  // MARK: - Position translation
+extension LineTable {
+  // MARK: line:column <-> String.Index
 
-  /// Returns `String.Index` of given logical position.
+  /// Converts the given UTF-16-based `line:column`` position to a `String.Index`.
+  ///
+  /// If the position does not refer to a valid position with in the source file, returns `nil` and logs a fault
+  /// containing the file and line of the caller (from `callerFile` and `callerLine`).
   ///
   /// - parameter line: Line number (zero-based).
   /// - parameter utf16Column: UTF-16 column offset (zero-based).
   @inlinable
-  public func stringIndexOf(line: Int, utf16Column: Int) -> String.Index? {
+  public func stringIndexOf(
+    line: Int,
+    utf16Column: Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
+  ) -> String.Index? {
     guard line < count else {
-      // Line out of range.
+      logger.fault(
+        """
+        Unable to get string index for \(line):\(utf16Column) (UTF-16) because line is out of range \
+        (\(callerFile, privacy: .public):\(callerLine, privacy: .public))
+        """
+      )
       return nil
     }
     let lineSlice = self[line]
-    return content.utf16.index(lineSlice.startIndex, offsetBy: utf16Column, limitedBy: lineSlice.endIndex)
+    guard let index = content.utf16.index(lineSlice.startIndex, offsetBy: utf16Column, limitedBy: lineSlice.endIndex)
+    else {
+      logger.fault(
+        """
+        Unable to get string index for \(line):\(utf16Column) (UTF-16) because column is out of range \
+        (\(callerFile, privacy: .public):\(callerLine, privacy: .public))
+        """
+      )
+      return nil
+    }
+    return index
   }
 
-  /// Returns `String.Index` of given logical position.
+  /// Converts the given UTF-8-based `line:column`` position to a `String.Index`.
+  ///
+  /// If the position does not refer to a valid position with in the source file, returns `nil` and logs a fault
+  /// containing the file and line of the caller (from `callerFile` and `callerLine`).
   ///
   /// - parameter line: Line number (zero-based).
   /// - parameter utf8Column: UTF-8 column offset (zero-based).
   @inlinable
-  public func stringIndexOf(line: Int, utf8Column: Int) -> String.Index? {
+  public func stringIndexOf(
+    line: Int,
+    utf8Column: Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
+  ) -> String.Index? {
     guard 0 <= line, line < count else {
-      // Line out of range.
+      logger.fault(
+        """
+        Unable to get string index for \(line):\(utf8Column) (UTF-8) because line is out of range \
+        (\(callerFile, privacy: .public):\(callerLine, privacy: .public))
+        """
+      )
       return nil
     }
     guard 0 <= utf8Column else {
-      // Column out of range.
+      logger.fault(
+        """
+        Unable to get string index for \(line):\(utf8Column) (UTF-8) because column is out of range \
+        (\(callerFile, privacy: .public):\(callerLine, privacy: .public))
+        """
+      )
       return nil
     }
     let lineSlice = self[line]
     return content.utf8.index(lineSlice.startIndex, offsetBy: utf8Column, limitedBy: lineSlice.endIndex)
   }
 
-  /// Returns UTF8 buffer offset of given logical position.
+  // MARK: line:column <-> UTF-8 offset
+
+  /// Converts the given UTF-16-based `line:column`` position to a UTF-8 offset within the source file.
+  ///
+  /// If the position does not refer to a valid position with in the source file, returns `nil` and logs a fault
+  /// containing the file and line of the caller (from `callerFile` and `callerLine`).
   ///
   /// - parameter line: Line number (zero-based).
   /// - parameter utf16Column: UTF-16 column offset (zero-based).
   @inlinable
-  public func utf8OffsetOf(line: Int, utf16Column: Int) -> Int? {
-    guard let stringIndex = stringIndexOf(line: line, utf16Column: utf16Column) else {
+  public func utf8OffsetOf(
+    line: Int,
+    utf16Column: Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
+  ) -> Int? {
+    guard
+      let stringIndex = stringIndexOf(
+        line: line,
+        utf16Column: utf16Column,
+        callerFile: callerFile,
+        callerLine: callerLine
+      )
+    else {
       return nil
     }
     return content.utf8.distance(from: content.startIndex, to: stringIndex)
   }
 
-  /// Returns UTF8 buffer offset of given logical position.
+  /// Converts the given UTF-8-based `line:column`` position to a UTF-8 offset within the source file.
+  ///
+  /// If the position does not refer to a valid position with in the source file, returns `nil` and logs a fault
+  /// containing the file and line of the caller (from `callerFile` and `callerLine`).
   ///
   /// - parameter line: Line number (zero-based).
   /// - parameter utf8Column: UTF-8 column offset (zero-based).
   @inlinable
-  public func utf8OffsetOf(line: Int, utf8Column: Int) -> Int? {
-    guard let stringIndex = stringIndexOf(line: line, utf8Column: utf8Column) else {
+  public func utf8OffsetOf(
+    line: Int,
+    utf8Column: Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
+  ) -> Int? {
+    guard
+      let stringIndex = stringIndexOf(
+        line: line,
+        utf8Column: utf8Column,
+        callerFile: callerFile,
+        callerLine: callerLine
+      )
+    else {
       return nil
     }
     return content.utf8.distance(from: content.startIndex, to: stringIndex)
   }
 
-  /// Returns logical position of given source offset.
+  /// Converts the given UTF-16-based line:column position to the UTF-8 offset of that position within the source file.
+  ///
+  /// If the position does not refer to a valid position with in the snapshot, returns `nil` and logs a fault
+  /// containing the file and line of the caller (from `callerFile` and `callerLine`).
   ///
   /// - parameter utf8Offset: UTF-8 buffer offset (zero-based).
   @inlinable
-  public func lineAndUTF16ColumnOf(utf8Offset: Int) -> (line: Int, utf16Column: Int)? {
+  public func lineAndUTF16ColumnOf(
+    utf8Offset: Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
+  ) -> (line: Int, utf16Column: Int)? {
     guard utf8Offset <= content.utf8.count else {
-      // Offset ouf of range.
+      logger.fault(
+        """
+        Unable to get line and UTF-16 column for UTF-8 offset \(utf8Offset) because offset is out of range \
+        (\(callerFile, privacy: .public):\(callerLine, privacy: .public))
+        """
+      )
       return nil
     }
     return lineAndUTF16ColumnOf(content.utf8.index(content.startIndex, offsetBy: utf8Offset))
   }
 
-  @inlinable func lineAndUTF8ColumnOf(utf8Offset: Int) -> (line: Int, utf8Column: Int)? {
-    guard let (line, utf16Column) = lineAndUTF16ColumnOf(utf8Offset: utf8Offset) else {
+  /// Converts the given UTF-8-based line:column position to the UTF-8 offset of that position within the source file.
+  ///
+  /// If the position does not refer to a valid position with in the snapshot, returns `nil` and logs a fault
+  /// containing the file and line of the caller (from `callerFile` and `callerLine`).
+  @inlinable func lineAndUTF8ColumnOf(
+    utf8Offset: Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
+  ) -> (line: Int, utf8Column: Int)? {
+    guard
+      let (line, utf16Column) = lineAndUTF16ColumnOf(
+        utf8Offset: utf8Offset,
+        callerFile: callerFile,
+        callerLine: callerLine
+      )
+    else {
       return nil
     }
-    guard let utf8Column = utf8ColumnAt(line: line, utf16Column: utf16Column) else {
+    guard
+      let utf8Column = utf8ColumnAt(
+        line: line,
+        utf16Column: utf16Column,
+        callerFile: callerFile,
+        callerLine: callerLine
+      )
+    else {
       return nil
     }
     return (line, utf8Column)
   }
 
-  /// Returns UTF16 column offset at UTF8 version of logical position.
+  // MARK: UTF-8 line:column <-> UTF-16 line:column
+
+  /// Returns UTF-16 column offset at UTF-8 based `line:column` position.
+  ///
+  /// If the position does not refer to a valid position with in the snapshot, returns `nil` and logs a fault
+  /// containing the file and line of the caller (from `callerFile` and `callerLine`).
   ///
   /// - parameter line: Line number (zero-based).
   /// - parameter utf8Column: UTF-8 column offset (zero-based).
   @inlinable
-  public func utf16ColumnAt(line: Int, utf8Column: Int) -> Int? {
+  public func utf16ColumnAt(
+    line: Int,
+    utf8Column: Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
+  ) -> Int? {
     return convertColumn(
       line: line,
       column: utf8Column,
       indexFunction: content.utf8.index(_:offsetBy:limitedBy:),
-      distanceFunction: content.utf16.distance(from:to:)
+      distanceFunction: content.utf16.distance(from:to:),
+      callerFile: callerFile,
+      callerLine: callerLine
     )
   }
 
-  /// Returns UTF8 column offset at UTF16 version of logical position.
+  /// Returns UTF-8 column offset at UTF-16 based `line:column` position.
+  ///
+  /// If the position does not refer to a valid position with in the snapshot, returns `nil` and logs a fault
+  /// containing the file and line of the caller (from `callerFile` and `callerLine`).
   ///
   /// - parameter line: Line number (zero-based).
   /// - parameter utf16Column: UTF-16 column offset (zero-based).
   @inlinable
-  public func utf8ColumnAt(line: Int, utf16Column: Int) -> Int? {
+  public func utf8ColumnAt(
+    line: Int,
+    utf16Column: Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
+  ) -> Int? {
     return convertColumn(
       line: line,
       column: utf16Column,
       indexFunction: content.utf16.index(_:offsetBy:limitedBy:),
-      distanceFunction: content.utf8.distance(from:to:)
+      distanceFunction: content.utf8.distance(from:to:),
+      callerFile: callerFile,
+      callerLine: callerLine
     )
   }
 
@@ -238,15 +369,27 @@ extension LineTable {
     line: Int,
     column: Int,
     indexFunction: (Substring.Index, Int, Substring.Index) -> Substring.Index?,
-    distanceFunction: (Substring.Index, Substring.Index) -> Int
+    distanceFunction: (Substring.Index, Substring.Index) -> Int,
+    callerFile: StaticString = #fileID,
+    callerLine: UInt = #line
   ) -> Int? {
     guard line < count else {
-      // Line out of range.
+      logger.fault(
+        """
+        Unable to convert column of \(line):\(column) because line is out of range \
+        (\(callerFile, privacy: .public):\(callerLine, privacy: .public))
+        """
+      )
       return nil
     }
     let lineSlice = self[line]
     guard let targetIndex = indexFunction(lineSlice.startIndex, column, lineSlice.endIndex) else {
-      // Column out of range
+      logger.fault(
+        """
+        Unable to convert column of \(line):\(column) because column is out of range \
+        (\(callerFile, privacy: .public):\(callerLine, privacy: .public))
+        """
+      )
       return nil
     }
     return distanceFunction(lineSlice.startIndex, targetIndex)

Sources/SourceKitLSP/DocumentManager.swift

@@ -57,10 +57,6 @@ public struct DocumentSnapshot: Identifiable {
     self.language = language
     self.lineTable = lineTable
   }
-
-  func index(of pos: Position) -> String.Index? {
-    return lineTable.stringIndexOf(line: pos.line, utf16Column: pos.utf16index)
-  }
 }
 
 public final class Document {

Sources/SourceKitLSP/Rename.swift

@@ -266,9 +266,24 @@ fileprivate struct SyntacticRenameName {
 }
 
 private extension LineTable {
-  subscript(range: Range<Position>) -> Substring? {
-    guard let start = self.stringIndexOf(line: range.lowerBound.line, utf16Column: range.lowerBound.utf16index),
-      let end = self.stringIndexOf(line: range.upperBound.line, utf16Column: range.upperBound.utf16index)
+  /// Returns the string in the source file that's with the given position range.
+  ///
+  /// If either the lower or upper bound of `range` do not refer to valid positions with in the snapshot, returns
+  /// `nil` and logs a fault containing the file and line of the caller (from `callerFile` and `callerLine`).
+  subscript(range: Range<Position>, callerFile: StaticString = #fileID, callerLine: UInt = #line) -> Substring? {
+    guard
+      let start = self.stringIndexOf(
+        line: range.lowerBound.line,
+        utf16Column: range.lowerBound.utf16index,
+        callerFile: callerFile,
+        callerLine: callerLine
+      ),
+      let end = self.stringIndexOf(
+        line: range.upperBound.line,
+        utf16Column: range.upperBound.utf16index,
+        callerFile: callerFile,
+        callerLine: callerLine
+      )
     else {
       return nil
     }
@@ -1092,7 +1107,6 @@ extension SwiftLanguageService {
     newName: CrossLanguageName
   ) async -> [TextEdit] {
     guard let position = snapshot.absolutePosition(of: renameLocation) else {
-      logger.fault("Failed to convert \(renameLocation.line):\(renameLocation.utf8Column) to AbsolutePosition")
       return []
     }
     let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
@@ -1155,7 +1169,6 @@ extension SwiftLanguageService {
       )
 
       guard let parameterPosition = snapshot.position(of: parameter.positionAfterSkippingLeadingTrivia) else {
-        logger.fault("Failed to convert position of \(parameter.firstName.text) to line-column")
         continue
       }
 
@@ -1447,7 +1460,6 @@ fileprivate extension RelatedIdentifiersResponse {
       let position = relatedIdentifier.range.lowerBound
       guard let utf8Column = snapshot.lineTable.utf8ColumnAt(line: position.line, utf16Column: position.utf16index)
       else {
-        logger.fault("Unable to find UTF-8 column for \(position.description, privacy: .public)")
         return nil
       }
       return RenameLocation(line: position.line + 1, utf8Column: utf8Column + 1, usage: relatedIdentifier.usage)

Sources/SourceKitLSP/Swift/CodeCompletion.swift

@@ -23,9 +23,6 @@ extension SwiftLanguageService {
     let completionPos = await adjustPositionToStartOfIdentifier(req.position, in: snapshot)
 
     guard let offset = snapshot.utf8Offset(of: completionPos) else {
-      logger.error(
-        "invalid completion position \(req.position, privacy: .public) (adjusted: \(completionPos, privacy: .public)"
-      )
       return CompletionList(isIncomplete: true, items: [])
     }
 
@@ -34,7 +31,6 @@ extension SwiftLanguageService {
     guard let start = snapshot.indexOf(utf8Offset: offset),
       let end = snapshot.index(of: req.position)
     else {
-      logger.error("invalid completion position \(req.position, privacy: .public)")
       return CompletionList(isIncomplete: true, items: [])
     }