Address review comments

Author: award999

Contributor Documentation/LSP Extensions.md

@@ -692,12 +692,12 @@ export interface PeekDocumentsResult {
 
 ## `workspace/playgrounds`
 
-New request for return the list of all #Playground macro expansions in the workspace.
+New request for returning the list of all #Playground macros in the workspace.
 
 Primarily designed to allow editors to provide a list of available playgrounds in the project workspace and allow
 jumping to the locations where the #Playground macro was expanded.
 
-The request fetches the list of all macro expansions found in the workspace, returning the location, identifier, and optional label
+The request fetches the list of all macros found in the workspace, returning the location, identifier, and optional label
 when available for each #Playground macro expansion. If you want to keep the list of playgrounds up to date without needing to
 call `workspace/playgrounds` each time a document is changed, you can filter for `swift.play` CodeLens returned by the `textDocument/codelens` request.
 
@@ -710,7 +710,7 @@ SourceKit-LSP will advertise `workspace/playgrounds` in its experimental server
 export interface WorkspacePlaygroundParams {}
 
 /**
- * A `Playground` represents an expansion of the #Playground macro, providing the editor with the
+ * A `Playground` represents a usage of the #Playground macro, providing the editor with the
  * location of the playground and identifiers to allow executing the playground through a "swift play" command.
  */
 export interface Playground {
@@ -730,7 +730,7 @@ export interface Playground {
   label?: string
 
   /**
-   * The location of the of where the #Playground macro expansion occured in the source code.
+   * The location of where the #Playground macro was used in the source code.
    */
   location: Location
 }

Sources/LanguageServerProtocol/SupportTypes/Location.swift

@@ -13,7 +13,9 @@
 /// Range within a particular document.
 ///
 /// For a location where the document is implied, use `Position` or `Range<Position>`.
-public struct Location: ResponseType, Hashable, Codable, CustomDebugStringConvertible, Comparable, Sendable {
+public struct Location: ResponseType, Hashable, Codable, CustomDebugStringConvertible, Comparable, Sendable,
+  LSPAnyCodable
+{
   public static func < (lhs: Location, rhs: Location) -> Bool {
     if lhs.uri != rhs.uri {
       return lhs.uri.stringValue < rhs.uri.stringValue
@@ -34,14 +36,27 @@ public struct Location: ResponseType, Hashable, Codable, CustomDebugStringConver
     self._range = CustomCodable<PositionRange>(wrappedValue: range)
   }
 
+  public init?(fromLSPDictionary dictionary: [String: LSPAny]) {
+    guard
+      case .string(let uriString) = dictionary["uri"],
+      case .dictionary(let rangeDict) = dictionary["range"],
+      let uri = try? DocumentURI(string: uriString),
+      let range = Range<Position>(fromLSPDictionary: rangeDict)
+    else {
+      return nil
+    }
+    self.uri = uri
+    self._range = CustomCodable<PositionRange>(wrappedValue: range)
+  }
+
   public var debugDescription: String {
     return "\(uri):\(range.lowerBound)-\(range.upperBound)"
   }
 
   public func encodeToLSPAny() -> LSPAny {
     return .dictionary([
       "uri": .string(uri.stringValue),
-      "range": range.encodeToLSPAny()
+      "range": range.encodeToLSPAny(),
     ])
   }
 }

Sources/LanguageServerProtocol/SupportTypes/Playground.swift

@@ -10,17 +10,23 @@
 //
 //===----------------------------------------------------------------------===//
 
-/// A playground item that can be used to identify playgrounds alongside a source file.
-public struct Playground: ResponseType, Equatable {
-  /// Identifier for the `Playground`.
+/// A `Playground` represents a usage of the #Playground macro, providing the editor with the
+/// location of the playground and identifiers to allow executing the playground through a "swift play" command.
+///
+/// **(LSP Extension)**
+public struct Playground: ResponseType, Equatable, LSPAnyCodable {
+  /// Unique identifier for the `Playground`. Client can run the playground by executing `swift play <id>`.
   ///
-  /// This identifier uniquely identifies the playground. It can be used to run an individual playground with `swift play`.
+  /// This property is always present whether the `Playground` has a `label` or not.
+  ///
+  /// Follows the format output by `swift play --list`.
   public var id: String
 
-  /// Display name describing the playground.
+  /// The label that can be used as a display name for the playground. This optional property is only available
+  /// for named playgrounds. For example: `#Playground("hello") { print("Hello!) }` would have a `label` of `"hello"`.
   public var label: String?
 
-  /// The location of the #Playground macro expansion in the source code.
+  /// The location of where the #Playground macro was used in the source code.
   public var location: Location
 
   public init(
@@ -33,10 +39,25 @@ public struct Playground: ResponseType, Equatable {
     self.location = location
   }
 
+  public init?(fromLSPDictionary dictionary: [String: LSPAny]) {
+    guard
+      case .string(let id) = dictionary["id"],
+      case .dictionary(let locationDict) = dictionary["location"],
+      let location = Location(fromLSPDictionary: locationDict)
+    else {
+      return nil
+    }
+    self.id = id
+    self.location = location
+    if case .string(let label) = dictionary["label"] {
+      self.label = label
+    }
+  }
+
   public func encodeToLSPAny() -> LSPAny {
     var dict: [String: LSPAny] = [
       "id": .string(id),
-      "location": location.encodeToLSPAny()
+      "location": location.encodeToLSPAny(),
     ]
 
     if let label {

Sources/LanguageServerProtocol/SupportTypes/TextDocumentPlayground.swift

@@ -10,18 +10,23 @@
 //
 //===----------------------------------------------------------------------===//
 
-/// A playground item that can be used to identify playground. Differs from `TextDocumentPlayground`
-/// by not including location which is given for `textDocument/playgrounds` request
-public struct TextDocumentPlayground: ResponseType, Equatable {
-  /// Identifier for the `TextDocumentPlayground`.
+/// A `TextDocumentPlayground` item can be used to identify playground and identify it
+/// to allow executing the playground through a "swift play" command. Differs from `Playground`
+/// by only including the `range` instead of full `location` with the expectation being that
+/// it is only returned as part of a textDocument/* request such as textDocument/codelens
+public struct TextDocumentPlayground: ResponseType, Equatable, LSPAnyCodable {
+  /// Unique identifier for the `Playground`. Client can run the playground by executing `swift play <id>`.
   ///
-  /// This identifier uniquely identifies the playground. It can be used to run an individual playground with `swift play`.
+  /// This property is always present whether the `Playground` has a `label` or not.
+  ///
+  /// Follows the format output by `swift play --list`.
   public var id: String
 
-  /// Display name describing the playground.
+  /// The label that can be used as a display name for the playground. This optional property is only available
+  /// for named playgrounds. For example: `#Playground("hello") { print("Hello!) }` would have a `label` of `"hello"`.
   public var label: String?
 
-  /// The range of the #Playground macro expansion in the given file.
+  /// The full range of the #Playground macro body in the given file.
   public var range: Range<Position>
 
   public init(
@@ -34,16 +39,29 @@ public struct TextDocumentPlayground: ResponseType, Equatable {
     self.range = range
   }
 
+  public init?(fromLSPDictionary dictionary: [String: LSPAny]) {
+    guard
+      case .string(let id) = dictionary["id"],
+      case .dictionary(let rangeDict) = dictionary["range"],
+      let range = Range<Position>(fromLSPDictionary: rangeDict)
+    else {
+      return nil
+    }
+    self.id = id
+    self.range = range
+    if case .string(let label) = dictionary["label"] {
+      self.label = label
+    }
+  }
+
   public func encodeToLSPAny() -> LSPAny {
     var dict: [String: LSPAny] = [
       "id": .string(id),
-      "range": range.encodeToLSPAny()
+      "range": range.encodeToLSPAny(),
     ]
-
     if let label {
       dict["label"] = .string(label)
     }
-
     return .dictionary(dict)
   }
 }

Sources/SwiftLanguageService/SwiftCodeLensScanner.swift

@@ -65,15 +65,34 @@ final class SwiftCodeLensScanner: SyntaxVisitor {
     var codeLenses: [CodeLens] = []
     let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
     if snapshot.text.contains("@main") {
-      let visitor = SwiftCodeLensScanner(snapshot: snapshot, targetName: targetDisplayName, supportedCommands: supportedCommands)
+      let visitor = SwiftCodeLensScanner(
+        snapshot: snapshot,
+        targetName: targetDisplayName,
+        supportedCommands: supportedCommands
+      )
       visitor.walk(syntaxTree)
       codeLenses += visitor.result
     }
 
     // "swift.play" CodeLens should be ignored if "swift-play" is not in the toolchain as the client has no way of running
-    if toolchain.swiftPlay != nil, let workspace, let playCommand = supportedCommands[SupportedCodeLensCommand.play], snapshot.text.contains("#Playground") {
-      let playgrounds = await SwiftPlaygroundsScanner.findDocumentPlaygrounds(in: syntaxTree, workspace: workspace, snapshot: snapshot)
-      codeLenses += playgrounds.map({ CodeLens(range: $0.range, command: Command(title: "Play \"\($0.label ?? $0.id)\"", command: playCommand, arguments: [$0.encodeToLSPAny()])) })
+    if toolchain.swiftPlay != nil, let workspace, let playCommand = supportedCommands[SupportedCodeLensCommand.play],
+      snapshot.text.contains("#Playground")
+    {
+      let playgrounds = await SwiftPlaygroundsScanner.findDocumentPlaygrounds(
+        in: syntaxTree,
+        workspace: workspace,
+        snapshot: snapshot
+      )
+      codeLenses += playgrounds.map({
+        CodeLens(
+          range: $0.range,
+          command: Command(
+            title: "Play \"\($0.label ?? $0.id)\"",
+            command: playCommand,
+            arguments: [$0.encodeToLSPAny()]
+          )
+        )
+      })
     }
 
     return codeLenses

Tests/SourceKitLSPTests/CodeLensTests.swift

@@ -19,7 +19,7 @@ final class CodeLensTests: XCTestCase {
 
   var toolchain: Toolchain!
   var toolchainWithSwiftPlay: Toolchain!
-  
+
   override func setUp() async throws {
     toolchain = try await unwrap(ToolchainRegistry.forTesting.default)
     toolchainWithSwiftPlay = Toolchain(
@@ -29,7 +29,7 @@ final class CodeLensTests: XCTestCase {
       clang: toolchain.clang,
       swift: toolchain.swift,
       swiftc: toolchain.swiftc,
-      swiftPlay: URL(string: "/path/to/swift-play"),
+      swiftPlay: URL(fileURLWithPath: "/dummy/usr/bin/swift-play"),
       clangd: toolchain.clangd,
       sourcekitd: toolchain.sourcekitd,
       sourceKitClientPlugin: toolchain.sourceKitClientPlugin,
@@ -191,7 +191,7 @@ final class CodeLensTests: XCTestCase {
   func testMultiplePlaygroundCodeLensOnLine() async throws {
     var codeLensCapabilities = TextDocumentClientCapabilities.CodeLens()
     codeLensCapabilities.supportedCommands = [
-      SupportedCodeLensCommand.play: "swift.play",
+      SupportedCodeLensCommand.play: "swift.play"
     ]
     let capabilities = ClientCapabilities(textDocument: TextDocumentClientCapabilities(codeLens: codeLensCapabilities))
     let toolchainRegistry = ToolchainRegistry(toolchains: [toolchainWithSwiftPlay])
@@ -266,8 +266,21 @@ final class CodeLensTests: XCTestCase {
       SupportedCodeLensCommand.play: "swift.play",
     ]
     let capabilities = ClientCapabilities(textDocument: TextDocumentClientCapabilities(codeLens: codeLensCapabilities))
-    let toolchain = try await unwrap(ToolchainRegistry.forTesting.default)
-    let toolchainRegistry = ToolchainRegistry(toolchains: [toolchain])
+    let toolchainWithoutSwiftPlay = Toolchain(
+      identifier: "\(toolchain.identifier)-swift-swift",
+      displayName: "\(toolchain.identifier) with swift-play",
+      path: toolchain.path,
+      clang: toolchain.clang,
+      swift: toolchain.swift,
+      swiftc: toolchain.swiftc,
+      swiftPlay: nil,
+      clangd: toolchain.clangd,
+      sourcekitd: toolchain.sourcekitd,
+      sourceKitClientPlugin: toolchain.sourceKitClientPlugin,
+      sourceKitServicePlugin: toolchain.sourceKitServicePlugin,
+      libIndexStore: toolchain.libIndexStore
+    )
+    let toolchainRegistry = ToolchainRegistry(toolchains: [toolchainWithoutSwiftPlay])
 
     let project = try await SwiftPMTestProject(
       files: [
@@ -317,15 +330,15 @@ final class CodeLensTests: XCTestCase {
         CodeLens(
           range: positions["1️⃣"]..<positions["2️⃣"],
           command: Command(title: "Debug MyApp", command: "swift.debug", arguments: [.string("MyApp")])
-        )
+        ),
       ]
     )
   }
 
   func testNoImportPlaygrounds() async throws {
     var codeLensCapabilities = TextDocumentClientCapabilities.CodeLens()
     codeLensCapabilities.supportedCommands = [
-      SupportedCodeLensCommand.play: "swift.play",
+      SupportedCodeLensCommand.play: "swift.play"
     ]
     let capabilities = ClientCapabilities(textDocument: TextDocumentClientCapabilities(codeLens: codeLensCapabilities))
     let toolchainRegistry = ToolchainRegistry(toolchains: [toolchainWithSwiftPlay])
@@ -369,7 +382,7 @@ final class CodeLensTests: XCTestCase {
   func testCodeLensRequestNoPlaygrounds() async throws {
     var codeLensCapabilities = TextDocumentClientCapabilities.CodeLens()
     codeLensCapabilities.supportedCommands = [
-      SupportedCodeLensCommand.play: "swift.play",
+      SupportedCodeLensCommand.play: "swift.play"
     ]
     let capabilities = ClientCapabilities(textDocument: TextDocumentClientCapabilities(codeLens: codeLensCapabilities))
     let toolchainRegistry = ToolchainRegistry(toolchains: [toolchainWithSwiftPlay])