Export client from activation as API for use in other extensions

[thegecko]

This PR exports the language client so other VS Code extensions can take advantage of the clangd LSP features.

For example, domain specific functionality (e.g. intrinsics) could be implemented in a separate extension and trivially leverage the full-featured AST functionality exposed by clangd.

This has been tested and working with a trivial hover example:

import * as vscode from 'vscode';
import { BaseLanguageClient, Range, TextDocumentIdentifier, RequestType } from 'vscode-languageclient';
import { ClangdExtension } from ''

const CLANGD_EXTENSION = 'llvm-vs-code-extensions.vscode-clangd';
const CLANGD_API_VERSION = 1;

interface ClangdApiV1 {
  languageClient: BaseLanguageClient
}

interface ClangdExtension {
  getApi(version: 1): ClangdApiV1;
}

interface ASTParams {
    textDocument: TextDocumentIdentifier;
    range: Range;
}

interface ASTNode {
    role: string;    // e.g. expression
    kind: string;    // e.g. BinaryOperator
    detail?: string; // e.g. ||
    arcana?: string; // e.g. BinaryOperator <0x12345> <col:12, col:1> 'bool' '||'
    children?: Array<ASTNode>;
    range?: Range;
}

// This should work, but an error about kind: auto is thrown :/
// const ASTRequestType = new RequestType<ASTParams, ASTNode|null, void>('textDocument/ast');
const AST_REQUEST_TYPE = 'textDocument/ast';

export class LanguageFeatures {
    public constructor(protected context: vscode.ExtensionContext) {
        context.subscriptions.push(
            vscode.languages.registerHoverProvider(['c', 'cpp'], {
                provideHover: (document, position, token) => this.provideHover(document, position, token)
            })
        );
    }

    protected async provideHover(document: vscode.TextDocument, position: vscode.Position, _token: vscode.CancellationToken): Promise<vscode.Hover | undefined> {
        try {
            const languageClient = await this.getLanguageClient();
            if (!languageClient) {
                return undefined;
            }

            const textDocument = languageClient.code2ProtocolConverter.asTextDocumentIdentifier(document);
            const range = languageClient.code2ProtocolConverter.asRange(new vscode.Range(position, position));
            const params: ASTParams = { textDocument, range };
            const ast: ASTNode | undefined = await languageClient.sendRequest(AST_REQUEST_TYPE, params);

            if (!ast) {
                return undefined;
            }

            return {
                contents: [ast.kind]
            };
        } catch {
            // Ignore any errors
        }
    }

    protected async getLanguageClient(): Promise<BaseLanguageClient | undefined> {
        const extension = vscode.extensions.getExtension(CLANGD_EXTENSION);
        if (!extension) {
            return undefined;
        }

        const activated = await extension.activate() as ClangdExtension;
        const api = activated.getApi(CLANGD_API_VERSION);
        return api.languageClient;
    }
}

[HighCommander4]

Added some reviewers for feedback.

This has been proposed before in https://github.com/clangd/vscode-clangd/issues/501 with some work in progress towards it, but the approach discussed there was to expose the AST request specifically, rather than exposing the entire language client.

[thegecko]

This has been proposed before in https://github.com/clangd/vscode-clangd/issues/501 with some work in progress towards it...

Thanks, I hadn't seen that

...but the approach discussed there was to expose the AST request specifically, rather than exposing the entire language client.

I see, the benefit of exposing the entire client is that all functionality is available through a known VS Code type (BaseLanguageClient) and there is less need to maintain a hand-crafted interface. It also means additions to the LSP in future should be available through the client automatically.

There are 2 potential areas of change for this PR, but I wanted to keep it simple initially.

• Exposing the client directly effectively "blocks" the extension from exposing other APIs in future. This can be changed by instead returning an object containing the client. This approach really requires types to be exposed. If we are sure there will be no more to expose in future, we could get away with just documenting this returns a language client.

• On types, a common approach is to publish a versioned API package (either on npm or GitHub) exposing the types, with the versioning future-proofing API users. I've done this before, so happy to do that here if we see this as the right approach.

If exposing the client (rather than a custom AST API), I would suggest the API includes the RequestTypes as well as the shape of the API (e.g. ASTRequestType).

[thegecko]

