Unable to extend DefaultNodeKindProvider

[snarkipus]

Langium version: 3.1.0 (91219ec) Package name: Langium

Steps To Reproduce

1. Create CustomNodeKindProvider by extending DefaultNodeKindProvider to support custom document symbols

Link to code example:

export interface NodeKindProvider {
    /**
     * Returns a `SymbolKind` as used by `WorkspaceSymbolProvider` or `DocumentSymbolProvider`.
     */
    getSymbolKind(node: AstNode | AstNodeDescription): SymbolKind;
    /**
     * Returns a `CompletionItemKind` as used by the `CompletionProvider`.
     */
    getCompletionItemKind(node: AstNode | AstNodeDescription): CompletionItemKind;
}

export class DefaultNodeKindProvider implements NodeKindProvider {
    getSymbolKind(): SymbolKind {
        return SymbolKind.Field;
    }
    getCompletionItemKind(): CompletionItemKind {
        return CompletionItemKind.Reference;
    }
}

The current behavior

Unable to extend default class due to method signatures - interesting that TypeScript allows it ... Claude told me it is something about function parameter bivariance which made my head hurt.

The expected behavior

Class should be extensible allowing for document symbol customization per grammar.

Maybe something like this will resolve it: https://github.com/eclipse-langium/langium/commit/6d1ee545c48373a79edaf08376048a832ecfa64d

Not sure how keen you guys are on disabling lints.

[msujew]

Ah, yes. I always forget that omitting method parameters makes overrides behave pretty weird in TypeScript...

A PR is appreciated, I think you can circumvent the eslint issues by just prefixing the parameter name with _.

[snarkipus]

Please see https://github.com/eclipse-langium/langium/pull/1580

[snarkipus]

Out of curiosity, how strictly do you expect/need the DefaultXYZ class implementation to adhere to the interface it implements? Looks like there 's other places where arguments are omitted (see below). CancellationToken seems to be a common one ... although I honestly have no idea how critical that would be for anyone extending the class.

export interface DocumentSymbolProvider {
    /**
     * Handle a document symbols request.
     *
     * @throws `OperationCancelled` if cancellation is detected during execution
     * @throws `ResponseError` if an error is detected that should be sent as response to the client
     */
    getSymbols(document: LangiumDocument, params: DocumentSymbolParams, cancelToken?: CancellationToken): MaybePromise<DocumentSymbol[]>;
}

export class DefaultDocumentSymbolProvider implements DocumentSymbolProvider {

    protected readonly nameProvider: NameProvider;
    protected readonly nodeKindProvider: NodeKindProvider;

    constructor(services: LangiumServices) {
        this.nameProvider = services.references.NameProvider;
        this.nodeKindProvider = services.shared.lsp.NodeKindProvider;
    }

    getSymbols(document: LangiumDocument): MaybePromise<DocumentSymbol[]> {
        return this.getSymbol(document, document.parseResult.value);
    }

[spoenemann]

The only reason the situation is like that is that we weren't aware that it's a problem. You are right, we need to review all default impls and add missing parameters, otherwise subclasses can't use them.

[spoenemann]

Example with DefaultDocumentSymbolProvider:

Property 'getSymbols' in type 'SpecialDocumentSymbolProvider' is not assignable to the same property in base type 'DefaultDocumentSymbolProvider'.
  Type '(document: LangiumDocument<AstNode>, params: DocumentSymbolParams, cancelToken?: CancellationToken | undefined) => MaybePromise<...>' is not assignable to type '(document: LangiumDocument<AstNode>) => MaybePromise<DocumentSymbol[]>'.
    Target signature provides too few arguments. Expected 2 or more, but got 1.ts(2416)

[snarkipus]

Created a separate issue (#1584) - will create a separate PR (or PRs?) for the larger fix.

[snarkipus]

Addressed with #1580