Add LSP extension request for retrieving macro expansions

[fwcd]

This implements #755 by introducing a new, non-standard request for fetching macro expansions, wrapping the corresponding SourceKitD request, SyntacticMacroExpansion.

Motivation

• There is precedent in rust-analyzer for such a request, see this document
• Using a dedicated request for this reduces the impedance mismatch between SourceKitD and the LSP layer
• Not imposing any particular presentation (as e.g. including it in hovers would) gives LSP clients freedom to present the expansion in a custom way.
  • In VSCode, for example, I would imagine that a read-only inline code editor, akin to the one shown when browsing references/definitions, might look nice:
    

Similar to what we did with inlay hints last year (#465), my hope is that a macro expansion request could eventually be upstreamed to LSP, given that such a request might be more generally useful to other languages too, as evidenced by the rust-analyzer implementation. This would then give clients full flexibility to implement a tailored UI too.

Feel free to leave some thoughts on whether this is the right approach to go with or alternative suggestions.

To do

• Add an LSP request (sourcekit-lsp/macroExpansion) and support structures
  • Specifics are up for discussion, currently the request only takes a single position and returns a single expansion, should we support batch expansions, like SourceKitD does?
• Add the SourceKit request SyntacticMacroExpansion
• Add boilerplate to ToolchainLanguageServer and its implementations to support the request
• Implement the actual request handler (SwiftLanguageServer.macroExpansion)
• Support nested expansions (perhaps customizable via a boolean flag in the request, e.g. nested: Bool or recursive: Bool?)
• Add unit tests (might require some more machinery to generate the macro plugin, similar to the upstream tests in SourceKit

See also

• SourceKit/upstream:
  • The SyntacticMacroExpansion test
  • The SyntacticMacroExpansion implementation
  • [SourceKit] Add request to expand macros syntactically swift#66295
• VSCode:
  • Add client-side support for macro expansions swift-server/vscode-swift#621

[ahoppen]

Thanks for starting the work on this @fwcd 🙏🏽

A couple of high-level thoughts, I haven’t looked at the actual code yet:

1. I know it’s very non-obvious, but the best way to expand the macro is to invoke the source.refactoring.kind.expand.macro refactoring. That’s what Xcode does as well. In case you are interested, you can view the sourcekitd request that Xcode sends by running Xcode as SOURCEKIT_SERVICE_LOG=1 /Applications/Xcode.app/Contents/MacOS/Xcode, opening a macro project, waiting for the log to calm down and then expanding a macro. You can set the log level to 2 to also see the responses
2. We should have the infrastructure to support nested macro expansions. The way this works on the sourcekitd side is that the macro expansion returns a buffer name for every edit. You can then expand macros inside the expansion buffer by setting key.primary_file to the originating source file and key.sourcefile to the buffer’s name (something like @__swiftmacro…). It’s probably best to see this in SourceKit service log in Xcode.
3. Do you know what the name in the rust-analyzer response is used for? Is it something that’s displayed to the user or could we use that for the buffer name?
4. @adam-fowler Would you be able to use a request like this to show the expanded macro in VSCode?

[fwcd]

Those are already very helpful insights, thanks!

Most of the implementation in this PR is currently just standard boilerplate for adding a request, the actual logic is yet to be implemented. I will definitely have to take a closer look at how this refactoring works and how Xcode uses it, though.

Regarding 3 and 4: I've opened swift-server/vscode-swift#621 with a super basic client implementation of the request that, admittedly, is still in dire need of a good UI. That thread also contains a short discussion of how rust-analyzer uses this request.

[ahoppen]

Oh, nice. I didn’t see the VSCode editor extension PR. ❤️

[fwcd]

This actually ended up being a lot easier to implement than I thought thanks to the semantic refactoring!

[ahoppen]

Thanks, that was indeed easy.

I think the major thing to consider here, is how to support LSP functionality inside the expansion buffer itself. It doesn’t need to be part of this PR but I would like to design it in a way that allows us to add semantic functionality to the expansion buffers in the future.

[ahoppen]

Do you think it would be valuable to link to the rust-analyzer definition of the request? Just so we can check in the future that we’re still in sync with theirs.

[fwcd]

We've already diverged a bit from rust-analyzer by passing a range in the request and by returning positions along with the expansions in the response.

After researching this a bit more, I have found that upstream (LSP) seems to favor a slightly more general approach where the server acts as a content provider for a custom URI scheme, see

• Feature request: Show generated code microsoft/language-server-protocol#689
• Proposal: Allow a language server to provide a reference content for a specific uri scheme microsoft/language-server-protocol#336

Under that model it would probably make more sense to implement macro expansion as a command where the server then invokes workspace/openDocument/workspace/peekDocument (or whichever request ends up getting implemented) with a custom URI (swift-macro-expansion://...).

Until upstream has figured out those details, my initial gut feeling was to stick with a custom request here, since that affords us some flexibility with the precise request design. It shouldn't be too hard to move over to a command-based implementation eventually either.

Reading #892 (comment) however got me thinking, that would actually work really well with the model that upstream LSP favors. I think the request-based design is a bit simpler for now, but maybe we should still move our own implementation in that direction too?

Perhaps

• sourcekit-lsp/getReferenceDocument (client -> server request)
• sourcekit-lsp/peekDocument (server -> client request)

and then add a new command that peeks the macro expansion at the given position?

One complication would be that the client would have to support a custom request too, though I don't think that should be that hard to implement.

Another complication would be that SourceKit-LSP already seems to support semantic refactorings via commands. This is super nice in theory, since we wouldn't have to do anything (in principle) to support macro expansions, however (if I understand them correctly), they would actually insert the expansion currently due to being modeled as text edits.

A crazy thought: Let the client pass an additional argument to ExecuteCommandRequest that contains e.g. a config for how the refactoring is to be applied (e.g. whether the edits should actually be performed or just previewed, e.g. with the proposed sourcekit-lsp/peekDocument request). This could then be used to show macro expansions without actually applying them, but would even work for other semantic refactorings.

So many options! Perhaps you have some more thoughts on this.

[ahoppen]

I like the idea of having a URL scheme to describe the expanded macro buffers and implementing a sourcekit-lsp/getReferenceDocument request to get the generated contents to the client. The main advantage I see with it are:

• We have a URI that we can use to talk about the expanded macro buffer. Adding support for semantic functionality within the generated buffer is just a matter of implementing LSP requests for that URL scheme.
• We can use it to show the content of generated interfaces where we currently write the generated interface to disk and then open it.

I don’t quite understand where the sourcekit-lsp/peekDocument request comes in. My design of this feature would be using ExecuteCommandRequest.

1. The client sends a textDocument/codeAction request to the server
2. If we are at a macro, the server responds with a Expand Macro code action that has a command with the custom URI scheme in the arguments.
3. If the user decides to pick that action, the client sends a workspace/executeCommand for that command.
4. And now is where it get’s a little tricky: The server replies with some kind of special response (reply of workspace/executeCommand is LSPAny?) that the client understands as: OK, I need to show an inline text buffer with the text of this URI scheme. E.g. the reply is

{ 
  "action": "showInlineBuffer",
  "uri": "swift-macro-expansion:///path/to/file.swift?buffer=@swiftmacro_…&line=6&column=8"
}

5. The client then sends a sourcekit-lsp/getReferenceDocument request to the server with that URI
6. The server replies with the contents of the buffer to display

Steps 3 and 4 are a little bit roundabout and might not actually be necessary if we just short-circuit the command in the editor.

[fwcd]

Yes, your showInlineBuffer reply is effectively what I meant by sourcekit-lsp/peekDocument (only that it wouldn't be a reply, but a separate request (which would effectively let the server invoke it multiple times, if it so desires).

But until upstream has decided on a specific design, I would be fine either way.

[ahoppen]

In my mind the client should always be driving the user interaction and the LSP server should just reply to the user’s request, that’s why I think peekDocument would be a little weird.

Is peekDocument a request you came up with or is it based on some proposal? I couldn’t find anything related to it in my short search.

[ahoppen]

and simply return a swift-macro-expansion:// URI

Which request would this be a reply to (and in which field)? AFAICT CodeAction doesn’t have a URI field (unless we just shove it in the arguments: [LSPAny]?)

Edit: I am not sure if there's a way to handle custom responses for commands triggered by a code action though, since code actions are handled by the vscode-languageclient library.

Oh, that would indeed be a major limitation, unfortunately.

[fwcd]

keep our non-standard macro expansion request for now and simply return a swift-macro-expansion://

with that I meant using the current design from this PR and returning something along the lines of

struct MacroExpansion {
  let uri: DocumentURI // = DocumentURI("swift-macro-expansion://...")
  let position: Position
}

from sourcekit-lsp/macroExpansion, which the client then resolves. But, yeah that wouldn't work as a code action, unfortunately, the client would have to implement the command manually (as my vscode-swift PR currently does).

[ahoppen]

But, yeah that wouldn't work as a code action, unfortunately, the client would have to implement the command manually (as my vscode-swift PR currently does).

That’s the major downside I see with the current design. IIUC it currently offers “Expand Macro” everywhere, even when the cursor isn’t currently on a macro. I think implementing it on top of Code Actions would be the cleanest design that also yields the best UI results.

[fwcd]

So implementing it as a command where the server calls the client back seems like the only option that fulfills all requirements (works as a code action and doesn't need a custom response)?

[ahoppen]

Yes, I think that is the case. So your design was the right one all along 😄

[ahoppen]

Just an idle thought: This is different from rust-analyzer’s macro expansion, right. Do you know what they use their name property for?

All I’m thinking here is that the closer we keep the two requests, the easier it might make life for us when/if expand macro becomes part of the LSP spec.

[fwcd]

See my comments regarding staying in sync with rust-analyzer/LSP above.

[ahoppen]

I would like to include the buffer name here as well. My thinking is that we could to encode it like

swift-macro-expansion:///path/to/file.swift?buffer=@swiftmacro_...

That way the editor could open the buffer with that URI and we could start supporting semantic functionality (like nested expansion) inside the expansion buffer as well.

[fwcd]

Yes, I like the idea (I'll also refer to my longer comment here)

[NSExceptional]

Hi, before I open a new issue, does this address use of unknown directive '#Preview' errors? (#Preview being the built in macaro)

[ahoppen]

Hi, before I open a new issue, does this address use of unknown directive '#Preview' errors? (#Preview being the built in macaro)

No, that’s completely unrelated. It probably means that some compiler arguments are missing.

[NSExceptional]

No, that’s completely unrelated. It probably means that some compiler arguments are missing.

In this case, there is no Package.swift, it's just me opening a folder with Swift files in it. We use Bazel.

Should the default arguments be adjusted?

[ahoppen]

Let’s discuss this in an issue to not spam this PR because it’s unrelated to this PR.