I've updated this PR to add a versioned API which can be extended (the languageClient no longer stomps the root export).

I would appreciate some feedback on getting this merged @HighCommander4 @sam-mccall @kadircet @hokein

[HighCommander4]

I would appreciate some feedback on getting this merged @HighCommander4 @sam-mccall @kadircet @hokein

I'm broadly supportive of making vscode-clangd make its functionality available to other vscode extensions.

I do think this is a significant enough decision for the project that I'm not comfortable making it on my own, and would like one of the other mentioned folks to weigh in as well.

[HighCommander4]

I think a link to a real-world example usage of the proposed API (perhaps in a branch of another repo) would help reason about it.

[thegecko]

I think a link to a real-world example usage of the proposed API (perhaps in a branch of another repo) would help reason about it.

Our usage of this is not OSS to link to, but essentially exposing this access means this AST can be leveraged and enhanced functionality can be offered by another vs code extension without having to load clangd twice. Simply put, if a user needs access to a C/C++ AST for adding better IDE experience, this PR offers a simple approach to doing this.

Our use cases are primarily around accessing the AST, but it is cleaner, easier and more extensible to expose the language client in case there are other needs in future.

AST use cases include:

• adding support for target-specific instructions without having to ship a specific clangd flavour
• adding support for documentation links in a custom context e.g. to point at docs for new instructions being exposed
• I assume the request in #501 was a specific need to

[HighCommander4]

I think a link to a real-world example usage of the proposed API (perhaps in a branch of another repo) would help reason about it.

Our usage of this is not OSS to link to, but essentially exposing this access means this AST can be leveraged and enhanced functionality can be offered by another vs code extension without having to load clangd twice. Simply put, if a user needs access to a C/C++ AST for adding better IDE experience, this PR offers a simple approach to doing this.

My question was more geared towards understanding what the code consuming the API will look like.

I do see that there is a code example in the PR description -- is that what consuming code would actually look like? For example, it would have to provide its own definition of interface ASTNode and const ASTRequestType?

AST use cases include:

• adding support for target-specific instructions without having to ship a specific clangd flavour
• adding support for documentation links in a custom context e.g. to point at docs for new instructions being exposed

Not sure I'm following these particular examples -- by "instructions", do you mean assembly instructions? (Would that come up in the context of editing inline assembly code in a C/C++ code file then?)

