Quirks surrounding inlay hints

[tinder-tannerbennett]

Inlay hints provided by this extension pose a couple of issues.

1. First, hints for tuple payload types look strange; the types are surrounded with parenthesis, like this:

   

2. Second, you can double click to insert these hints, and they are not all valid syntax.

Regarding the first one, I suppose SourceKit-LSP is giving us back bad data. I can open an issue on that repo if someone here can confirm that is a change Apple needs to make.

As for the second issue, I believe it may be possible to disable interacting with certain hints (via the vscode headers), or at the very least, disable inserting any of the hints at all. Best case scenario, we can make it so that double clicking on "partially valid" hints—such as type labels for closure arguments—inserts the required parenthesis surrounding the parameters, as well as the selected type, and disable interacting with tuple payload hints entirely.

I am happy to help with this if this is something you want to move forward with!

[adam-fowler]

Both of these would be resolved from changes to sourcekit-lsp. The textEdits field is returned from sourcekit-lsp as part of the InlayHint. If you set the sourcekit-lsp:trace.server option to verbose you can view all the communication with the sourcekit-lsp server via the sourcekit-lsp output pane.

I guess in theory a language client middleware could be written to catch inlay hints and remove the textEdits field but there would be no easy way to recognise which inlay hints should have their textEdits removed. So it would most likely remove all or none.

[adam-fowler]

@fwcd given you wrote the SourceKit-LSP InlayHint implementation do you have any ideas how we may go about recognising InlayHints that shouldn't provide test edits?

[NSExceptional]

I guess in theory a language client middleware could be written to catch inlay hints and remove the textEdits field but there would be no easy way to recognise which inlay hints should have their textEdits removed. So it would most likely remove all or none.

Sounds like the change needs to happen entirely on the SourceKit side then! Although, I didn't even see us passing any text edits to VS Code. Did I miss it? The default text edit is to insert the text. Is the LSP giving us any edits back at all right now?

(This is my main; I am @tinder-tannerbennett)

[fwcd]

Yeah the tuple quirk is something I've noticed too, that needs to be fixed in SourceKit, possibly even somewhere deeper in the compiler. Hovers have the same issue IIRC.

As for the invalid syntax, adding parentheses on unparenthesized closure parameters as edits sounds absolutely reasonable. This is probably primarily something for SourceKit-LSP, but might require updates on the SourceKit-side too, given that the CollectVariableType request doesn't expose any higher-level semantic information about the kind of variable (if I remember correctly, it's been a while).

If nothing else, disabling the 'click-to-accept' would be a good idea. I am not quite sure how this is handled across LSP clients, e.g. whether VSCode interprets an empty array of edits as 'cannot accept' or whether the annotation itself has to be provided as an insert edit. In any case, that's most likely not something for the VSCode Swift extension to handle, given that inlay hints are LSP-native.

[adam-fowler]

It can be fixed in the VSCode extension via a middleware. I can probably catch the InlayHint response and clear the textEdits field. But in reality it is a SourceKit-LSP issue as it shouldn't be returning invalid text edits.

[fwcd]

Yes, we should tackle the root cause on the server instead of patching around the issue on the client-side (which would be less accurate too, given that those hints don't have a lot of semantic info attached to them).

[NSExceptional]

I can probably catch the InlayHint response and clear the textEdits field. But in reality it is a SourceKit-LSP issue as it shouldn't be returning invalid text edits.

I don't think you're forwarding text edits at all, at least that I could find. The default text edits just inserts the hint I believe, so if we aren't passing any text edits in, that would explain this behavior

[adam-fowler]

I don't think you're forwarding text edits at all, at least that I could find. The default text edits just inserts the hint I believe, so if we aren't passing any text edits in, that would explain this behavior

Here is an example response I just took from the SourceKit-LSP output pane. You'll see there is a textEdit section there

[
    {
        "label": ": EventLoop",
        "kind": 1,
        "textEdits": [
            {
                "range": {
                    "start": {
                        "line": 44,
                        "character": 21
                    },
                    "end": {
                        "line": 44,
                        "character": 21
                    }
                },
                "newText": ": EventLoop"
            }
        ],
        "position": {
            "line": 44,
            "character": 21
        }
    },

[NSExceptional]

Yes, but… are we returning that back to VS Code? This is the code I could find, and we're not giving back the text edits here:

https://github.com/swift-server/vscode-swift/blob/67c766d236ea138a2316334b6fb2be8969f9efaa/src/sourcekit-lsp/inlayHints.ts#L55-L60

Or is it happening somewhere else?

[adam-fowler]

@NSExceptional that is the legacy code needed for InlayHints to work on Swift 5.6 and earlier. We updated Sourcekit-LSP in 5.7 to provide the official LSP spec when VSCode add support for it.

[fwcd]

Yup, the text edit is added here:

https://github.com/apple/sourcekit-lsp/blob/d66705c8a5b5a7974f515a1df668ea4ed61385fe/Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift#L1328

[NSExceptional]

@fwcd That's SourceKit code though, where are we passing that to VS Code?

[fwcd]

That happens in the vscode-languageclient lib (since we are using the standard LSP inlay hints, no further processing is needed on the client side)

[adam-fowler]

It one of the wonders of LSP 😃 . As long as SourceKit-LSP, advertises it supports a feature and provides the correct responses the vscode-swift extension doesn't need to do anything for feature to work.

[NSExceptional]

Oh, gotcha 🥸 Thanks!