Merge pull request #975 from ahoppen/ahoppen/normalize-identifier-start

Support cursor info-based requests when cursor is placed at the end of an identifier

Author: ahoppen

Sources/SourceKitLSP/CMakeLists.txt

@@ -29,6 +29,7 @@ target_sources(SourceKitLSP PRIVATE
   Swift/SourceKitD+ResponseError.swift
   Swift/SwiftCommand.swift
   Swift/SwiftLanguageServer.swift
+  Swift/SymbolInfo.swift
   Swift/SyntaxHighlightingToken.swift
   Swift/SyntaxHighlightingTokenParser.swift
   Swift/SyntaxTreeManager.swift

Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift

@@ -540,15 +540,6 @@ extension SwiftLanguageServer {
     return HoverResponse(contents: .markupContent(MarkupContent(kind: .markdown, value: result)), range: nil)
   }
 
-  public func symbolInfo(_ req: SymbolInfoRequest) async throws -> [SymbolDetails] {
-    let uri = req.textDocument.uri
-    let position = req.position
-    guard let cursorInfo = try await cursorInfo(uri, position..<position) else {
-      return []
-    }
-    return [cursorInfo.symbolInfo]
-  }
-
   public func documentColor(_ req: DocumentColorRequest) async throws -> [ColorInformation] {
     let snapshot = try self.documentManager.latestSnapshot(req.textDocument.uri)
 
@@ -1223,6 +1214,13 @@ extension DocumentSnapshot {
     return lowerBound..<upperBound
   }
 
+  func position(of position: Position) -> AbsolutePosition? {
+    guard let offset = utf8Offset(of: position) else {
+      return nil
+    }
+    return AbsolutePosition(utf8Offset: offset)
+  }
+
   func indexOf(utf8Offset: Int) -> String.Index? {
     return text.utf8.index(text.startIndex, offsetBy: utf8Offset, limitedBy: text.endIndex)
   }

Sources/SourceKitLSP/Swift/SymbolInfo.swift

@@ -0,0 +1,78 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+    } 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)
+    let position = await self.adjustPositionToStartOfIdentifier(req.position, in: snapshot)
+    guard let cursorInfo = try await cursorInfo(uri, position..<position) else {
+      return []
+    }
+    return [cursorInfo.symbolInfo]
+  }
+}

Tests/SourceKitLSPTests/DefinitionTests.swift

@@ -0,0 +1,39 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 SKTestSupport
+import XCTest
+
+class DefinitionTests: XCTestCase {
+  func testJumpToDefinitionAtEndOfIdentifier() async throws {
+    let testClient = try await TestSourceKitLSPClient()
+    let uri = DocumentURI.for(.swift)
+
+    let positions = testClient.openDocument(
+      """
+      let 1️⃣foo = 1
+      _ = foo2️⃣
+      """,
+      uri: uri
+    )
+
+    let response = try await testClient.send(
+      DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
+    )
+    guard case .locations(let locations) = response else {
+      XCTFail("Expected locations response")
+      return
+    }
+    XCTAssertEqual(locations, [Location(uri: uri, range: Range(positions["1️⃣"]))])
+  }
+}