(This question is mostly out of curiosity. I don't doubt that there are interesting things an extension can do with a C/C++ AST.)

[thegecko]

I do see that there is a code example in the PR description -- is that what consuming code would actually look like? For example, it would have to provide its own definition of interface ASTNode and const ASTRequestType?

In this simplistic form, yes. But it is also common practice to extract this API as typescript types and publish them on GitHub releases or npm. I'm happy to do that too and can look into it if this PR gets merged.

Not sure I'm following these particular examples -- by "instructions", do you mean assembly instructions? (Would that come up in the context of editing inline assembly code in a C/C++ code file then?)

e.g.

1 A customised clangd is created from a downstream LLVM which includes extra instructions/macros 2 Rather than create another vscode extension to expose this LSP, this extension is used (pointing at the custom clangd) 3 We want to add docs links to these additional instructions in the hover text (which is possible when the AST is exposed in this way)

Another interesting use case would be to expose AST data for consumption by AI engines for code helper inference etc.

This PR has been open for >3 Months, is there any further changes required in order to increase interest in merging it @sam-mccall @kadircet @hokein

[HighCommander4]

I do see that there is a code example in the PR description -- is that what consuming code would actually look like? For example, it would have to provide its own definition of interface ASTNode and const ASTRequestType?

In this simplistic form, yes. But it is also common practice to extract this API as typescript types and publish them on GitHub releases or npm. I'm happy to do that too and can look into it if this PR gets merged.

Is it possible to have those type definitions live in this repo (I think that doesn't preclude them being a distinct npm package), and have them added as part of this PR?

1 A customised clangd is created from a downstream LLVM which includes extra instructions/macros 2 Rather than create another vscode extension to expose this LSP, this extension is used (pointing at the custom clangd) 3 We want to add docs links to these additional instructions in the hover text (which is possible when the AST is exposed in this way)

In a situation like this, where you're working with a downstream LLVM (which includes clangd in-tree), why not make these hover enhancements on the server side in your downstream clangd?

[thegecko]

Is it possible to have those type definitions live in this repo (I think that doesn't preclude them being a distinct npm package), and have them added as part of this PR?

I've extracted these into a package which can be published to npm/GitHub

In a situation like this, where you're working with a downstream LLVM (which includes clangd in-tree), why not make these hover enhancements on the server side in your downstream clangd?

Because:

• We don't manage the downstream repo nor have access to it
• My team are primarily typescript/extension engineers and management is easier in the extension
• Additions in clangd are somewhat restricted to the language server protocol (TBF docs links is covered but other use cases may not be). This can be extended with custom calls, but without this extension mechanism, you won't be able to access the LSP extensions of clangd from vscode anyway

[thegecko]

Can I have an update on this, please?

[HighCommander4]

In the interest of trying to move this forward, I'm going to add myself as a reviewer, and if we don't hear a strong objection from @sam-mccall, @kadircet, or @hokein, I will take the liberty of approving this.

I want to play around with the patch a bit, including writing a (toy) example plugin that consumes the API to get a feel for what using the API looks like. I have a few other patches in my review queue ahead of this, so it may take me a couple of weeks to get around to that. Your patience is appreciated.

[thegecko]

In the interest of trying to move this forward, I'm going to add myself as a reviewer, and if we don't hear a strong objection from @sam-mccall, @kadircet, or @hokein, I will take the liberty of approving this. I want to play around with the patch a bit, including writing a (toy) example plugin that consumes the API to get a feel for what using the API looks like. I have a few other patches in my review queue ahead of this, so it may take me a couple of weeks to get around to that. Your patience is appreciated.

Thanks!

[HighCommander4]

@thegecko I checked out this branch in order to play around with it; however, it's not building for me.

After running npm install and then npm run package, I get:

$ npm run package

> [email protected] package
> vsce package --baseImagesUrl https://raw.githubusercontent.com/clangd/vscode-clangd/master/

Executing prepublish script 'npm run vscode:prepublish'...

> [email protected] vscode:prepublish
> npm run check-ts && npm run esbuild -- --minify --keep-names


> [email protected] check-ts
> tsc -noEmit -p ./


> [email protected] esbuild
> esbuild ./src/extension.ts --bundle --outfile=out/bundle.js --external:vscode --format=cjs --platform=node --minify --keep-names

✘ [ERROR] Could not resolve "../api/vscode-clangd"

    src/ast.ts:7:42:
      7 │ import {ASTParams, ASTNode, ASTType} from '../api/vscode-clangd';
        ╵                                           ~~~~~~~~~~~~~~~~~~~~~~

1 error
 ERROR  npm failed with exit code 1

Apologies if I'm missing something obvious, I'm mainly a C++ dev and I get lost easily in Typescript land.

[thegecko]

Thanks @HighCommander4

I added a const to the types at the last minute which isn't supported!

That would have failed straight away if the CI had run, but is now fixed.

I can look at adding the extension instantiation (along with the AST Type const) into the api package, but would prefer that to be a follow up PR.

[thegecko]

Thanks for the feedback @HighCommander4 , comments above. Also ran the format command which should (hopefully) fix the CI.

[thegecko]

Can you get the clang-format CI check to pass please? Then this should be good to merge.

Took me a while to realise the error output is a diff and it wasn't happy with line length. Should now be fixed.

[HighCommander4]

Merged. Thanks for your work on this!

If anyone writes publically available plugins that make use of this API, please feel free to mention them in a comment for visibility. At some point we could list such plugins (if they are of general interest) on vscode-clangd's Marketplace page.

[sr-tream]

I’m very glad that this PR was accepted - now, instead of supporting my own fork, I just moved everything I needed into separate extensions:

• CMake integration
• Open documentation on Shift+F1
• CodeLens support for D91930

[HighCommander4]

I’m very glad that this PR was accepted - now, instead of supporting my own fork, I just moved everything I needed into separate extensions:

• CMake integration

• Open documentation on Shift+F1

• CodeLens support for D91930

Very neat!

• CodeLens support for D91930

This one still requires a forked server, right?

[sr-tream]

This one still requires a forked server, right?

right

[HighCommander4]

Sent out https://github.com/clangd/vscode-clangd/pull/653 so the API appears in a vscode-clangd release