From 968ff0f4e2732764b06a28b06f719b4b6503ef22 Mon Sep 17 00:00:00 2001 From: Josh Caswell Date: Sat, 26 Oct 2024 20:59:07 -0700 Subject: [PATCH] Add config option for trailing closure rewriting Default to the automatic expansion introduced in 90c124c369fa2938908508ee4c540033f95761e3 but allow the user to disable it via a configuration file. This is expressed as an enumeration rather than a boolean flag because there is room for other levels of rewriting. E.g. for trailing closures, a "basic" level might be implemented as rewriting to a pair of brackets on the same line, with a single placeholder, rather than the "full" multi-line behavior. --- Documentation/Configuration File.md | 2 + Sources/SKOptions/SourceKitLSPOptions.swift | 39 +++++++++ .../TestSourceKitLSPClient.swift | 10 +++ .../Swift/CodeCompletionSession.swift | 9 +- .../SwiftCompletionTests.swift | 87 +++++++++++++++++++ 5 files changed, 145 insertions(+), 2 deletions(-) diff --git a/Documentation/Configuration File.md b/Documentation/Configuration File.md index 4f6b31f70..8d5542240 100644 --- a/Documentation/Configuration File.md +++ b/Documentation/Configuration File.md @@ -39,6 +39,8 @@ The structure of the file is currently not guaranteed to be stable. Options may - `indexPrefixMap: [string: string]`: Path remappings for remapping index data for local use. - `maxCoresPercentageToUseForBackgroundIndexing: double`: A hint indicating how many cores background indexing should use at most (value between 0 and 1). Background indexing is not required to honor this setting - `updateIndexStoreTimeout: int`: Number of seconds to wait for an update index store task to finish before killing it. +- `codeCompletion`: Dictionary with the following keys, defining options related to code completion actions + - `rewriteTrailingClosures: "full"|"never"`: Whether to pre-expand trailing closures when completing a function call expression - `logging`: Dictionary with the following keys, changing SourceKit-LSP’s logging behavior on non-Apple platforms. On Apple platforms, logging is done through the [system log](Diagnose%20Bundle.md#Enable%20Extended%20Logging). These options can only be set globally and not per workspace. - `logLevel: "debug"|"info"|"default"|"error"|"fault"`: The level from which one onwards log messages should be written. - `privacyLevel: "public"|"private"|"sensitive"`: Whether potentially sensitive information should be redacted. Default is `public`, which redacts potentially sensitive information. diff --git a/Sources/SKOptions/SourceKitLSPOptions.swift b/Sources/SKOptions/SourceKitLSPOptions.swift index 58e0065b1..4620922f1 100644 --- a/Sources/SKOptions/SourceKitLSPOptions.swift +++ b/Sources/SKOptions/SourceKitLSPOptions.swift @@ -219,6 +219,33 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable { } } + /// User settings controlling code completion. + public struct CodeCompletionOptions: Sendable, Codable, Equatable { + /// The extent to which a completion action should apply stylistic or + /// convenience transformations before passing a result to the client. + public enum RewriteLevel: String, Sendable, Codable { + case full, never + } + + /// Whether trailing closures should be eagerly expanded by SourceKit-LSP + /// before being passed to the client. + public var rewriteTrailingClosures: RewriteLevel + + public init(rewriteTrailingClosures: RewriteLevel = .full) { + self.rewriteTrailingClosures = rewriteTrailingClosures + } + + static func merging( + base: CodeCompletionOptions, + override: CodeCompletionOptions? + ) -> CodeCompletionOptions { + return CodeCompletionOptions( + rewriteTrailingClosures: override?.rewriteTrailingClosures + ?? base.rewriteTrailingClosures + ) + } + } + public enum BackgroundPreparationMode: String { /// Build a target to prepare it case build @@ -271,6 +298,12 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable { set { logging = newValue } } + private var codeCompletion: CodeCompletionOptions? + public var codeCompletionOrDefault: CodeCompletionOptions { + get { codeCompletion ?? .init() } + set { codeCompletion = newValue } + } + /// Default workspace type (buildserver|compdb|swiftpm). Overrides workspace type selection logic. public var defaultWorkspaceType: WorkspaceType? public var generatedFilesPath: String? @@ -349,6 +382,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable { clangdOptions: [String]? = nil, index: IndexOptions = .init(), logging: LoggingOptions = .init(), + codeCompletion: CodeCompletionOptions? = nil, defaultWorkspaceType: WorkspaceType? = nil, generatedFilesPath: String? = nil, backgroundIndexing: Bool? = nil, @@ -365,6 +399,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable { self.clangdOptions = clangdOptions self.index = index self.logging = logging + self.codeCompletion = codeCompletion self.generatedFilesPath = generatedFilesPath self.defaultWorkspaceType = defaultWorkspaceType self.backgroundIndexing = backgroundIndexing @@ -420,6 +455,10 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable { clangdOptions: override?.clangdOptions ?? base.clangdOptions, index: IndexOptions.merging(base: base.indexOrDefault, override: override?.index), logging: LoggingOptions.merging(base: base.loggingOrDefault, override: override?.logging), + codeCompletion: CodeCompletionOptions.merging( + base: base.codeCompletionOrDefault, + override: override?.codeCompletion + ), defaultWorkspaceType: override?.defaultWorkspaceType ?? base.defaultWorkspaceType, generatedFilesPath: override?.generatedFilesPath ?? base.generatedFilesPath, backgroundIndexing: override?.backgroundIndexing ?? base.backgroundIndexing, diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index 935247846..9f2772c04 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -46,6 +46,16 @@ extension SourceKitLSPOptions { } } +extension SourceKitLSPOptions.CodeCompletionOptions { + package static func testDefault( + rewriteTrailingClosures: RewriteLevel = .full + ) -> SourceKitLSPOptions.CodeCompletionOptions { + return SourceKitLSPOptions.CodeCompletionOptions( + rewriteTrailingClosures: rewriteTrailingClosures + ) + } +} + fileprivate struct NotificationTimeoutError: Error, CustomStringConvertible { var description: String = "Failed to receive next notification within timeout" } diff --git a/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift b/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift index d419cfcfa..f0839a2e6 100644 --- a/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift +++ b/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift @@ -363,8 +363,13 @@ class CodeCompletionSession { let docBrief: String? = value[sourcekitd.keys.docBrief] let utf8CodeUnitsToErase: Int = value[sourcekitd.keys.numBytesToErase] ?? 0 - if let closureExpanded = expandClosurePlaceholders(insertText: insertText) { - insertText = closureExpanded + switch options.codeCompletionOrDefault.rewriteTrailingClosures { + case .full: + if let closureExpanded = expandClosurePlaceholders(insertText: insertText) { + insertText = closureExpanded + } + case .never: + break } let text = rewriteSourceKitPlaceholders(in: insertText, clientSupportsSnippets: clientSupportsSnippets) diff --git a/Tests/SourceKitLSPTests/SwiftCompletionTests.swift b/Tests/SourceKitLSPTests/SwiftCompletionTests.swift index 4ac1f2baf..fd5372d60 100644 --- a/Tests/SourceKitLSPTests/SwiftCompletionTests.swift +++ b/Tests/SourceKitLSPTests/SwiftCompletionTests.swift @@ -13,6 +13,7 @@ import LanguageServerProtocol import SKTestSupport import SourceKitLSP +import SKOptions import XCTest final class SwiftCompletionTests: XCTestCase { @@ -1036,6 +1037,92 @@ final class SwiftCompletionTests: XCTestCase { ) } + func testExpandClosuresDisabledByConfig() async throws { + let testClient = try await TestSourceKitLSPClient( + options: SourceKitLSPOptions(codeCompletion: .testDefault(rewriteTrailingClosures: .never)), + capabilities: snippetCapabilities + ) + let uri = DocumentURI(for: .swift) + let positions = testClient.openDocument( + """ + struct MyArray { + func myMap(_ body: (Int) -> Bool) {} + } + func test(x: MyArray) { + x.1️⃣ + } + """, + uri: uri + ) + let completions = try await testClient.send( + CompletionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + XCTAssertEqual( + completions.items.filter { $0.label.contains("myMap") }, + [ + CompletionItem( + label: "myMap(body: (Int) -> Bool)", + kind: .method, + detail: "Void", + deprecated: false, + sortText: nil, + filterText: "myMap(:)", + insertText: "myMap(${1:(Int) -> Bool})", + insertTextFormat: .snippet, + textEdit: .textEdit( + TextEdit( + range: Range(positions["1️⃣"]), + newText: "myMap(${1:(Int) -> Bool})" + ) + ) + ) + ] + ) + } + + func testExpandMultipleTrailingClosuresDisabledByConfig() async throws { + let testClient = try await TestSourceKitLSPClient( + options: SourceKitLSPOptions(codeCompletion: .testDefault(rewriteTrailingClosures: .never)), + capabilities: snippetCapabilities + ) + let uri = DocumentURI(for: .swift) + let positions = testClient.openDocument( + """ + struct MyArray { + func myMap(_ body: (Int) -> Bool, second: (Int) -> String) {} + } + func test(x: MyArray) { + x.1️⃣ + } + """, + uri: uri + ) + let completions = try await testClient.send( + CompletionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + XCTAssertEqual( + completions.items.filter { $0.label.contains("myMap") }, + [ + CompletionItem( + label: "myMap(body: (Int) -> Bool, second: (Int) -> String)", + kind: .method, + detail: "Void", + deprecated: false, + sortText: nil, + filterText: "myMap(:second:)", + insertText: "myMap(${1:(Int) -> Bool}, second: ${2:(Int) -> String})", + insertTextFormat: .snippet, + textEdit: .textEdit( + TextEdit( + range: Range(positions["1️⃣"]), + newText: "myMap(${1:(Int) -> Bool}, second: ${2:(Int) -> String})" + ) + ) + ) + ] + ) + } + func testInferIndentationWhenExpandingClosurePlaceholder() async throws { let testClient = try await TestSourceKitLSPClient(capabilities: snippetCapabilities) let uri = DocumentURI(for: .swift)