Skip to content

Fetch full documentation in code completion #2207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Sources/Csourcekitd/include/CodeCompletionSwiftInterop.h
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,12 @@ typedef struct {
void (^_Null_unspecified handler)(const char *_Null_unspecified)
);

void (*_Nullable completion_item_get_doc_full)(
_Nonnull swiftide_api_completion_response_t,
_Nonnull swiftide_api_completion_item_t,
void (^_Nonnull handler)(const char *_Nullable)
);

void (*_Nonnull completion_item_get_associated_usrs)(
_Null_unspecified swiftide_api_completion_response_t,
_Null_unspecified swiftide_api_completion_item_t,
Expand Down
33 changes: 26 additions & 7 deletions Sources/SKTestSupport/SkipUnless.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//

import Csourcekitd
import Foundation
import LanguageServerProtocol
import LanguageServerProtocolExtensions
Expand Down Expand Up @@ -250,13 +251,8 @@ package actor SkipUnless {
line: UInt = #line
) async throws {
return try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 2), file: file, line: line) {
guard let sourcekitdPath = await ToolchainRegistry.forTesting.default?.sourcekitd else {
throw GenericError("Could not find SourceKitD")
}
let sourcekitd = try await SourceKitD.getOrCreate(
dylibPath: sourcekitdPath,
pluginPaths: try sourceKitPluginPaths
)
let sourcekitd = try await getSourceKitD()

do {
let response = try await sourcekitd.send(
\.codeCompleteSetPopularAPI,
Expand All @@ -274,6 +270,17 @@ package actor SkipUnless {
}
}

package static func sourcekitdSupportsFullDocumentationInCompletion(
file: StaticString = #filePath,
line: UInt = #line
) async throws {
return try await shared.skipUnlessSupportedByToolchain(swiftVersion: SwiftVersion(6, 2), file: file, line: line) {
let sourcekitd = try await getSourceKitD()

return sourcekitd.ideApi.completion_item_get_doc_full != nil
}
}

package static func canLoadPluginsBuiltByToolchain(
file: StaticString = #filePath,
line: UInt = #line
Expand Down Expand Up @@ -352,6 +359,18 @@ package actor SkipUnless {
return .featureSupported
}
}

private static func getSourceKitD() async throws -> SourceKitD {
guard let sourcekitdPath = await ToolchainRegistry.forTesting.default?.sourcekitd else {
throw GenericError("Could not find SourceKitD")
}
let sourcekitd = try await SourceKitD.getOrCreate(
dylibPath: sourcekitdPath,
pluginPaths: try sourceKitPluginPaths
)

return sourcekitd
}
}

// MARK: - Parsing Swift compiler version
Expand Down
1 change: 1 addition & 0 deletions Sources/SourceKitD/sourcekitd_functions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ extension sourcekitd_ide_api_functions_t {
completion_item_get_source_text: try loadRequired("swiftide_completion_item_get_source_text"),
completion_item_get_type_name: try loadRequired("swiftide_completion_item_get_type_name"),
completion_item_get_doc_brief: try loadRequired("swiftide_completion_item_get_doc_brief"),
completion_item_get_doc_full: loadOptional("swiftide_completion_item_get_doc_full"),
completion_item_get_associated_usrs: try loadRequired("swiftide_completion_item_get_associated_usrs"),
completion_item_get_kind: try loadRequired("swiftide_completion_item_get_kind"),
completion_item_get_associated_kind: try loadRequired("swiftide_completion_item_get_associated_kind"),
Expand Down
14 changes: 13 additions & 1 deletion Sources/SourceKitLSP/Swift/CodeCompletionSession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -574,13 +574,25 @@ class CodeCompletionSession {
fileContents: nil
)
}
if let docString: String = documentationResponse?[sourcekitd.keys.docBrief] {

if let response = documentationResponse,
let docString = documentationString(from: response, sourcekitd: sourcekitd) {
item.documentation = .markupContent(MarkupContent(kind: .markdown, value: docString))
}
}
return item
}

