From a87d8b412203f217596a889db6b4068e6a5fd718 Mon Sep 17 00:00:00 2001 From: Josh Caswell Date: Mon, 11 Nov 2024 14:44:07 -0800 Subject: [PATCH] Handle new swift-syntax closure expansion behavior This resolves , following the discussion of alternatives on . The bulk of the change updates the translation from SourceKit placeholders to LSP placeholders to handle nesting. --- .../Swift/CodeCompletionSession.swift | 6 +- .../Swift/RewriteSourceKitPlaceholders.swift | 167 ++++++++++++++++-- .../RewriteSourceKitPlaceholdersTests.swift | 54 ++++++ .../SwiftCompletionTests.swift | 48 ++--- 4 files changed, 215 insertions(+), 60 deletions(-) create mode 100644 Tests/SourceKitLSPTests/RewriteSourceKitPlaceholdersTests.swift diff --git a/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift b/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift index 54b645379..1e2f84adb 100644 --- a/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift +++ b/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift @@ -323,9 +323,9 @@ class CodeCompletionSession { var parser = Parser(exprToExpand) let expr = ExprSyntax.parse(from: &parser) guard let call = OutermostFunctionCallFinder.findOutermostFunctionCall(in: expr), - let expandedCall = ExpandEditorPlaceholdersToTrailingClosures.refactor( + let expandedCall = ExpandEditorPlaceholdersToLiteralClosures.refactor( syntax: call, - in: ExpandEditorPlaceholdersToTrailingClosures.Context(indentationWidth: indentationWidth) + in: ExpandEditorPlaceholdersToLiteralClosures.Context(indentationWidth: indentationWidth) ) else { return nil @@ -334,7 +334,7 @@ class CodeCompletionSession { let bytesToExpand = Array(exprToExpand.utf8) var expandedBytes: [UInt8] = [] - // Add the prefix that we stripped of to allow expression parsing + // Add the prefix that we stripped off to allow expression parsing expandedBytes += strippedPrefix.utf8 // Add any part of the expression that didn't end up being part of the function call expandedBytes += bytesToExpand[0.. String { - var result = string - var index = 1 - while let start = result.range(of: "<#") { - guard let end = result[start.upperBound...].range(of: "#>") else { - logger.fault("Invalid placeholder in \(string)") - return string - } - let rawPlaceholder = String(result[start.lowerBound..` — in `input` to LSP +/// placeholder syntax: `${n:foo}`. +/// +/// If `clientSupportsSnippets` is `false`, the placeholder is rendered as an +/// empty string, to prevent the client from inserting special placeholder +/// characters as if they were literal text. +func rewriteSourceKitPlaceholders(in input: String, clientSupportsSnippets: Bool) -> String { + var result = "" + var nextPlaceholderNumber = 1 + // Current stack of nested placeholders, most nested last. Each element needs + // to be rendered inside the element before it. + var placeholders: [(number: Int, contents: String)] = [] + let tokens = tokenize(input) + for token in tokens { + switch token { + case let .text(text) where placeholders.isEmpty: + result += text + + case let .text(text): + placeholders.latest.contents += text + + case let .curlyBrace(brace) where placeholders.isEmpty: + result.append(brace) + + case let .curlyBrace(brace): + // Braces are only escaped _inside_ a placeholder; otherwise the client + // would include the backslashes literally. + placeholders.latest.contents.append(contentsOf: ["\\", brace]) + + case .open: + placeholders.append((number: nextPlaceholderNumber, contents: "")) + nextPlaceholderNumber += 1 + + case .close: + guard let (number, placeholderBody) = placeholders.popLast() else { + logger.fault("Invalid placeholder in \(input)") + return input + } + guard let displayName = nameForSnippet(placeholderBody) else { + logger.fault("Failed to decode placeholder \(placeholderBody) in \(input)") + return input + } + let placeholder = + clientSupportsSnippets + ? formatLSPPlaceholder(displayName, number: number) + : "" + if placeholders.isEmpty { + result += placeholder + } else { + placeholders.latest.contents += placeholder + } } - let snippet = clientSupportsSnippets ? "${\(index):\(displayName)}" : "" - result.replaceSubrange(start.lowerBound.. String? { - var text = text +/// Scan `input` to identify special elements within: curly braces, which may +/// need to be escaped; and SourceKit placeholder open/close delimiters. +private func tokenize(_ input: String) -> [SnippetToken] { + var index = input.startIndex + var isAtEnd: Bool { index == input.endIndex } + func match(_ char: Character) -> Bool { + if isAtEnd || input[index] != char { + return false + } else { + input.formIndex(after: &index) + return true + } + } + func next() -> Character? { + guard !isAtEnd else { return nil } + defer { input.formIndex(after: &index) } + return input[index] + } + + var tokens: [SnippetToken] = [] + var text = "" + while let char = next() { + switch char { + case "<": + if match("#") { + tokens.append(.text(text)) + text.removeAll() + tokens.append(.open) + } else { + text.append(char) + } + + case "#": + if match(">") { + tokens.append(.text(text)) + text.removeAll() + tokens.append(.close) + } else { + text.append(char) + } + + case "{", "}": + tokens.append(.text(text)) + text.removeAll() + tokens.append(.curlyBrace(char)) + + case let c: + text.append(c) + } + } + + tokens.append(.text(text)) + + return tokens +} + +/// A syntactical element inside a SourceKit snippet. +private enum SnippetToken { + /// A placeholder delimiter. + case open, close + /// One of '{' or '}', which may need to be escaped in the output. + case curlyBrace(Character) + /// Any other consecutive run of characters from the input, which needs no + /// special treatment. + case text(String) +} + +/// Given the interior text of a SourceKit placeholder, extract a display name +/// suitable for a LSP snippet. +private func nameForSnippet(_ body: String) -> String? { + var text = rewrappedAsPlaceholder(body) return text.withSyntaxText { guard let data = RawEditorPlaceholderData(syntaxText: $0) else { return nil @@ -45,3 +149,28 @@ fileprivate func nameForSnippet(_ text: String) -> String? { return String(syntaxText: data.typeForExpansionText ?? data.displayText) } } + +private let placeholderStart = "<#" +private let placeholderEnd = "#>" +private func rewrappedAsPlaceholder(_ body: String) -> String { + return placeholderStart + body + placeholderEnd +} + +/// Wrap `body` in LSP snippet placeholder syntax, using `number` as the +/// placeholder's index in the snippet. +private func formatLSPPlaceholder(_ body: String, number: Int) -> String { + "${\(number):\(body)}" +} + +private extension Array { + /// Mutable access to the final element of an array. + /// + /// - precondition: The array must not be empty. + var latest: Element { + get { self.last! } + _modify { + let index = self.index(before: self.endIndex) + yield &self[index] + } + } +} diff --git a/Tests/SourceKitLSPTests/RewriteSourceKitPlaceholdersTests.swift b/Tests/SourceKitLSPTests/RewriteSourceKitPlaceholdersTests.swift new file mode 100644 index 000000000..75d6bbc59 --- /dev/null +++ b/Tests/SourceKitLSPTests/RewriteSourceKitPlaceholdersTests.swift @@ -0,0 +1,54 @@ +import SKTestSupport +@testable import SourceKitLSP +import XCTest + +final class RewriteSourceKitPlaceholdersTests : XCTestCase { + func testClientDoesNotSupportSnippets() { + let input = "foo(bar: <#T##bar##Int#>)" + let rewritten = rewriteSourceKitPlaceholders(in: input, clientSupportsSnippets: false) + + XCTAssertEqual(rewritten, "foo(bar: )") + } + + func testInputWithoutPlaceholders() { + let input = "foo()" + let rewritten = rewriteSourceKitPlaceholders(in: input, clientSupportsSnippets: true) + + XCTAssertEqual(rewritten, "foo()") + } + + func testPlaceholderWithType() { + let input = "foo(bar: <#T##bar##Int#>)" + let rewritten = rewriteSourceKitPlaceholders(in: input, clientSupportsSnippets: true) + + XCTAssertEqual(rewritten, "foo(bar: ${1:Int})") + } + + func testMultiplePlaceholders() { + let input = "foo(bar: <#T##Int##Int#>, baz: <#T##String##String#>, quux: <#T##String##String#>)" + let rewritten = rewriteSourceKitPlaceholders(in: input, clientSupportsSnippets: true) + + XCTAssertEqual(rewritten, "foo(bar: ${1:Int}, baz: ${2:String}, quux: ${3:String})") + } + + func testClosurePlaceholderReturnType() { + let input = "foo(bar: <#{ <#T##Int##Int#> }#>)" + let rewritten = rewriteSourceKitPlaceholders(in: input, clientSupportsSnippets: true) + + XCTAssertEqual(rewritten, "foo(bar: ${1:\\{ ${2:Int} \\}})") + } + + func testClosurePlaceholderArgumentType() { + let input = "foo(bar: <#{ <#T##Int##Int#> in <#T##Void##Void#> }#>)" + let rewritten = rewriteSourceKitPlaceholders(in: input, clientSupportsSnippets: true) + + XCTAssertEqual(rewritten, "foo(bar: ${1:\\{ ${2:Int} in ${3:Void} \\}})") + } + + func testMultipleClosurePlaceholders() { + let input = "foo(<#{ <#T##Int##Int#> }#>, baz: <#{ <#Int#> in <#T##Bool##Bool#> }#>)" + let rewritten = rewriteSourceKitPlaceholders(in: input, clientSupportsSnippets: true) + + XCTAssertEqual(rewritten, "foo(${1:\\{ ${2:Int} \\}}, baz: ${3:\\{ ${4:Int} in ${5:Bool} \\}})") + } +} diff --git a/Tests/SourceKitLSPTests/SwiftCompletionTests.swift b/Tests/SourceKitLSPTests/SwiftCompletionTests.swift index 4ac1f2baf..e757ff1fd 100644 --- a/Tests/SourceKitLSPTests/SwiftCompletionTests.swift +++ b/Tests/SourceKitLSPTests/SwiftCompletionTests.swift @@ -864,18 +864,14 @@ final class SwiftCompletionTests: XCTestCase { sortText: nil, filterText: "myMap(:)", insertText: """ - myMap { ${1:Int} in - ${2:Bool} - } + myMap(${1:\\{ ${2:Int} in ${3:Bool} \\}}) """, insertTextFormat: .snippet, textEdit: .textEdit( TextEdit( range: Range(positions["1️⃣"]), newText: """ - myMap { ${1:Int} in - ${2:Bool} - } + myMap(${1:\\{ ${2:Int} in ${3:Bool} \\}}) """ ) ) @@ -912,18 +908,14 @@ final class SwiftCompletionTests: XCTestCase { sortText: nil, filterText: ".myMap(:)", insertText: """ - ?.myMap { ${1:Int} in - ${2:Bool} - } + ?.myMap(${1:\\{ ${2:Int} in ${3:Bool} \\}}) """, insertTextFormat: .snippet, textEdit: .textEdit( TextEdit( range: positions["1️⃣"]..