Implement completionItem/resolve

[LebJe]

This pull request adds support for completionItem/resolve to SourceKit-LSP.

Unfortunately, the tests are disabled (and this PR is a draft) because SourceKit cannot provide cursorinfo via an USR for anything other than things like structs:

{
  key.internal_diagnostic: "Unable to resolve type from USR."
}

Test Code:

/// `a` (does not work)
let a = 1

/// `b` (does not work)
func b(c: Int) {}

/// `E` (works)
enum E: CaseIterable {
	/// `a` (does not work)
	case a
	
	/// `b` (does not work)
	case b
	
	/// `c` (does not work)
	case c
}

/// `A` (works)
struct A: Equatable {
	
	/// `a` (does not work)
	let a: String
		
  	/// - Parameters:
  	///    - a: `String`
	/// - Note: does not work
	init(a: String) {
		self.a = a
	}
}

To test the above code, type something like E.a, or A.init, and see if the `` is rendered as a code block in the completion popup.

Perhaps this is related to this issue.

[ahoppen]

Thanks a lot for looking into this @LebJe. I’ll need to look into why sourcektid doesn’t return cursor info for the USR. Maybe @benlangmuir has an idea if this is something that’s supported or what might be going wrong here from the top of his head.

For now, I’ve added some comments inline.

[ahoppen]

I don’t think we should make any assumptions about the requests sent to us by the client and, in particular, not crash if it is malformed. Could we return a failure value (e.g.nil) here instead of fatalErroring?

[LebJe]

Is TextDocumentIdentifier(DocumentURI(string: "")) ok?

[ahoppen]

I think you are not using this computed property at all, so let’s just remove it.

[ahoppen]

I think the method property is always the first property in a request type. Could you move it up to match this style?

[benlangmuir]

Unfortunately, the tests are disabled (and this PR is a draft) because SourceKit cannot provide cursorinfo via an USR for anything other than things like structs:

Cursor info on USRs only works on a subset of USRs for nominal types IIRC. Please don't use this functionality. We should never have added it.

The way I would implement this would be

1. Use the existing sourcekitd completion items from the active completion session to fill in the data (if we aren't already keeping them alive, we could). This allows reducing the data we send in the original completion response (notably not serializing the brief documentation string).
2. Over time we can make this more lazy by avoiding computing the full completion item in sourcekitd up front and add new requests to query the details.

[LebJe]

@benlangmuir, are you saying I should:

• Have a Dictionary<String, SKDResponseDictionary> that stores the codecomplete results.
• Rewrite completionsFromSKDResponse so that it:
  • Clears the old completions form the dictionary, and fills it with <unique id>: <completion response>.
  • Returns the minimum amount of information to the LSP client.
• Rewrite _completionResolve so that it:
  • Uses the completion dictionary to fill in the rest of the data, and do any necessary processing (like rewriting placeholders).

With this approach though, it is still impossible to get full documentation.

Is there something I'm not noticing in the codecomplete results that would help me get the full documentation?

---

Cursor info on USRs only works on a subset of USRs for nominal types IIRC. Please don't use this functionality. We should never have added it.

Was the implementation never finished, or is something causing cursor info via USRs on things other than nominal types to not work?

[ahoppen]

I think you are not using this computed property at all, so let’s just remove it.

[ahoppen]

Is there a reason why you need to refactor kind anymore? I think the previous version read better 😉

[ahoppen]

Would it make sense to define the magic "textDocURI" string as a static property on CompletionItem instead of repeating it throughout the codebase? IIUC it’s not part of the LSP standard but just a key that you’re using for the opaque data dictionary.
Same for "usr"

[benlangmuir]

@benlangmuir, are you saying I should:

Something along these lines is what I had in mind for an initial implementation, but I haven't thought through the details of whether it makes sense to clear it vs. extend the data, etc.

Is there something I'm not noticing in the codecomplete results that would help me get the full documentation?

No, that's correct in the current API. We should separate out two possible goals here:

• Improve performance by being more lazy about returning all the information for a code-completion item.
• Return the full documentation instead of the brief documentation.

I'm not sure one way or the other whether that second item is a good idea -- it's certainly reasonable to ask for the full documentation of a completion item, but doing so _by default using the existing API _ is more a question of how it corresponds to what LSP editors are expecting.

If we do want to return the full documentation, I think a better foundation for that is looking up information from a code-completion item in the current session, and potentially adding more lazy APIs to support that in sourcekitd, rather than via cursor info with a USR.

CC @rintaro

Cursor info on USRs only works on a subset of USRs for nominal types IIRC. Please don't use this functionality. We should never have added it.

Was the implementation never finished, or is something causing cursor info via USRs on things other than nominal types to not work?

It was never properly implemented. I think there were some partial attempts to extend it, but they were unmaintainable and later removed.