private static func documentationString(from response: SKDResponseDictionary, sourcekitd: SourceKitD) -> String? {
if let docFullAsXML: String = response[sourcekitd.keys.docFullAsXML] {
return orLog("Converting XML documentation to markdown") {
try xmlDocumentationToMarkdown(docFullAsXML)
}
}

return response[sourcekitd.keys.docBrief]
}

private func computeCompletionTextEdit(
completionPos: Position,
requestPosition: Position,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,16 @@ struct ExtendedCompletionInfo {
return result
}

var fullDocumentation: String? {
var result: String? = nil
session.sourcekitd.ideApi.completion_item_get_doc_full?(session.response, rawItem) {
if let cstr = $0 {
result = String(cString: cstr)
}
}
return result
}

var associatedUSRs: [String] {
var result: [String] = []
session.sourcekitd.ideApi.completion_item_get_associated_usrs(session.response, rawItem) { ptr, len in
Expand Down
13 changes: 10 additions & 3 deletions Sources/SwiftSourceKitPlugin/CompletionProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,17 @@ actor CompletionProvider {
func handleCompletionDocumentation(_ request: SKDRequestDictionaryReader) throws -> SKDResponseDictionaryBuilder {
let info = try handleExtendedCompletionRequest(request)

return request.sourcekitd.responseDictionary([
request.sourcekitd.keys.docBrief: info.briefDocumentation,
request.sourcekitd.keys.associatedUSRs: info.associatedUSRs as [SKDResponseValue]?,
let response = request.sourcekitd.responseDictionary([
request.sourcekitd.keys.associatedUSRs: info.associatedUSRs as [SKDResponseValue]?
])

if let fullDocumentation = info.fullDocumentation {
response.set(request.sourcekitd.keys.docFullAsXML, to: fullDocumentation)
} else {
response.set(request.sourcekitd.keys.docBrief, to: info.briefDocumentation)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open question: Does it make sense to also return the brief documentation in addition to the full documentation? Eg. an editor might want to display a brief documentation of the symbol by default and then expand that to full documentation once the user performs an action.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a specific flow in mind? AFAIK this doesn't happen in code completion since documentation is only shown when the completion item is selected in the completions list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, you could think about a UI where the brief documentation is displayed in the completion list (similar to how Xcode shows documentation) and only showing the full documentation when the user requests it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the plugin is used in SourceKit-LSP only, no? If so, then we don't have to return the brief documentation now as we don't need it. We can always add it later if we found a use case for it.

If not, it'd probably make sense to return both yes, especially if it's used by Xcode which already has this use case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plugin can potentially be used by any SourceKit client yes, so I think we should return both by default. We can always add options to the request for cases where the client only wants the brief or full doc

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it makes sense, thanks for the clarification @ahoppen and @hamishknight 🙏🏼

I've changed the request to return both now.

}

return response
}

func handleCompletionDiagnostic(_ dict: SKDRequestDictionaryReader) throws -> SKDResponseDictionaryBuilder {
Expand Down
70 changes: 65 additions & 5 deletions Tests/SourceKitLSPTests/SwiftCompletionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ final class SwiftCompletionTests: XCTestCase {

func testCompletionBasic() async throws {
try await SkipUnless.sourcekitdSupportsPlugin()
try await SkipUnless.sourcekitdSupportsFullDocumentationInCompletion()

let testClient = try await TestSourceKitLSPClient()
let uri = DocumentURI(for: .swift)
Expand Down Expand Up @@ -67,7 +68,15 @@ final class SwiftCompletionTests: XCTestCase {
if let abc = abc {
XCTAssertEqual(abc.kind, .property)
XCTAssertEqual(abc.detail, "Int")
XCTAssertEqual(abc.documentation, .markupContent(MarkupContent(kind: .markdown, value: "Documentation for abc.")))
assertMarkdown(
documentation: abc.documentation,
expected: """
```swift
var abc: Int
```
Documentation for `abc`.
"""
)
XCTAssertEqual(abc.filterText, "abc")
XCTAssertEqual(abc.textEdit, .textEdit(TextEdit(range: Range(positions["1️⃣"]), newText: "abc")))
XCTAssertEqual(abc.insertText, "abc")
Expand All @@ -87,7 +96,15 @@ final class SwiftCompletionTests: XCTestCase {
// If we switch to server-side filtering this will change.
XCTAssertEqual(abc.kind, .property)
XCTAssertEqual(abc.detail, "Int")
XCTAssertEqual(abc.documentation, .markupContent(MarkupContent(kind: .markdown, value: "Documentation for abc.")))
assertMarkdown(
documentation: abc.documentation,
expected: """
```swift
var abc: Int
```
Documentation for `abc`.
"""
)
XCTAssertEqual(abc.filterText, "abc")
XCTAssertEqual(abc.textEdit, .textEdit(TextEdit(range: positions["1️⃣"]..<offsetPosition, newText: "abc")))
XCTAssertEqual(abc.insertText, "abc")
Expand Down Expand Up @@ -1154,6 +1171,7 @@ final class SwiftCompletionTests: XCTestCase {

func testCompletionItemResolve() async throws {
try await SkipUnless.sourcekitdSupportsPlugin()
try await SkipUnless.sourcekitdSupportsFullDocumentationInCompletion()

let capabilities = ClientCapabilities(
textDocument: TextDocumentClientCapabilities(
Expand Down Expand Up @@ -1187,9 +1205,42 @@ final class SwiftCompletionTests: XCTestCase {
let item = try XCTUnwrap(completions.items.only)
XCTAssertNil(item.documentation)
let resolvedItem = try await testClient.send(CompletionItemResolveRequest(item: item))
XCTAssertEqual(
resolvedItem.documentation,
.markupContent(MarkupContent(kind: .markdown, value: "Creates a true value"))
assertMarkdown(
documentation: resolvedItem.documentation,
expected: """
```swift
func makeBool() -> Bool
```
Creates a true value
"""
)
}

func testCompletionBriefDocumentationFallback() async throws {
try await SkipUnless.sourcekitdSupportsPlugin()
try await SkipUnless.sourcekitdSupportsFullDocumentationInCompletion()

let testClient = try await TestSourceKitLSPClient()
let uri = DocumentURI(for: .swift)

// We test completion for result builder build functions since they don't have full documentation
// but still have brief documentation.
let positions = testClient.openDocument(
"""
@resultBuilder
struct AnyBuilder {
static func 1️⃣
}
""",
uri: uri
)
let completions = try await testClient.send(
CompletionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
)
let item = try XCTUnwrap(completions.items.filter { $0.label.contains("buildBlock") }.only)
assertMarkdown(
documentation: item.documentation,
expected: "Required by every result builder to build combined results from statement blocks"
)
}

Expand Down Expand Up @@ -1253,6 +1304,15 @@ private func countFs(_ response: CompletionList) -> Int {
return response.items.filter { $0.label.hasPrefix("f") }.count
}

private func assertMarkdown(
documentation: StringOrMarkupContent?,
expected: String,
file: StaticString = #filePath,
line: UInt = #line
) {
XCTAssertEqual(documentation, .markupContent(MarkupContent(kind: .markdown, value: expected)))
}

fileprivate extension Position {
func adding(columns: Int) -> Position {
return Position(line: line, utf16index: utf16index + columns)
Expand Down
38 changes: 34 additions & 4 deletions Tests/SwiftSourceKitPluginTests/SwiftSourceKitPluginTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ final class SwiftSourceKitPluginTests: XCTestCase {

func testBasicCompletion() async throws {
try await SkipUnless.sourcekitdSupportsPlugin()
try await SkipUnless.sourcekitdSupportsFullDocumentationInCompletion()

let sourcekitd = try await getSourceKitD()
let path = scratchFilePath()
let positions = try await sourcekitd.openDocument(
Expand Down Expand Up @@ -203,7 +205,8 @@ final class SwiftSourceKitPluginTests: XCTestCase {
XCTAssertEqual(result2.items.count, 1)
XCTAssertEqual(result2.items[0].name, "")
let doc = try await sourcekitd.completeDocumentation(id: result2.items[0].id)
XCTAssertEqual(doc.docBrief, nil)
XCTAssertNil(doc.docFullAsXML)
XCTAssertNil(doc.docBrief)
}

func testMultipleFiles() async throws {
Expand Down Expand Up @@ -403,6 +406,8 @@ final class SwiftSourceKitPluginTests: XCTestCase {

func testDocumentation() async throws {
try await SkipUnless.sourcekitdSupportsPlugin()
try await SkipUnless.sourcekitdSupportsFullDocumentationInCompletion()

let sourcekitd = try await getSourceKitD()
let path = scratchFilePath()
let positions = try await sourcekitd.openDocument(
Expand Down Expand Up @@ -436,15 +441,38 @@ final class SwiftSourceKitPluginTests: XCTestCase {
let sym3 = try unwrap(result.items.first(where: { $0.name == "foo3()" }), "did not find foo3; got \(result.items)")

let sym1Doc = try await sourcekitd.completeDocumentation(id: sym1.id)
XCTAssertEqual(sym1Doc.docBrief, "Protocol P foo1")
XCTAssertEqual(sym1Doc.docFullAsXML,
"""
<Function file="\(path)" line="3" column="8">\
<Name>foo1()</Name>\
<USR>s:1a1PP4foo1yyF</USR>\
<Declaration>func foo1()</Declaration>\
<CommentParts>\
<Abstract><Para>Protocol P foo1</Para></Abstract>\
<Discussion><Note>\
<Para>This documentation comment was inherited from <codeVoice>P</codeVoice>.</Para>\
</Note></Discussion>\
</CommentParts>\
</Function>
""")
XCTAssertEqual(sym1Doc.associatedUSRs, ["s:1a1SV4foo1yyF", "s:1a1PP4foo1yyF"])

let sym2Doc = try await sourcekitd.completeDocumentation(id: sym2.id)
XCTAssertEqual(sym2Doc.docBrief, "Struct S foo2")
XCTAssertEqual(sym2Doc.docFullAsXML,
"""
<Function file="\(path)" line="8" column="8">\
<Name>foo2()</Name>\
<USR>s:1a1SV4foo2yyF</USR>\
<Declaration>func foo2()</Declaration>\
<CommentParts>\
<Abstract><Para>Struct S foo2</Para></Abstract>\
</CommentParts>\
</Function>
""")
XCTAssertEqual(sym2Doc.associatedUSRs, ["s:1a1SV4foo2yyF"])

let sym3Doc = try await sourcekitd.completeDocumentation(id: sym3.id)
XCTAssertNil(sym3Doc.docBrief)
XCTAssertNil(sym3Doc.docFullAsXML)
XCTAssertEqual(sym3Doc.associatedUSRs, ["s:1a1SV4foo3yyF"])
}

Expand Down Expand Up @@ -1766,11 +1794,13 @@ fileprivate struct CompletionResult: Equatable, Sendable {
}

fileprivate struct CompletionDocumentation {
var docFullAsXML: String? = nil
var docBrief: String? = nil
var associatedUSRs: [String] = []

init(_ dict: SKDResponseDictionary) {
let keys = dict.sourcekitd.keys
self.docFullAsXML = dict[keys.docFullAsXML]
self.docBrief = dict[keys.docBrief]
self.associatedUSRs = dict[keys.associatedUSRs]?.asStringArray ?? []
}
Expand Down