Skip to content

[IDE] Add full documentation to code completion result #82464

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 58 commits into
base: main
Choose a base branch
from

Conversation

a7medev
Copy link
Contributor

@a7medev a7medev commented Jun 24, 2025

Retrieve and return full documentation in code completion results and expose it in:

  • SourceKit plugin through swiftide_completion_item_get_doc_full
  • Code completion request (used by sourcekitd-test though not used in SourceKit LSP) in the key.doc.full_as_xml key.

This PR depends on #82887.

@a7medev a7medev force-pushed the feat/full-documentation-in-code-completion branch from 314f83c to b70fcc7 Compare July 11, 2025 11:36
@a7medev a7medev changed the title [IDE] Add full documentation to code completion result [WIP] [IDE] Add full documentation to code completion result Jul 11, 2025
@a7medev a7medev marked this pull request as draft July 11, 2025 11:43
@a7medev a7medev requested a review from hamishknight July 25, 2025 17:53
@a7medev
Copy link
Contributor Author

a7medev commented Jul 25, 2025

@rintaro @hamishknight I've reverted changes to SourceKit tests (except for invalid redeclaration fixes) and added some tests under test/IDE. Can you please recheck? 🙏🏼

@rintaro
Copy link
Member

rintaro commented Jul 25, 2025

@swift-ci Please test

@a7medev
Copy link
Contributor Author

a7medev commented Jul 25, 2025

Some tests are failing due to failures in USR to Decl verification. I suspect this is the same issue we had with extension binding causing the creation of an incomplete SourceLookupCache for the module. 3 tests may be related to extension binding (though I haven't confirmed), 1 doesn't have any extensions.

I'm looking into it.

a7medev added 2 commits July 26, 2025 18:29
…less extension

The USR for declarations within the extension with no type name have a USR that looks like a top-level declaration causing the lookup within swift::Demangle::getDeclForUSR to fail.
@a7medev
Copy link
Contributor Author

a7medev commented Jul 26, 2025

Failing tests can be categorized into 3 buckets:

  1. IDE/crashers_fixed/003-swift-typebase-getcanonicaltype.swift, IDE/crashers_fixed/006-swift-declcontext-getprotocolself.swift, and IDE/crashers_fixed/064-swift-genericfunctiontype-get.swift: these have decls within an extension with no type name (extension {), causing these decls to have a USR that does looks like a top-level decl's USR, which causes the lookup inside Demangle::getDeclForUSR to fail. There's not much, in the scope of this PR, that we can do here, so I disabled USR to Decl verification for these tests.

  2. IDE/crashers_fixed/093-swift-valuedecl-getformalaccessscope.swift: the test has an (invalid) function operator decl func <() (created by parser after recovery), we didn't handle operators in DeclNameExtractor since their FuncDecls weren't passed to CodeCompletionResultBuilder::setAssociatedDecl but rather their OperatorDecls which is not a ValueDecl. I added support for operator name extraction in DeclNameExtractor to fix this and also for future changes where we pass the operator's FuncDecl rather than OperatorDecl to CodeCompletionResultBuilder (this will allow us to show brief & full documentation for operators which is broken now).

  3. IDE/complete_from_cocoa.swift, IDE/complete_from_cocoa_2.swift, and IDE/complete_sdk_platform.swift: USR to Decl reconstruction fails since:

    1. The EnumDecl for an Objective-C enum (AppKit.NSLineBreakMode) is added to completion results and USR to Decl verification kicks off (with Swift USR: s:So15NSLineBreakModeV).
    2. When we try to find the NSLineBreakMode type using createTypeDecl, ClangImporter::lookupTypeDecl does find the clang::EnumDecl but when it imports it using ClangImporter::Implementation::importDecl it returns a TypeAliasDecl with the name LineBreakMode instead (a type-alias to the real enum).
    3. Verification fails since the original EnumDecl != the reconstructed TypeAliasDecl crashing the tests.

    I'm still not sure why this behavior happens, and whether we should fix it in this PR or leave it for now.

@a7medev a7medev force-pushed the feat/full-documentation-in-code-completion branch from df0a177 to fe26400 Compare July 28, 2025 13:31
@hamishknight
Copy link
Contributor

@swift-ci please test

@a7medev
Copy link
Contributor Author

a7medev commented Jul 28, 2025

@hamishknight It's going to fail again since we've neither addressed nor muted the issues in tests that import Cocoa, I think we can cancel it until we decide on whether we're going to put time into fixing this or if we're gonna skip it for now. wdyt?

@hamishknight
Copy link
Contributor

I know, I just wanted to run the tests on the operator name change

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Thanks!

@a7medev a7medev force-pushed the feat/full-documentation-in-code-completion branch from 5adc3b4 to 6b75749 Compare August 4, 2025 12:44
Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Let me double check with the Clang Importer folks that they're happy with the compatibility alias change, but this LGTM thank you!

@a7medev
Copy link
Contributor Author

a7medev commented Aug 5, 2025

@hamishknight macOS and Linux tests don't seem to be running, any idea why this is happening?

@hamishknight
Copy link
Contributor

Weird, let's try again

@swift-ci please test

@a7medev
Copy link
Contributor Author

a7medev commented Aug 5, 2025

@hamishknight They're failing now. There seems to be an issue in the shell script it's trying to run (an unmatched single quote).

/Users/ec2-user/jenkins/workspace/swift-PR-macos@tmp/durable-de9ff1d2/script.sh.copy: line 1: unexpected EOF while looking for matching `''

@hamishknight
Copy link
Contributor

@swift-ci please test

@a7medev a7medev force-pushed the feat/full-documentation-in-code-completion branch from 6b75749 to 380b304 Compare August 16, 2025 21:01
@a7medev
Copy link
Contributor Author

a7medev commented Aug 16, 2025

I've rebased the PR on main to get changes from #83613 and removed its related FIXMEs.
I also added a way to retrieve the raw documentation (swiftide_completion_item_get_doc_raw) and renamed swiftide_completion_item_get_doc_full to swiftide_completion_item_get_doc_full_as_xml for clarity since SourceKit-LSP has issues rendering the XML documentation to markdown and we'll switch to using the raw comment similar to what the hover LSP request (cursor info in SourceKitD) does.

One more side change: I added a -code-completion-sort-by-name option to swift-ide-test so we can have a simple deterministic order to use in documentation tests and remove the use of CHECK-DAG allowing us to use CHECK-SAME, CHECK-NEXT, and CHECK-EMPTY to do more complex matching required for multiline raw doc comment and splitting long single-line checks since CHECK-DAG doesn't compose well with these pragmas.

@hamishknight @rintaro Can you please re-review and run the tests? 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants