Skip to content

Commit a714d41

Browse files
committed
Customize closure expansion behavior
This resolves <swiftlang#1788>, following the discussion of alternatives on <https://github.com/swiftlang/sourcekit-lsp/pulls/1789>. The bulk of the change updates the translation from SourceKit placeholders to LSP placeholders to handle nesting. The `CodeCompletionSession` also passes a new custom formatter to the swift-syntax expansion routine, which disables the transformation to trailing closures.
1 parent 3131ca3 commit a714d41

File tree

6 files changed

+458
-80
lines changed

6 files changed

+458
-80
lines changed
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#if compiler(>=6.0)
14+
public import SwiftBasicFormat
15+
public import SwiftSyntax
16+
#else
17+
import SwiftBasicFormat
18+
import SwiftSyntax
19+
#endif
20+
21+
/// A specialization of `BasicFormat` for closure literals in a code completion
22+
/// context.
23+
///
24+
/// This is more conservative about newline insertion: unless the closure has
25+
/// multiple statements in its body it will not be reformatted to multiple
26+
/// lines.
27+
@_spi(Testing)
28+
public class ClosureCompletionFormat: BasicFormat {
29+
@_spi(Testing)
30+
public override func requiresNewline(
31+
between first: TokenSyntax?,
32+
and second: TokenSyntax?
33+
) -> Bool {
34+
if let first, isEndOfSmallClosureSignature(first) {
35+
return false
36+
} else if let first, isSmallClosureDelimiter(first, kind: \.leftBrace) {
37+
return false
38+
} else if let second, isSmallClosureDelimiter(second, kind: \.rightBrace) {
39+
return false
40+
} else {
41+
return super.requiresNewline(between: first, and: second)
42+
}
43+
}
44+
45+
/// Returns `true` if `token` is an opening or closing brace (according to
46+
/// `kind`) of a closure, and that closure has no more than one statement in
47+
/// its body.
48+
private func isSmallClosureDelimiter(
49+
_ token: TokenSyntax,
50+
kind: KeyPath<ClosureExprSyntax, TokenSyntax>
51+
) -> Bool {
52+
guard token.keyPathInParent == kind,
53+
let closure = token.parent?.as(ClosureExprSyntax.self)
54+
else {
55+
return false
56+
}
57+
58+
return closure.statements.count <= 1
59+
}
60+
61+
/// Returns `true` if `token` is the last token in the signature of a closure,
62+
/// and that closure has no more than one statement in its body.
63+
private func isEndOfSmallClosureSignature(_ token: TokenSyntax) -> Bool {
64+
guard
65+
token.keyPathInParent == \ClosureSignatureSyntax.inKeyword,
66+
let closure = token.ancestorOrSelf(mapping: { $0.as(ClosureExprSyntax.self) })
67+
else {
68+
return false
69+
}
70+
71+
return closure.statements.count <= 1
72+
}
73+
}

