Avoid using decoders for retrieving dictionary data

Sources/LanguageServerProtocol/CodeAction.swift

@@ -48,30 +48,6 @@ public struct CodeActionRequest: TextDocumentRequest, Hashable {
     self.context = context
     self.textDocument = textDocument
   }
-
-  public func injectMetadata(atResponse response: CodeActionRequestResponse?) -> CodeActionRequestResponse? {
-    let metadata = SourceKitLSPCommandMetadata(textDocument: textDocument)
-    guard let data = try? JSONEncoder().encode(metadata),
-          let metadataArgument = try? JSONDecoder().decode(CommandArgumentType.self, from: data) else
-    {
-      log("failed to inject metadata in codeAction response", level: .error)
-      return nil
-    }
-    switch response {
-    case .codeActions(var codeActions)?:
-      for i in 0..<codeActions.count {
-        codeActions[i].command?.arguments?.append(metadataArgument)
-      }
-      return .codeActions(codeActions)
-    case .commands(var commands)?:
-      for i in 0..<commands.count {
-        commands[i].arguments?.append(metadataArgument)
-      }
-      return .commands(commands)
-    case nil:
-      return nil
-    }
-  }
 }
 
 /// Wrapper type for the response of a CodeAction request.

Sources/LanguageServerProtocol/ExecuteCommand.swift

@@ -35,35 +35,8 @@ public struct ExecuteCommandRequest: RequestType {
   /// Arguments that the command should be invoked with.
   public var arguments: [CommandArgumentType]?
 
-  /// The document in which the command was invoked.
-  public var textDocument: TextDocumentIdentifier? {
-    return metadata?.textDocument
-  }
-
-  /// Optional metadata containing SourceKit-LSP infomration about this command.
-  public var metadata: SourceKitLSPCommandMetadata? {
-    guard case .dictionary(let dictionary)? = arguments?.last else {
-      return nil
-    }
-    guard let data = try? JSONEncoder().encode(dictionary) else {
-      return nil
-    }
-    return try? JSONDecoder().decode(SourceKitLSPCommandMetadata.self, from: data)
-  }
-
   public init(command: String, arguments: [CommandArgumentType]?) {
     self.command = command
     self.arguments = arguments
   }
 }
-
-/// Represents metadata that SourceKit-LSP injects at every command returned by code actions.
-/// The ExecuteCommand is not a TextDocumentRequest, so metadata is injected to allow SourceKit-LSP
-/// to determine where a command should be executed.
-public struct SourceKitLSPCommandMetadata: Codable, Hashable {
-  public var textDocument: TextDocumentIdentifier
-
-  public init(textDocument: TextDocumentIdentifier) {
-    self.textDocument = textDocument
-  }
-}

Sources/LanguageServerProtocol/TextDocumentIdentifier.swift

