Merge pull request #1114 from ahoppen/ahoppen/no-internal-name-in-snippet

Don’t include internal parameter names in LSP snippets

Author: ahoppen

Sources/SourceKitLSP/CMakeLists.txt

@@ -27,10 +27,10 @@ target_sources(SourceKitLSP PRIVATE
   Swift/DiagnosticReportManager.swift
   Swift/DocumentFormatting.swift
   Swift/DocumentSymbols.swift
-  Swift/EditorPlaceholder.swift
   Swift/FoldingRange.swift
   Swift/OpenInterface.swift
   Swift/RelatedIdentifiers.swift
+  Swift/RewriteSourceKitPlaceholders.swift
   Swift/SemanticRefactorCommand.swift
   Swift/SemanticRefactoring.swift
   Swift/SemanticTokens.swift

Sources/SourceKitLSP/Swift/CodeCompletionSession.swift

@@ -361,7 +361,7 @@ class CodeCompletionSession {
       }
 
       let text = insertText.map {
-        rewriteSourceKitPlaceholders(inString: $0, clientSupportsSnippets: clientSupportsSnippets)
+        rewriteSourceKitPlaceholders(in: $0, clientSupportsSnippets: clientSupportsSnippets)
       }
       let isInsertTextSnippet = clientSupportsSnippets && text != insertText
 

Sources/SourceKitLSP/Swift/Diagnostic.swift

@@ -129,10 +129,7 @@ extension TextEdit {
     {
       // Snippets are only suppored in code completion.
       // Remove SourceKit placeholders from Fix-Its because they can't be represented in the editor properly.
-      let replacementWithoutPlaceholders = rewriteSourceKitPlaceholders(
-        inString: replacement,
-        clientSupportsSnippets: false
-      )
+      let replacementWithoutPlaceholders = rewriteSourceKitPlaceholders(in: replacement, clientSupportsSnippets: false)
 
       // If both the replacement without placeholders and the fixit are empty, no TextEdit should be created.
       if (replacementWithoutPlaceholders.isEmpty && length == 0) {

Sources/SourceKitLSP/Swift/EditorPlaceholder.swift

@@ -0,0 +1,43 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 LSPLogging
+@_spi(SourceKitLSP) import SwiftRefactor
+
+func rewriteSourceKitPlaceholders(in string: String, clientSupportsSnippets: Bool) -> String {
+  var result = string
+  var index = 1
+  while let start = result.range(of: "<#") {
+    guard let end = result[start.upperBound...].range(of: "#>") else {
+      logger.error("invalid placeholder in \(string)")
+      return string
+    }
+    let rawPlaceholder = String(result[start.lowerBound..<end.upperBound])
+    guard let displayName = EditorPlaceholderData(text: rawPlaceholder)?.nameForSnippet else {
+      logger.error("failed to decode placeholder \(rawPlaceholder) in \(string)")
+      return string
+    }
+    let snippet = clientSupportsSnippets ? "${\(index):\(displayName)}" : ""
+    result.replaceSubrange(start.lowerBound..<end.upperBound, with: snippet)
+    index += 1
+  }
+  return result
+}
+
+fileprivate extension EditorPlaceholderData {
+  var nameForSnippet: Substring {
+    switch self {
+    case .basic(text: let text): return text
+    case .typed(text: let text, type: _): return text
+    }
+  }
+}

Sources/SourceKitLSP/Swift/RewriteSourceKitPlaceholders.swift

@@ -64,7 +64,7 @@ struct SemanticRefactoring {
         {
           // Snippets are only suppored in code completion.
           // Remove SourceKit placeholders in refactoring actions because they can't be represented in the editor properly.
-          let textWithSnippets = rewriteSourceKitPlaceholders(inString: text, clientSupportsSnippets: false)
+          let textWithSnippets = rewriteSourceKitPlaceholders(in: text, clientSupportsSnippets: false)
           let edit = TextEdit(range: startPosition..<endPosition, newText: textWithSnippets)
           textEdits.append(edit)
         }

Sources/SourceKitLSP/Swift/SemanticRefactoring.swift

@@ -1364,57 +1364,6 @@ final class LocalSwiftTests: XCTestCase {
     }
   }
 
-  func testEditorPlaceholderParsing() {
-    var text =
-      "<#basic placeholder"  // Need to end this in another line so Xcode doesn't treat it as a real placeholder
-      + "#>"
-    var data = EditorPlaceholder(text)
-    XCTAssertNotNil(data)
-    if let data = data {
-      XCTAssertEqual(data, .basic("basic placeholder"))
-      XCTAssertEqual(data.displayName, "basic placeholder")
-    }
-    text = "<#T##x: Int##Int" + "#>"
-    data = EditorPlaceholder(text)
-    XCTAssertNotNil(data)
-    if let data = data {
-      XCTAssertEqual(data, .typed(displayName: "x: Int", type: "Int", typeForExpansion: "Int"))
-      XCTAssertEqual(data.displayName, "x: Int")
-    }
-    text = "<#T##x: Int##Blah##()->Int" + "#>"
-    data = EditorPlaceholder(text)
-    XCTAssertNotNil(data)
-    if let data = data {
-      XCTAssertEqual(data, .typed(displayName: "x: Int", type: "Blah", typeForExpansion: "()->Int"))
-      XCTAssertEqual(data.displayName, "x: Int")
-    }
-    text = "<#T##Int" + "#>"
-    data = EditorPlaceholder(text)
-    XCTAssertNotNil(data)
-    if let data = data {
-      XCTAssertEqual(data, .typed(displayName: "Int", type: "Int", typeForExpansion: "Int"))
-      XCTAssertEqual(data.displayName, "Int")
-    }
-    text = "<#foo"
-    data = EditorPlaceholder(text)
-    XCTAssertNil(data)
-    text = " <#foo"
-    data = EditorPlaceholder(text)
-    XCTAssertNil(data)
-    text = "foo"
-    data = EditorPlaceholder(text)
-    XCTAssertNil(data)
-    text = "foo#>"
-    data = EditorPlaceholder(text)
-    XCTAssertNil(data)
-    text = "<#foo#"
-    data = EditorPlaceholder(text)
-    XCTAssertNil(data)
-    text = " <#foo" + "#>"
-    data = EditorPlaceholder(text)
-    XCTAssertNil(data)
-  }
-
   func testIncrementalParse() async throws {
     let testClient = try await TestSourceKitLSPClient()
     let url = URL(fileURLWithPath: "/\(UUID())/a.swift")

Tests/SourceKitLSPTests/LocalSwiftTests.swift

@@ -172,11 +172,11 @@ final class SwiftCompletionTests: XCTestCase {
         .textEdit(
           TextEdit(
             range: Position(line: 9, utf16index: 9)..<Position(line: 9, utf16index: 9),
-            newText: "test(${1:b: Int})"
+            newText: "test(${1:Int})"
           )
         )
       )
-      XCTAssertEqual(test.insertText, "test(${1:b: Int})")
+      XCTAssertEqual(test.insertText, "test(${1:Int})")
       XCTAssertEqual(test.insertTextFormat, .snippet)
     }
   }