Merge pull request #1073 from ahoppen/ahoppen/adjust-completion-pos-using-swiftsyntax

Find completion start position using swift-syntax instead of ASCII-based implementation

Author: ahoppen

Sources/SourceKitLSP/CMakeLists.txt

@@ -17,6 +17,7 @@ add_library(SourceKitLSP STATIC
 target_sources(SourceKitLSP PRIVATE
   Clang/ClangLanguageServer.swift)
 target_sources(SourceKitLSP PRIVATE
+  Swift/AdjustPositionToStartOfIdentifier.swift
   Swift/CodeCompletion.swift
   Swift/CodeCompletionSession.swift
   Swift/CommentXML.swift

Sources/SourceKitLSP/Swift/AdjustPositionToStartOfIdentifier.swift

@@ -0,0 +1,67 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import LanguageServerProtocol
+import SwiftSyntax
+
+fileprivate class StartOfIdentifierFinder: SyntaxAnyVisitor {
+  let requestedPosition: AbsolutePosition
+  var resolvedPosition: AbsolutePosition?
+
+  init(position: AbsolutePosition) {
+    self.requestedPosition = position
+    super.init(viewMode: .sourceAccurate)
+  }
+
+  override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
+    if (node.position...node.endPosition).contains(requestedPosition) {
+      return .visitChildren
+    }
+    return .skipChildren
+  }
+
+  override func visit(_ token: TokenSyntax) -> SyntaxVisitorContinueKind {
+    if token.tokenKind.isPunctuation || token.tokenKind == .endOfFile {
+      return .skipChildren
+    }
+    if (token.positionAfterSkippingLeadingTrivia...token.endPositionBeforeTrailingTrivia).contains(requestedPosition) {
+      self.resolvedPosition = token.positionAfterSkippingLeadingTrivia
+    }
+    return .skipChildren
+  }
+}
+
+extension SwiftLanguageServer {
+  /// VS Code considers the position after an identifier as part of an identifier. Ie. if you have `let foo| = 1`, then
+  /// it considers the cursor to be positioned at the identifier. This scenario is hit, when selecting an identifier by
+  /// double-clicking it and then eg. performing jump-to-definition. In that case VS Code will send the position after
+  /// the identifier.
+  /// `sourcekitd`, on the other hand, does not consider the position after the identifier as part of the identifier.
+  /// To bridge the gap here, normalize any positions inside, or directly after, an identifier to the identifier's
+  /// start.
+  func adjustPositionToStartOfIdentifier(
+    _ position: Position,
+    in snapshot: DocumentSnapshot
+  ) async -> Position {
+    let tree = await self.syntaxTreeManager.syntaxTree(for: snapshot)
+    guard let swiftSyntaxPosition = snapshot.position(of: position) else {
+      return position
+    }
+    let visitor = StartOfIdentifierFinder(position: swiftSyntaxPosition)
+    visitor.walk(tree)
+    if let resolvedPosition = visitor.resolvedPosition {
+      return snapshot.position(of: resolvedPosition) ?? position
+    }
+    return position
+  }
+
+}

Sources/SourceKitLSP/Swift/CodeCompletion.swift

@@ -20,10 +20,7 @@ extension SwiftLanguageServer {
   public func completion(_ req: CompletionRequest) async throws -> CompletionList {
     let snapshot = try documentManager.latestSnapshot(req.textDocument.uri)
 
-    guard let completionPos = adjustCompletionLocation(req.position, in: snapshot) else {
-      logger.error("invalid completion position \(req.position, privacy: .public)")
-      return CompletionList(isIncomplete: true, items: [])
-    }
+    let completionPos = await adjustPositionToStartOfIdentifier(req.position, in: snapshot)
 
     guard let offset = snapshot.utf8Offset(of: completionPos) else {
       logger.error(
@@ -58,35 +55,4 @@ extension SwiftLanguageServer {
       filterText: filterText
     )
   }
-
-  /// Adjust completion position to the start of identifier characters.
-  private func adjustCompletionLocation(_ pos: Position, in snapshot: DocumentSnapshot) -> Position? {
-    guard pos.line < snapshot.lineTable.count else {
-      // Line out of range.
-      return nil
-    }
-    let lineSlice = snapshot.lineTable[pos.line]
-    let startIndex = lineSlice.startIndex
-
-    let identifierChars = CharacterSet.alphanumerics.union(CharacterSet(charactersIn: "_"))
-
-    guard var loc = lineSlice.utf16.index(startIndex, offsetBy: pos.utf16index, limitedBy: lineSlice.endIndex) else {
-      // Column out of range.
-      return nil
-    }
-    while loc != startIndex {
-      let prev = lineSlice.index(before: loc)
-      if !identifierChars.contains(lineSlice.unicodeScalars[prev]) {
-        break
-      }
-      loc = prev
-    }
-
-    // ###aabccccccdddddd
-    // ^  ^- loc  ^-requestedLoc
-    // `- startIndex
-
-    let adjustedOffset = lineSlice.utf16.distance(from: startIndex, to: loc)
-    return Position(line: pos.line, utf16index: adjustedOffset)
-  }
 }

Sources/SourceKitLSP/Swift/SymbolInfo.swift

@@ -11,61 +11,8 @@
 //===----------------------------------------------------------------------===//
 
 import LanguageServerProtocol
-import SwiftSyntax
-
-fileprivate class StartOfIdentifierFinder: SyntaxAnyVisitor {
-  let requestedPosition: AbsolutePosition
-  var resolvedPosition: AbsolutePosition?
-
-  init(position: AbsolutePosition) {
-    self.requestedPosition = position
-    super.init(viewMode: .sourceAccurate)
-  }
-
-  override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
-    if (node.position...node.endPosition).contains(requestedPosition) {
-      return .visitChildren
-    } else {
-      return .skipChildren
-    }
-  }
-
-  override func visit(_ token: TokenSyntax) -> SyntaxVisitorContinueKind {
-    if token.tokenKind.isPunctuation || token.tokenKind == .endOfFile {
-      return .skipChildren
-    }
-    if (token.positionAfterSkippingLeadingTrivia...token.endPositionBeforeTrailingTrivia).contains(requestedPosition) {
-      self.resolvedPosition = token.positionAfterSkippingLeadingTrivia
-    }
-    return .skipChildren
-  }
-}
 
 extension SwiftLanguageServer {
-  /// VS Code considers the position after an identifier as part of an identifier. Ie. if you have `let foo| = 1`, then
-  /// it considers the cursor to be positioned at the identifier. This scenario is hit, when selecting an identifier by
-  /// double-clicking it and then eg. performing jump-to-definition. In that case VS Code will send the position after
-  /// the identifier.
-  /// `sourcekitd`, on the other hand, does not consider the position after the identifier as part of the identifier.
-  /// To bridge the gap here, normalize any positions inside, or directly after, an identifier to the identifier's
-  /// start.
-  private func adjustPositionToStartOfIdentifier(
-    _ position: Position,
-    in snapshot: DocumentSnapshot
-  ) async -> Position {
-    let tree = await self.syntaxTreeManager.syntaxTree(for: snapshot)
-    guard let swiftSyntaxPosition = snapshot.position(of: position) else {
-      return position
-    }
-    let visitor = StartOfIdentifierFinder(position: swiftSyntaxPosition)
-    visitor.walk(tree)
-    if let resolvedPosition = visitor.resolvedPosition {
-      return snapshot.position(of: resolvedPosition) ?? position
-    } else {
-      return position
-    }
-  }
-
   public func symbolInfo(_ req: SymbolInfoRequest) async throws -> [SymbolDetails] {
     let uri = req.textDocument.uri
     let snapshot = try documentManager.latestSnapshot(uri)

Tests/SourceKitLSPTests/SwiftCompletionTests.swift

@@ -945,6 +945,39 @@ final class SwiftCompletionTests: XCTestCase {
     XCTAssert(secondCompletionResults.items.isEmpty)
   }
 
+  func testNonAsciiCompletionFilter() async throws {
+    let testClient = try await TestSourceKitLSPClient()
+    let uri = DocumentURI.for(.swift)
+    let positions = testClient.openDocument(
+      """
+      struct Foo {
+        func 👨‍👩‍👦👨‍👩‍👦() {}
+        func test() {
+          self.1️⃣👨‍👩‍👦2️⃣
+        }
+      }
+      """,
+      uri: uri
+    )
+    let completions = try await testClient.send(
+      CompletionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
+    )
+    XCTAssertEqual(
+      completions.items,
+      [
+        CompletionItem(
+          label: "👨‍👩‍👦👨‍👩‍👦()",
+          kind: .method,
+          detail: "Void",
+          deprecated: false,
+          filterText: "👨‍👩‍👦👨‍👩‍👦()",
+          insertText: "👨‍👩‍👦👨‍👩‍👦()",
+          insertTextFormat: .plain,
+          textEdit: .textEdit(TextEdit(range: positions["1️⃣"]..<positions["2️⃣"], newText: "👨‍👩‍👦👨‍👩‍👦()"))
+        )
+      ]
+    )
+  }
 }
 
 private func countFs(_ response: CompletionList) -> Int {