AbstractSemanticTokensProvider.getDocumentTokensBuilder

[cdietrich]

Hi, is there a reason AbstractSemanticTokensProvider.getDocumentTokensBuilder
reuses
SemanticTokensBuilder
i get this warning on the client side on the 2nd call Overlapping semantic tokens detected at lineNumber 1, column 10

here in _data there is already the previous tokens from the previous call.
is there something missing that flushes? e.g previousResult call?

pot. workaround:

    protected override getDocumentTokensBuilder(document: LangiumDocument): SemanticTokensBuilder {
        const existing = this.tokensBuilders.get(document.uri.toString());
        if (existing) {
            existing.previousResult(existing.id)
            return existing;
        }
        const builder = new SemanticTokensBuilder();
        this.tokensBuilders.set(document.uri.toString(), builder);
        return builder;
    }
    ```

[msujew]

We do compute a delta based on the previous result for each open file. See here.

I think using existing.previousResult(existing.id) for every request flushes the whole cache, which makes delta updates kind of useless. But we should probably use it for every request that isn't a delta request.

[cdietrich]

=> should semanticHighlight and semanticHighlightRange flush?

[msujew]

Yes, that's what I meant. They should probably flush but currently don't do that.

[msujew]

Can you check whether these overrides also solve your issue? So far, I'm having trouble reproducing this.

    async semanticHighlight(document: LangiumDocument, _params: SemanticTokensParams, cancelToken = CancellationToken.None): Promise<SemanticTokens> {
        this.currentRange = undefined;
        this.currentDocument = document;
        this.currentTokensBuilder = this.getDocumentTokensBuilder(document);
        this.currentTokensBuilder.previousResult(this.currentTokensBuilder.id);
        await this.computeHighlighting(document, this.createAcceptor(), cancelToken);
        return this.currentTokensBuilder.build();
    }

    async semanticHighlightRange(document: LangiumDocument, params: SemanticTokensRangeParams, cancelToken = CancellationToken.None): Promise<SemanticTokens> {
        this.currentRange = params.range;
        this.currentDocument = document;
        this.currentTokensBuilder = this.getDocumentTokensBuilder(document);
        this.currentTokensBuilder.previousResult(this.currentTokensBuilder.id);
        await this.computeHighlighting(document, this.createAcceptor(), cancelToken);
        return this.currentTokensBuilder.build();
    }

[cdietrich]

yes it does

[msujew]

Alright, that's good. Then I'll create a PR for that. It definitely sounds reasonable to call previousResult with the existing ID (thereby flushing the cache), in case we don't have a delta update. The code for the SemanticTokenBuilder on Microsoft's side is pretty undocumented, so it wasn't quite clear on how to use it...