@@ -28,7 +28,7 @@ public struct TextDocumentIdentifier: Hashable {
 
 // Encode using the key "uri" to match LSP.
 extension TextDocumentIdentifier: Codable {
-  private enum CodingKeys: String, CodingKey {
+  public enum CodingKeys: String, CodingKey {
     case url = "uri"
   }
 }

Sources/SourceKit/SourceKitLSPCommandMetadata.swift

@@ -0,0 +1,93 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2018 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 Foundation
+import SKSupport
+
+/// Represents metadata that SourceKit-LSP injects at every command returned by code actions.
+/// The ExecuteCommand is not a TextDocumentRequest, so metadata is injected to allow SourceKit-LSP
+/// to determine where a command should be executed.
+public struct SourceKitLSPCommandMetadata: Codable, Hashable {
+
+  public static func decode(fromDictionary dictionary: [String: CommandArgumentType]) -> SourceKitLSPCommandMetadata? {
+    guard case .dictionary(let textDocumentDict)? = dictionary[CodingKeys.sourcekitlsp_textDocument.stringValue],
+          case .string(let urlString)? = textDocumentDict[TextDocumentIdentifier.CodingKeys.url.stringValue],
+          let url = URL(string: urlString) else
+    {
+      return nil
+    }
+    let textDocument = TextDocumentIdentifier(url)
+    return SourceKitLSPCommandMetadata(textDocument: textDocument)
+  }
+
+  public var sourcekitlsp_textDocument: TextDocumentIdentifier
+
+  public init(textDocument: TextDocumentIdentifier) {
+    self.sourcekitlsp_textDocument = textDocument
+  }
+
+  func asCommandArgument() -> CommandArgumentType {
+    let textDocumentArgument = CommandArgumentType.dictionary(
+      [TextDocumentIdentifier.CodingKeys.url.stringValue: .string(sourcekitlsp_textDocument.url.absoluteString)]
+    )
+    return .dictionary([CodingKeys.sourcekitlsp_textDocument.stringValue: textDocumentArgument])
+  }
+}
+
+extension CodeActionRequest {
+  public func injectMetadata(atResponse response: CodeActionRequestResponse?) -> CodeActionRequestResponse? {
+    let metadata = SourceKitLSPCommandMetadata(textDocument: textDocument)
+    let metadataArgument = metadata.asCommandArgument()
+    switch response {
+    case .codeActions(var codeActions)?:
+      for i in 0..<codeActions.count {
+        codeActions[i].command?.arguments?.append(metadataArgument)
+      }
+      return .codeActions(codeActions)
+    case .commands(var commands)?:
+      for i in 0..<commands.count {
+        commands[i].arguments?.append(metadataArgument)
+      }
+      return .commands(commands)
+    case nil:
+      return nil
+    }
+  }
+}
+
+extension ExecuteCommandRequest {
+  /// The document in which the command was invoked.
+  public var textDocument: TextDocumentIdentifier? {
+    return metadata?.sourcekitlsp_textDocument
+  }
+
+  /// Optional metadata containing SourceKit-LSP information about this command.
+  public var metadata: SourceKitLSPCommandMetadata? {
+    guard case .dictionary(let dictionary)? = arguments?.last else {
+      return nil
+    }
+    guard let metadata = SourceKitLSPCommandMetadata.decode(fromDictionary: dictionary) else {
+      log("failed to decode lsp metadata in executeCommand request", level: .error)
+      return nil
+    }
+    return metadata
+  }
+
+  /// Returns this Command's arguments without SourceKit-LSP's injected metadata, if it exists.
+  public var argumentsWithoutLSPMetadata: [CommandArgumentType]? {
+    guard metadata != nil else {
+      return arguments
+    }
+    return arguments?.dropLast()
+  }
+}

Sources/SourceKit/SourceKitServer.swift

@@ -384,7 +384,9 @@ extension SourceKitServer {
       req.reply(nil)
       return
     }
-    sendRequest(req, url: url, workspace: workspace, fallback: nil)
+    var params = req.params
+    params.arguments = params.argumentsWithoutLSPMetadata
+    sendRequest(req, params: params, url: url, workspace: workspace, fallback: nil)
   }
 
   func definition(_ req: Request<DefinitionRequest>, workspace: Workspace) {
@@ -503,11 +505,12 @@ extension SourceKitServer {
     fallback: @autoclosure () -> PositionRequest.Response)
   where PositionRequest: TextDocumentRequest
   {
-    sendRequest(req, url: req.params.textDocument.url, workspace: workspace, resultHandler: resultHandler, fallback: fallback)
+    sendRequest(req, params: req.params, url: req.params.textDocument.url, workspace: workspace, resultHandler: resultHandler, fallback: fallback)
   }
 
   func sendRequest<PositionRequest>(
     _ req: Request<PositionRequest>,
+    params: PositionRequest,
     url: URL,
     workspace: Workspace,
     resultHandler: ((LSPResult<PositionRequest.Response>) -> LSPResult<PositionRequest.Response>)? = nil,
@@ -518,7 +521,7 @@ extension SourceKitServer {
       return
     }
 
-    let id = service.send(req.params, queue: DispatchQueue.global()) { result in
+    let id = service.send(params, queue: DispatchQueue.global()) { result in
       req.reply(resultHandler?(result) ?? result)
     }
     req.cancellationToken.addCancellationHandler { [weak service] in

Sources/SourceKit/sourcekitd/SwiftCommand.swift

@@ -22,14 +22,15 @@ public let builtinSwiftCommands: [String] = []
 /// A `Command` that should be executed by Swift's language server.
 public protocol SwiftCommand: Codable, Hashable {
   static var identifier: String { get }
+  static func decode(fromDictionary dictionary: [String: CommandArgumentType]) -> Self?
   var title: String { get set }
+  func asCommandArgument() -> CommandArgumentType
 }
 
 extension SwiftCommand {
   /// Converts this `SwiftCommand` to a generic LSP `Command` object.
   public func asCommand() throws -> Command {
-    let data = try JSONEncoder().encode(self)
-    let argument = try JSONDecoder().decode(CommandArgumentType.self, from: data)
+    let argument = asCommandArgument()
     return Command(title: title, command: Self.identifier, arguments: [argument])
   }
 }
@@ -50,14 +51,12 @@ extension ExecuteCommandRequest {
     guard case let .dictionary(dictionary) = argument else {
       return nil
     }
-    guard let data = try? JSONEncoder().encode(dictionary) else {
-      return nil
-    }
-    return try? JSONDecoder().decode(type, from: data)
+    return type.decode(fromDictionary: dictionary)
   }
 }
 
 public struct SemanticRefactorCommand: SwiftCommand {
+
   public static var identifier: String {
     return "semantic.refactor.command"
   }
@@ -80,6 +79,26 @@ public struct SemanticRefactorCommand: SwiftCommand {
   /// The text document related to the refactoring action.
   public var textDocument: TextDocumentIdentifier
 
+  public static func decode(fromDictionary dictionary: [String: CommandArgumentType]) -> SemanticRefactorCommand? {
+    guard case .dictionary(let dict)? = dictionary[CodingKeys.textDocument.stringValue],
+          case .string(let title)? = dictionary[CodingKeys.title.stringValue],
+          case .string(let actionString)? = dictionary[CodingKeys.actionString.stringValue],
+          case .int(let line)? = dictionary[CodingKeys.line.stringValue],
+          case .int(let column)? = dictionary[CodingKeys.column.stringValue],
+          case .int(let length)? = dictionary[CodingKeys.length.stringValue],
+          case .string(let urlString)? = dict[TextDocumentIdentifier.CodingKeys.url.stringValue],
+          let url = URL(string: urlString) else
+    {
+      return nil
+    }
+    return SemanticRefactorCommand(title: title,
+                                   actionString: actionString,
+                                   line: line,
+                                   column: column,
+                                   length: length,
+                                   textDocument: TextDocumentIdentifier(url))
+  }
+
   public init(title: String, actionString: String, line: Int, column: Int, length: Int, textDocument: TextDocumentIdentifier) {
     self.title = title
     self.actionString = actionString
@@ -88,4 +107,16 @@ public struct SemanticRefactorCommand: SwiftCommand {
     self.length = length
     self.textDocument = textDocument
   }
+
+  public func asCommandArgument() -> CommandArgumentType {
+    let textDocumentArgument = CommandArgumentType.dictionary(
+      [TextDocumentIdentifier.CodingKeys.url.stringValue: .string(textDocument.url.absoluteString)]
+    )
+    return .dictionary([CodingKeys.title.stringValue: .string(title),
+                        CodingKeys.actionString.stringValue: .string(actionString),
+                        CodingKeys.line.stringValue: .int(line),
+                        CodingKeys.column.stringValue: .int(column),
+                        CodingKeys.length.stringValue: .int(length),
+                        CodingKeys.textDocument.stringValue: textDocumentArgument])
+  }
 }

Tests/SourceKitTests/CodeActionTests.swift

@@ -114,7 +114,7 @@ final class CodeActionTests: XCTestCase {
       let data = try! JSONEncoder().encode(metadata)
       return try! JSONDecoder().decode(CommandArgumentType.self, from: data)
     }()
-    XCTAssertEqual(expectedMetadata, .dictionary(["textDocument": ["uri": "file:///a.swift"]]))
+    XCTAssertEqual(expectedMetadata, .dictionary(["sourcekitlsp_textDocument": ["uri": "file:///a.swift"]]))
     let command = Command(title: "Title", command: "Command", arguments: [1, "text", 2.2, nil])
     let codeAction = CodeAction(title: "1")
     let codeAction2 = CodeAction(title: "2", command: command)

Tests/SourceKitTests/ExecuteCommandTests.swift

@@ -0,0 +1,54 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2019 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 SKSupport
+import SKTestSupport
+import XCTest
+
+@testable import SourceKit
+
+final class ExecuteCommandTests: XCTestCase {
+  func testLSPCommandMetadataRetrieval() {
+    var req = ExecuteCommandRequest(command: "", arguments: nil)
+    XCTAssertNil(req.metadata)
+    req.arguments = [1, 2, ""]
+    XCTAssertNil(req.metadata)
+    let url = URL(fileURLWithPath: "/a.swift")
+    let textDocument = TextDocumentIdentifier(url)
+    let metadata = SourceKitLSPCommandMetadata(textDocument: textDocument)
+    req.arguments = [metadata.asCommandArgument(), 1, 2, ""]
+    XCTAssertNil(req.metadata)
+    req.arguments = [1, 2, "", [metadata.asCommandArgument()]]
+    XCTAssertNil(req.metadata)
+    req.arguments = [1, 2, "", metadata.asCommandArgument()]
+    XCTAssertEqual(req.metadata, metadata)
+    req.arguments = [metadata.asCommandArgument()]
+    XCTAssertEqual(req.metadata, metadata)
+  }
+
+  func testLSPCommandMetadataRemoval() {
+    var req = ExecuteCommandRequest(command: "", arguments: nil)
+    XCTAssertNil(req.argumentsWithoutLSPMetadata)
+    req.arguments = [1, 2, ""]
+    XCTAssertEqual(req.arguments, req.argumentsWithoutLSPMetadata)
+    let url = URL(fileURLWithPath: "/a.swift")
+    let textDocument = TextDocumentIdentifier(url)
+    let metadata = SourceKitLSPCommandMetadata(textDocument: textDocument)
+    req.arguments = [metadata.asCommandArgument(), 1, 2, ""]
+    XCTAssertEqual(req.arguments, req.argumentsWithoutLSPMetadata)
+    req.arguments = [1, 2, "", [metadata.asCommandArgument()]]
+    XCTAssertEqual(req.arguments, req.argumentsWithoutLSPMetadata)
+    req.arguments = [1, 2, "", metadata.asCommandArgument()]
+    XCTAssertEqual([1, 2, ""], req.argumentsWithoutLSPMetadata)
+  }
+}

Tests/SourceKitTests/XCTestManifests.swift

@@ -38,6 +38,16 @@ extension DocumentSymbolTest {
     ]
 }
 
+extension ExecuteCommandTests {
+    // DO NOT MODIFY: This is autogenerated, use:
+    //   `swift test --generate-linuxmain`
+    // to regenerate.
+    static let __allTests__ExecuteCommandTests = [
+        ("testLSPCommandMetadataRemoval", testLSPCommandMetadataRemoval),
+        ("testLSPCommandMetadataRetrieval", testLSPCommandMetadataRetrieval),
+    ]
+}
+
 extension FoldingRangeTests {
     // DO NOT MODIFY: This is autogenerated, use:
     //   `swift test --generate-linuxmain`
@@ -92,6 +102,7 @@ public func __allTests() -> [XCTestCaseEntry] {
         testCase(CodeActionTests.__allTests__CodeActionTests),
         testCase(DocumentColorTests.__allTests__DocumentColorTests),
         testCase(DocumentSymbolTest.__allTests__DocumentSymbolTest),
+        testCase(ExecuteCommandTests.__allTests__ExecuteCommandTests),
         testCase(FoldingRangeTests.__allTests__FoldingRangeTests),
         testCase(LocalClangTests.__allTests__LocalClangTests),
         testCase(LocalSwiftTests.__allTests__LocalSwiftTests),