Add lsp stub services - #125
Conversation
1b9c746 to
154c364
Compare
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 68da275 | Previous: d803417 | Ratio |
|---|---|---|---|
BenchmarkWorkspaceCycle (typefox.dev/fastbelt/examples/statemachine) - MB/s |
12.98 MB/s |
5.21 MB/s |
2.49 |
This comment was automatically generated by workflow using github-action-benchmark.
154c364 to
a9cbd1c
Compare
a9cbd1c to
8be8f2b
Compare
a747f34 to
68da275
Compare
| // DefaultImplementationProvider is the default implementation of [ImplementationProvider]. | ||
| // | ||
| // The default implementation finds all references to the target symbol and uses an | ||
| // [ImplementationFilter] to determine which references represent actual implementations. | ||
| type DefaultImplementationProvider struct { | ||
| sc *service.Container | ||
| filter ImplementationFilter | ||
| } |
There was a problem hiding this comment.
Discussion: I'm unsure how much benefit such a default/abstract implementation has, since it requires a very specific language design for this to work as expected.
There was a problem hiding this comment.
I agree. Let's fall back to interface only.
| // grammarInlayHintProvider provides custom inlay hints for grammar language nodes. | ||
| // Provider-only pattern: adopter implements the full provider interface, | ||
| // using the shared server.NodesInRange helper for range-filtered iteration. | ||
| type grammarInlayHintProvider struct { | ||
| sc *service.Container | ||
| } |
There was a problem hiding this comment.
Question: Is this a real implementation or just for testing purposes?
There was a problem hiding this comment.
It is a working implementation currently only used in testing. The "provider-only" comment was misleading though. I changed that already.
| // grammarSignatureHelpProvider provides custom signature help for grammar | ||
| // language nodes. Provider-only pattern: adopter implements the full | ||
| // provider interface, using the shared server.NodeAtCursor helper for | ||
| // cursor-to-node resolution. | ||
| type grammarSignatureHelpProvider struct { | ||
| sc *service.Container | ||
| } |
There was a problem hiding this comment.
Question: Real or only for testing?
There was a problem hiding this comment.
Same as before, working implementation currently only used for testing.
| // if doc == nil { | ||
| // return nil, nil | ||
| // } | ||
| // node := server.NodeAtCursor(doc, params.Position) |
There was a problem hiding this comment.
Suggestion: I believe this example is misleading. If you call server.NodeAtCursor, you run into two issues:
- If the token at the cursor is a reference, you would expect that it shows the type hierarchy for the referenced type. However,
NodeAtCursorwill return the AST node that contains the reference. - If your cursor is over a random keyword of that type, like
t<cursor>ype SomeType, it shows the type hierarchy for that type. Normally, it should only show it, if the user hovers over the name of the type (or a reference to the name).
| type CodeLensProvider interface { | ||
| HandleCodeLensRequest(ctx context.Context, params *lsp.CodeLensParams) ([]lsp.CodeLens, error) | ||
| } |
There was a problem hiding this comment.
Suggestion: Can we also add a ResolvingCodeLensProvider that supports the codeLens/resolve request?
| type CodeActionProvider interface { | ||
| HandleCodeActionRequest(ctx context.Context, params *lsp.CodeActionParams) ([]lsp.CodeAction, error) | ||
| } |
There was a problem hiding this comment.
Suggestion: Here as well for the codeAction/resolve request.
There was a problem hiding this comment.
Should we add this also for InlayHint and DocumentLinks?
|
Thanks for the review @msujew. Included all suggestions.
|
Closes #28
Mostly ports from langium with go-idiomatic mechanisms for customization.
I refactored the original proposal. With the exceptions of Declaration, Implementation and Type Defintion, which provide meaningful defaults, all lsp stubs only provide the plain interface.
Semantic tokens is another exception, which I kept for comparison, but I guess it will be overwritten by the more sophisticated version in #136.
Added examples for Call Hierarchy, Code lens, Semantic tokens (which will be replaced), and Inlay Hints to the statemachine example to show adopters a working example.
ImplementationFilterremoved - the "type implements" heuristic requires language-specific type-system knowledge no generic filter can provideResolvingInlayHintProvidertoo?SignatureHelpProviderandSignatureHelpTriggersNodeAtCursor- now usesNameFinder. Fixed in both the godoc example and the statemachine exampleNodeAtCursorResolvingCodeActionProviderforcodeAction/resolvesupportResolvingCodeLensProviderforcodeLens/resolvesupport.ResolvingDocumentLinkProvidertoo?