Sources/SourceKitLSP/Swift/CodeCompletionSession.swift

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,14 @@ class CodeCompletionSession {
323323
var parser = Parser(exprToExpand)
324324
let expr = ExprSyntax.parse(from: &parser)
325325
guard let call = OutermostFunctionCallFinder.findOutermostFunctionCall(in: expr),
326-
let expandedCall = ExpandEditorPlaceholdersToTrailingClosures.refactor(
326+
let expandedCall = ExpandEditorPlaceholdersToLiteralClosures.refactor(
327327
syntax: call,
328-
in: ExpandEditorPlaceholdersToTrailingClosures.Context(indentationWidth: indentationWidth)
328+
in: ExpandEditorPlaceholdersToLiteralClosures.Context(
329+
format: .custom(
330+
ClosureCompletionFormat(indentationWidth: indentationWidth),
331+
allowNestedPlaceholders: true
332+
)
333+
)
329334
)
330335
else {
331336
return nil
@@ -334,7 +339,7 @@ class CodeCompletionSession {
334339
let bytesToExpand = Array(exprToExpand.utf8)
335340

336341
var expandedBytes: [UInt8] = []
337-
// Add the prefix that we stripped of to allow expression parsing
342+
// Add the prefix that we stripped off to allow expression parsing
338343
expandedBytes += strippedPrefix.utf8
339344
// Add any part of the expression that didn't end up being part of the function call
340345
expandedBytes += bytesToExpand[0..<call.position.utf8Offset]

Sources/SourceKitLSP/Swift/RewriteSourceKitPlaceholders.swift

Lines changed: 151 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,34 +14,166 @@ import Foundation
1414
import SKLogging
1515
@_spi(RawSyntax) import SwiftSyntax
1616

17-
func rewriteSourceKitPlaceholders(in string: String, clientSupportsSnippets: Bool) -> String {
18-
var result = string
19-
var index = 1
20-
while let start = result.range(of: "<#") {
21-
guard let end = result[start.upperBound...].range(of: "#>") else {
22-
logger.fault("Invalid placeholder in \(string)")
23-
return string
24-
}
25-
let rawPlaceholder = String(result[start.lowerBound..<end.upperBound])
26-
guard let displayName = nameForSnippet(rawPlaceholder) else {
27-
logger.fault("Failed to decode placeholder \(rawPlaceholder) in \(string)")
28-
return string
17+
/// Translate SourceKit placeholder syntax — `<#foo#>` — in `input` to LSP
18+
/// placeholder syntax: `${n:foo}`.
19+
///
20+
/// If `clientSupportsSnippets` is `false`, the placeholder is rendered as an
21+
/// empty string, to prevent the client from inserting special placeholder
22+
/// characters as if they were literal text.
23+
@_spi(Testing)
24+
public func rewriteSourceKitPlaceholders(in input: String, clientSupportsSnippets: Bool) -> String {
25+
var result = ""
26+
var nextPlaceholderNumber = 1
27+
// Current stack of nested placeholders, most nested last. Each element needs
28+
// to be rendered inside the element before it.
29+
var placeholders: [(number: Int, contents: String)] = []
30+
let tokens = tokenize(input)
31+
for token in tokens {
32+
switch token {
33+
case let .text(text):
34+
if placeholders.isEmpty {
35+
result += text
36+
} else {
37+
placeholders.latest.contents += text
38+
}
39+
40+
case let .curlyBrace(brace):
41+
if placeholders.isEmpty {
42+
result.append(brace)
43+
} else {
44+
// Braces are only escaped _inside_ a placeholder; otherwise the client
45+
// would include the backslashes literally.
46+
placeholders.latest.contents.append(contentsOf: ["\\", brace])
47+
}
48+
49+
case .placeholderOpen:
50+
placeholders.append((number: nextPlaceholderNumber, contents: ""))
51+
nextPlaceholderNumber += 1
52+
53+
case .placeholderClose:
54+
guard let (number, placeholderBody) = placeholders.popLast() else {
55+
logger.fault("Invalid placeholder in \(input)")
56+
return input
57+
}
58+
guard let displayName = nameForSnippet(placeholderBody) else {
59+
logger.fault("Failed to decode placeholder \(placeholderBody) in \(input)")
60+
return input
61+
}
62+
let placeholder =
63+
clientSupportsSnippets
64+
? formatLSPPlaceholder(displayName, number: number)
65+
: ""
66+
if placeholders.isEmpty {
67+
result += placeholder
68+
} else {
69+
placeholders.latest.contents += placeholder
70+
}
2971
}
30-
let snippet = clientSupportsSnippets ? "${\(index):\(displayName)}" : ""
31-
result.replaceSubrange(start.lowerBound..<end.upperBound, with: snippet)
32-
index += 1
3372
}
73+
3474
return result
3575
}
3676

37-
/// Parse a SourceKit placeholder and extract the display name suitable for a
38-
/// LSP snippet.
39-
fileprivate func nameForSnippet(_ text: String) -> String? {
40-
var text = text
77+
/// Scan `input` to identify special elements within: curly braces, which may
78+
/// need to be escaped; and SourceKit placeholder open/close delimiters.
79+
private func tokenize(_ input: String) -> [SnippetToken] {
80+
var index = input.startIndex
81+
var isAtEnd: Bool { index == input.endIndex }
82+
func match(_ char: Character) -> Bool {
83+
if isAtEnd || input[index] != char {
84+
return false
85+
} else {
86+
input.formIndex(after: &index)
87+
return true
88+
}
89+
}
90+
func next() -> Character? {
91+
guard !isAtEnd else { return nil }
92+
defer { input.formIndex(after: &index) }
93+
return input[index]
94+
}
95+
96+
var tokens: [SnippetToken] = []
97+
var text = ""
98+
while let char = next() {
99+
switch char {
100+
case "<":
101+
if match("#") {
102+
tokens.append(.text(text))
103+
text.removeAll()
104+
tokens.append(.placeholderOpen)
105+
} else {
106+
text.append(char)
107+
}
108+
109+
case "#":
110+
if match(">") {
111+
tokens.append(.text(text))
112+
text.removeAll()
113+
tokens.append(.placeholderClose)
114+
} else {
115+
text.append(char)
116+
}
117+
118+
case "{", "}":
119+
tokens.append(.text(text))
120+
text.removeAll()
121+
tokens.append(.curlyBrace(char))
122+
123+
case let c:
124+
text.append(c)
125+
}
126+
}
127+
128+
tokens.append(.text(text))
129+
130+
return tokens
131+
}
132+
133+
/// A syntactical element inside a SourceKit snippet.
134+
private enum SnippetToken {
135+
/// A placeholder delimiter.
136+
case placeholderOpen, placeholderClose
137+
/// One of '{' or '}', which may need to be escaped in the output.
138+
case curlyBrace(Character)
139+
/// Any other consecutive run of characters from the input, which needs no
140+
/// special treatment.
141+
case text(String)
142+
}
143+
144+
/// Given the interior text of a SourceKit placeholder, extract a display name
145+
/// suitable for a LSP snippet.
146+
private func nameForSnippet(_ body: String) -> String? {
147+
var text = rewrappedAsPlaceholder(body)
41148
return text.withSyntaxText {
42149
guard let data = RawEditorPlaceholderData(syntaxText: $0) else {
43150
return nil
44151
}
45152
return String(syntaxText: data.typeForExpansionText ?? data.displayText)
46153
}
47154
}
155+
156+
private let placeholderStart = "<#"
157+
private let placeholderEnd = "#>"
158+
private func rewrappedAsPlaceholder(_ body: String) -> String {
159+
return placeholderStart + body + placeholderEnd
160+
}
161+
162+
/// Wrap `body` in LSP snippet placeholder syntax, using `number` as the
163+
/// placeholder's index in the snippet.
164+
private func formatLSPPlaceholder(_ body: String, number: Int) -> String {
165+
"${\(number):\(body)}"
166+
}
167+
168+
private extension Array {
169+
/// Mutable access to the final element of an array.
170+
///
171+
/// - precondition: The array must not be empty.
172+
var latest: Element {
173+
get { self.last! }
174+
_modify {
175+
let index = self.index(before: self.endIndex)
176+
yield &self[index]
177+
}
178+
}
179+
}

0 commit comments

Comments
 (0)