From 61e776b97a767868635ebb9fb24de58fc14e4bdf Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 5 Apr 2024 21:22:18 +0200 Subject: [PATCH] feat: improved completion provider (#997) Closes #41 ### Summary of Changes Various improvements to the completion provider: * Proper icons * Documentation * Mark as deprecated * Fix suggestions for member accesses * Fix suggestions for member types * Filter keywords --- .../src/language/helpers/fileExtensions.ts | 6 +- .../lsp/safe-ds-completion-provider.ts | 115 +++++ .../lsp/safe-ds-node-info-provider.ts | 4 +- .../lsp/safe-ds-node-kind-provider.ts | 57 ++- .../src/language/safe-ds-module.ts | 2 + .../lsp/safe-ds.completion-provider.test.ts | 410 ++++++++++++++++++ packages/safe-ds-vscode/package.json | 2 + packages/safe-ds-vscode/snippets/safe-ds.json | 7 +- 8 files changed, 586 insertions(+), 17 deletions(-) create mode 100644 packages/safe-ds-lang/src/language/lsp/safe-ds-completion-provider.ts create mode 100644 packages/safe-ds-lang/tests/language/lsp/safe-ds.completion-provider.test.ts diff --git a/packages/safe-ds-lang/src/language/helpers/fileExtensions.ts b/packages/safe-ds-lang/src/language/helpers/fileExtensions.ts index d485bc6fe..c26d362ec 100644 --- a/packages/safe-ds-lang/src/language/helpers/fileExtensions.ts +++ b/packages/safe-ds-lang/src/language/helpers/fileExtensions.ts @@ -38,17 +38,17 @@ export type SdSFileExtension = typeof PIPELINE_FILE_EXTENSION | typeof STUB_FILE /** * Returns whether the object is contained in a pipeline file. */ -export const isInPipelineFile = (node: AstNode) => isPipelineFile(AstUtils.getDocument(node)); +export const isInPipelineFile = (node: AstNode | undefined) => node && isPipelineFile(AstUtils.getDocument(node)); /** * Returns whether the object is contained in a stub file. */ -export const isInStubFile = (node: AstNode) => isStubFile(AstUtils.getDocument(node)); +export const isInStubFile = (node: AstNode | undefined) => node && isStubFile(AstUtils.getDocument(node)); /** * Returns whether the object is contained in a test file. */ -export const isInTestFile = (node: AstNode) => isTestFile(AstUtils.getDocument(node)); +export const isInTestFile = (node: AstNode | undefined) => node && isTestFile(AstUtils.getDocument(node)); /** * Returns whether the resource represents a pipeline file. diff --git a/packages/safe-ds-lang/src/language/lsp/safe-ds-completion-provider.ts b/packages/safe-ds-lang/src/language/lsp/safe-ds-completion-provider.ts new file mode 100644 index 000000000..7e0cd2d82 --- /dev/null +++ b/packages/safe-ds-lang/src/language/lsp/safe-ds-completion-provider.ts @@ -0,0 +1,115 @@ +import { CompletionContext, CompletionValueItem, DefaultCompletionProvider } from 'langium/lsp'; +import { AstNode, AstNodeDescription, ReferenceInfo, Stream } from 'langium'; +import { SafeDsServices } from '../safe-ds-module.js'; +import { CompletionItemTag, MarkupContent } from 'vscode-languageserver'; +import { createMarkupContent } from '../documentation/safe-ds-comment-provider.js'; +import { SafeDsDocumentationProvider } from '../documentation/safe-ds-documentation-provider.js'; +import type { SafeDsAnnotations } from '../builtins/safe-ds-annotations.js'; +import { isSdsAnnotatedObject, isSdsModule, isSdsNamedType, isSdsReference } from '../generated/ast.js'; +import { getPackageName } from '../helpers/nodeProperties.js'; +import { isInPipelineFile, isInStubFile } from '../helpers/fileExtensions.js'; + +export class SafeDsCompletionProvider extends DefaultCompletionProvider { + private readonly builtinAnnotations: SafeDsAnnotations; + private readonly documentationProvider: SafeDsDocumentationProvider; + + readonly completionOptions = { + triggerCharacters: ['.', '@'], + }; + + constructor(service: SafeDsServices) { + super(service); + + this.builtinAnnotations = service.builtins.Annotations; + this.documentationProvider = service.documentation.DocumentationProvider; + } + + protected override getReferenceCandidates( + refInfo: ReferenceInfo, + context: CompletionContext, + ): Stream { + this.fixReferenceInfo(refInfo); + return super.getReferenceCandidates(refInfo, context); + } + + private fixReferenceInfo(refInfo: ReferenceInfo): void { + if (isSdsNamedType(refInfo.container) && refInfo.container.$containerProperty === 'declaration') { + const syntheticNode = refInfo.container.$container as AstNode; + if (isSdsNamedType(syntheticNode) && syntheticNode.$containerProperty === 'member') { + refInfo.container = { + ...refInfo.container, + $container: syntheticNode.$container, + $containerProperty: 'member', + }; + } else { + refInfo.container = { + ...refInfo.container, + $containerProperty: 'member', + }; + } + } else if (isSdsReference(refInfo.container) && refInfo.container.$containerProperty === 'member') { + const syntheticNode = refInfo.container.$container as AstNode; + if (isSdsReference(syntheticNode) && syntheticNode.$containerProperty === 'member') { + refInfo.container = { + ...refInfo.container, + $container: syntheticNode.$container, + $containerProperty: 'member', + }; + } + } + } + + protected override createReferenceCompletionItem(nodeDescription: AstNodeDescription): CompletionValueItem { + const node = nodeDescription.node; + + return { + nodeDescription, + documentation: this.getDocumentation(node), + kind: this.nodeKindProvider.getCompletionItemKind(nodeDescription), + tags: this.getTags(node), + sortText: '0', + }; + } + + private getDocumentation(node: AstNode | undefined): MarkupContent | undefined { + if (!node) { + /* c8 ignore next 2 */ + return undefined; + } + + const documentation = this.documentationProvider.getDescription(node); + return createMarkupContent(documentation); + } + + private getTags(node: AstNode | undefined): CompletionItemTag[] | undefined { + if (isSdsAnnotatedObject(node) && this.builtinAnnotations.callsDeprecated(node)) { + return [CompletionItemTag.Deprecated]; + } else { + return undefined; + } + } + + private illegalKeywordsInPipelineFile = new Set(['annotation', 'class', 'enum', 'fun', 'schema']); + private illegalKeywordsInStubFile = new Set(['pipeline', 'internal', 'private', 'segment']); + + protected override filterKeyword(context: CompletionContext, keyword: Keyword): boolean { + // Filter out keywords that do not contain any word character + if (!/\p{L}/u.test(keyword.value)) { + return false; + } + + if ((!context.node || isSdsModule(context.node)) && !getPackageName(context.node)) { + return keyword.value === 'package'; + } else if (isSdsModule(context.node) && isInPipelineFile(context.node)) { + return !this.illegalKeywordsInPipelineFile.has(keyword.value); + } else if (isSdsModule(context.node) && isInStubFile(context.node)) { + return !this.illegalKeywordsInStubFile.has(keyword.value); + } else { + return true; + } + } +} + +export interface Keyword { + value: string; +} diff --git a/packages/safe-ds-lang/src/language/lsp/safe-ds-node-info-provider.ts b/packages/safe-ds-lang/src/language/lsp/safe-ds-node-info-provider.ts index 33227f646..0c2fc8d18 100644 --- a/packages/safe-ds-lang/src/language/lsp/safe-ds-node-info-provider.ts +++ b/packages/safe-ds-lang/src/language/lsp/safe-ds-node-info-provider.ts @@ -18,7 +18,7 @@ export class SafeDsNodeInfoProvider { * Returns the detail string for the given node. This can be used, for example, to provide document symbols or call * hierarchies. */ - getDetails(node: AstNode): string | undefined { + getDetails(node: AstNode | undefined): string | undefined { if (isSdsAttribute(node)) { return `: ${this.typeComputer.computeType(node)}`; } else if (isSdsFunction(node) || isSdsSegment(node)) { @@ -32,7 +32,7 @@ export class SafeDsNodeInfoProvider { * Returns the tags for the given node. This can be used, for example, to provide document symbols or call * hierarchies. */ - getTags(node: AstNode): SymbolTag[] | undefined { + getTags(node: AstNode | undefined): SymbolTag[] | undefined { if (isSdsAnnotatedObject(node) && this.builtinAnnotations.callsDeprecated(node)) { return [SymbolTag.Deprecated]; } else { diff --git a/packages/safe-ds-lang/src/language/lsp/safe-ds-node-kind-provider.ts b/packages/safe-ds-lang/src/language/lsp/safe-ds-node-kind-provider.ts index 70e7a02e2..05ef99b55 100644 --- a/packages/safe-ds-lang/src/language/lsp/safe-ds-node-kind-provider.ts +++ b/packages/safe-ds-lang/src/language/lsp/safe-ds-node-kind-provider.ts @@ -29,13 +29,13 @@ export class SafeDsNodeKindProvider implements NodeKindProvider { return SymbolKind.Method; } + /* c8 ignore start */ const type = this.getNodeType(nodeOrDescription); switch (type) { case SdsAnnotation: return SymbolKind.Interface; case SdsAttribute: return SymbolKind.Property; - /* c8 ignore next 2 */ case SdsBlockLambdaResult: return SymbolKind.Variable; case SdsClass: @@ -48,36 +48,71 @@ export class SafeDsNodeKindProvider implements NodeKindProvider { return SymbolKind.Function; case SdsModule: return SymbolKind.Package; - /* c8 ignore next 2 */ case SdsParameter: return SymbolKind.Variable; case SdsPipeline: return SymbolKind.Function; - /* c8 ignore next 2 */ case SdsPlaceholder: return SymbolKind.Variable; - /* c8 ignore next 2 */ case SdsResult: return SymbolKind.Variable; case SdsSchema: return SymbolKind.Struct; case SdsSegment: return SymbolKind.Function; - /* c8 ignore next 2 */ case SdsTypeParameter: return SymbolKind.TypeParameter; - /* c8 ignore next 2 */ default: return SymbolKind.Null; } + /* c8 ignore stop */ } - /* c8 ignore start */ - getCompletionItemKind(_nodeOrDescription: AstNode | AstNodeDescription) { - return CompletionItemKind.Reference; - } + getCompletionItemKind(nodeOrDescription: AstNode | AstNodeDescription): CompletionItemKind { + // The WorkspaceSymbolProvider only passes descriptions, where the node might be undefined + const node = this.getNode(nodeOrDescription); + if (isSdsFunction(node) && AstUtils.hasContainerOfType(node, isSdsClass)) { + return CompletionItemKind.Method; + } - /* c8 ignore stop */ + /* c8 ignore start */ + const type = this.getNodeType(nodeOrDescription); + switch (type) { + case SdsAnnotation: + return CompletionItemKind.Interface; + case SdsAttribute: + return CompletionItemKind.Property; + case SdsBlockLambdaResult: + return CompletionItemKind.Variable; + case SdsClass: + return CompletionItemKind.Class; + case SdsEnum: + return CompletionItemKind.Enum; + case SdsEnumVariant: + return CompletionItemKind.EnumMember; + case SdsFunction: + return CompletionItemKind.Function; + case SdsModule: + return CompletionItemKind.Module; + case SdsParameter: + return CompletionItemKind.Variable; + case SdsPipeline: + return CompletionItemKind.Function; + case SdsPlaceholder: + return CompletionItemKind.Variable; + case SdsResult: + return CompletionItemKind.Variable; + case SdsSchema: + return CompletionItemKind.Struct; + case SdsSegment: + return CompletionItemKind.Function; + case SdsTypeParameter: + return CompletionItemKind.TypeParameter; + default: + return CompletionItemKind.Reference; + } + /* c8 ignore stop */ + } private getNode(nodeOrDescription: AstNode | AstNodeDescription): AstNode | undefined { if (isAstNode(nodeOrDescription)) { diff --git a/packages/safe-ds-lang/src/language/safe-ds-module.ts b/packages/safe-ds-lang/src/language/safe-ds-module.ts index 58a7b4ecc..d003c88db 100644 --- a/packages/safe-ds-lang/src/language/safe-ds-module.ts +++ b/packages/safe-ds-lang/src/language/safe-ds-module.ts @@ -43,6 +43,7 @@ import { SafeDsRenameProvider } from './lsp/safe-ds-rename-provider.js'; import { SafeDsRunner } from './runner/safe-ds-runner.js'; import { SafeDsTypeFactory } from './typing/safe-ds-type-factory.js'; import { SafeDsMarkdownGenerator } from './generation/safe-ds-markdown-generator.js'; +import { SafeDsCompletionProvider } from './lsp/safe-ds-completion-provider.js'; /** * Declaration of custom services - add your own service classes here. @@ -129,6 +130,7 @@ export const SafeDsModule: Module new SafeDsCallHierarchyProvider(services), + CompletionProvider: (services) => new SafeDsCompletionProvider(services), DocumentSymbolProvider: (services) => new SafeDsDocumentSymbolProvider(services), Formatter: () => new SafeDsFormatter(), InlayHintProvider: (services) => new SafeDsInlayHintProvider(services), diff --git a/packages/safe-ds-lang/tests/language/lsp/safe-ds.completion-provider.test.ts b/packages/safe-ds-lang/tests/language/lsp/safe-ds.completion-provider.test.ts new file mode 100644 index 000000000..30e509405 --- /dev/null +++ b/packages/safe-ds-lang/tests/language/lsp/safe-ds.completion-provider.test.ts @@ -0,0 +1,410 @@ +import { createSafeDsServices } from '../../../src/language/index.js'; +import { NodeFileSystem } from 'langium/node'; +import { expectCompletion } from 'langium/test'; +import { describe, expect, it } from 'vitest'; +import { CompletionItemTag } from 'vscode-languageserver'; + +const services = (await createSafeDsServices(NodeFileSystem, { omitBuiltins: true })).SafeDs; + +describe('SafeDsCompletionProvider', async () => { + describe('suggested labels', () => { + const testCases: CompletionTest[] = [ + // Keywords + { + testName: 'empty', + code: '<|>', + expectedLabels: { + shouldEqual: ['package'], + }, + }, + { + testName: 'after package (sdspipe)', + code: ` + package myPackage + <|> + `, + uri: `file:///test1.sdspipe`, + expectedLabels: { + shouldEqual: ['from', 'pipeline', 'internal', 'private', 'segment'], + }, + }, + { + testName: 'after package (sdsstub)', + code: ` + package myPackage + <|> + `, + uri: `file:///test2.sdsstub`, + expectedLabels: { + shouldEqual: ['from', 'annotation', 'class', 'enum', 'fun', 'schema'], + }, + }, + { + testName: 'after package (sdstest)', + code: ` + package myPackage + <|> + `, + uri: `file:///test3.sdstest`, + expectedLabels: { + shouldEqual: [ + 'from', + 'annotation', + 'class', + 'enum', + 'fun', + 'schema', + 'pipeline', + 'internal', + 'private', + 'segment', + ], + }, + }, + { + testName: 'in class', + code: ` + class MyClass { + <|> + `, + expectedLabels: { + shouldEqual: ['static', 'attr', 'class', 'enum', 'fun'], + }, + }, + + // Cross-references + { + testName: 'annotation calls (no prefix)', + code: ` + annotation MyAnnotation + + @<|> + `, + expectedLabels: { + shouldEqual: ['MyAnnotation'], + }, + }, + { + testName: 'annotation calls (with prefix)', + code: ` + annotation MyAnnotation + annotation OtherAnnotation + + @O<|> + `, + expectedLabels: { + shouldEqual: ['OtherAnnotation'], + }, + }, + { + testName: 'arguments (no prefix)', + code: ` + fun f(p: unknown) + + pipeline myPipeline { + f(<|> + `, + expectedLabels: { + shouldContain: ['p'], + }, + }, + { + testName: 'arguments (with prefix)', + code: ` + fun f(p1: unknown, q2: unknown) + + pipeline myPipeline { + f(p<|> + `, + expectedLabels: { + shouldContain: ['p1'], + shouldNotContain: ['q2'], + }, + }, + { + testName: 'member accesses (no prefix)', + code: ` + class MyClass { + static attr a: unknown + } + + pipeline myPipeline { + MyClass.<|> + `, + expectedLabels: { + shouldContain: ['a'], + shouldNotContain: ['MyClass', 'myPipeline'], + }, + }, + { + testName: 'member accesses (with prefix)', + code: ` + class MyClass { + static attr a1: unknown + static fun b2() + } + + pipeline myPipeline { + MyClass.a<|> + `, + expectedLabels: { + shouldContain: ['a1'], + shouldNotContain: ['MyClass', 'myPipeline'], + }, + }, + { + testName: 'member types (no prefix)', + code: ` + class MyClass { + class NestedClass + } + + fun f(p: MyClass.<|> + `, + expectedLabels: { + shouldContain: ['NestedClass'], + shouldNotContain: ['MyClass'], + }, + }, + { + testName: 'member types (with prefix)', + code: ` + class MyClass { + class NestedClass + class OtherClass + } + + fun f(p: MyClass.N<|> + `, + expectedLabels: { + shouldContain: ['NestedClass'], + shouldNotContain: ['MyClass', 'OtherClass'], + }, + }, + { + testName: 'named types (no prefix)', + code: ` + class MyClass + + fun f(p: <|> + `, + expectedLabels: { + shouldContain: ['MyClass'], + }, + }, + { + testName: 'named types (with prefix)', + code: ` + class MyClass + class OtherClass + + fun f(p: M<|> + `, + expectedLabels: { + shouldContain: ['MyClass'], + shouldNotContain: ['OtherClass'], + }, + }, + { + testName: 'parameter bounds (no prefix)', + code: ` + fun f(p: unknown) where { + <|> + `, + expectedLabels: { + shouldEqual: ['p'], + }, + }, + { + testName: 'parameter bounds (with prefix)', + code: ` + fun f(p1: unknown, q2: unknown) where { + p<|> + `, + expectedLabels: { + shouldContain: ['p1'], + shouldNotContain: ['q2'], + }, + }, + { + testName: 'reference (no prefix)', + code: ` + fun f() + + pipeline myPipeline { + <|> + `, + expectedLabels: { + shouldContain: ['f'], + }, + }, + { + testName: 'reference (with prefix)', + code: ` + fun f1() + fun g2() + + pipeline myPipeline { + f<|> + `, + expectedLabels: { + shouldContain: ['f1'], + shouldNotContain: ['g2'], + }, + }, + { + testName: 'type arguments (no prefix)', + code: ` + class MyClass { + attr a: MyClass<<|> + `, + expectedLabels: { + shouldContain: ['T'], + }, + }, + { + testName: 'type arguments (with prefix)', + code: ` + class MyClass { + attr a: MyClass + `, + expectedLabels: { + shouldContain: ['T1'], + shouldNotContain: ['U2'], + }, + }, + { + testName: 'yields (no prefix)', + code: ` + segment mySegment() -> (r: unknown) { + yield <|> + `, + expectedLabels: { + shouldContain: ['r'], + }, + }, + { + testName: 'yields (with prefix)', + code: ` + segment mySegment() -> (r1: unknown, s2: unknown) { + yield r<|> + `, + expectedLabels: { + shouldContain: ['r1'], + shouldNotContain: ['s2'], + }, + }, + ]; + + it.each(testCases)('$testName', async ({ code, uri, expectedLabels }) => { + await expectCompletion(services)({ + text: code, + index: 0, + parseOptions: { + documentUri: uri, + }, + assert(completion) { + const labels = completion.items.map((item) => item.label); + + if (expectedLabels.shouldEqual) { + expect(labels).toStrictEqual(expectedLabels.shouldEqual); + } + + if (expectedLabels.shouldContain) { + expect(labels).to.containSubset(expectedLabels.shouldContain); + } + + if (expectedLabels.shouldNotContain) { + expect(labels).not.to.containSubset(expectedLabels.shouldNotContain); + } + }, + disposeAfterCheck: true, + }); + }); + }); + + it('should return documentation if available', async () => { + await expectCompletion(services)({ + text: ` + /** + * My documentation + */ + annotation MyAnnotation + + @<|> + `, + index: 0, + assert(completion) { + expect(completion.items).toHaveLength(1); + expect(completion.items[0]!.documentation).toStrictEqual({ + kind: 'markdown', + value: 'My documentation', + }); + }, + }); + }); + + it('should add deprecated tag if needed', async () => { + const servicesWithBuiltins = (await createSafeDsServices(NodeFileSystem)).SafeDs; + await expectCompletion(servicesWithBuiltins)({ + text: ` + package test + + @Deprecated + annotation MyAnnotation + + @MyAnnotation<|> + `, + index: 0, + assert(completion) { + expect(completion.items).toHaveLength(1); + expect(completion.items[0]!.tags).toStrictEqual([CompletionItemTag.Deprecated]); + }, + }); + }); +}); + +/** + * A test case for {@link SafeDsCompletionProvider}. + */ +interface CompletionTest { + /** + * A short description of the test case. + */ + testName: string; + + /** + * The code to parse. + */ + code: string; + + /** + * The URI to assign to the parsed document. + */ + uri?: string; + + /** + * The expected completion labels. + */ + expectedLabels: ExpectedLabels; +} + +/** + * The expected completion labels. + */ +interface ExpectedLabels { + /** + * The actual labels must be equal to these labels. + */ + shouldEqual?: string[]; + + /** + * The actual labels must contain these labels. + */ + shouldContain?: string[]; + + /** + * The actual labels must not contain these labels. + */ + shouldNotContain?: string[]; +} diff --git a/packages/safe-ds-vscode/package.json b/packages/safe-ds-vscode/package.json index e7d654bb9..868afd2a6 100644 --- a/packages/safe-ds-vscode/package.json +++ b/packages/safe-ds-vscode/package.json @@ -128,6 +128,8 @@ "configurationDefaults": { "[safe-ds]": { "editor.semanticHighlighting.enabled": true, + "editor.suggest.snippetsPreventQuickSuggestions": true, + "editor.wordBasedSuggestions": "off", "editor.wordSeparators": "`~!@#%^&*()-=+[]{}\\|;:'\",.<>/?»«", "files.trimTrailingWhitespace": true } diff --git a/packages/safe-ds-vscode/snippets/safe-ds.json b/packages/safe-ds-vscode/snippets/safe-ds.json index 0f1d634b9..a71776048 100644 --- a/packages/safe-ds-vscode/snippets/safe-ds.json +++ b/packages/safe-ds-vscode/snippets/safe-ds.json @@ -69,7 +69,12 @@ }, "Segment": { "prefix": ["segment"], - "body": ["${1|internal ,private |}segment ${2:mySegment}($3) ${4:-> ($5)} {", " $0", "}"], + "body": ["${1|internal ,private |}segment ${2:mySegment}($3) ${4:-> ($5)} ${6:where {$7\\}} {", " $0", "}"], + "description": "A segment." + }, + "Minimal Segment": { + "prefix": ["minimal-segment"], + "body": ["${1|internal ,private |}segment ${2:mySegment}($3) {", " $0", "}"], "description": "A segment." }, "Block Lambda": {