diff --git a/packages/safe-ds-lang/src/language/codeActions/factories.ts b/packages/safe-ds-lang/src/language/codeActions/factories.ts new file mode 100644 index 000000000..150e93f3d --- /dev/null +++ b/packages/safe-ds-lang/src/language/codeActions/factories.ts @@ -0,0 +1,28 @@ +import { CodeAction, Diagnostic, TextEdit } from 'vscode-languageserver'; +import { LangiumDocument } from 'langium'; + +export const createQuickfixFromTextEditsToSingleDocument = ( + title: string, + diagnostic: Diagnostic, + document: LangiumDocument, + edits: TextEdit[], + isPreferred: boolean = false, +): CodeAction => { + return { + title, + kind: 'quickfix', + diagnostics: [diagnostic], + edit: { + documentChanges: [ + { + textDocument: { + uri: document.textDocument.uri, + version: document.textDocument.version, + }, + edits, + }, + ], + }, + isPreferred, + }; +}; diff --git a/packages/safe-ds-lang/src/language/codeActions/quickfixes/arguments.ts b/packages/safe-ds-lang/src/language/codeActions/quickfixes/arguments.ts index dcfc1ac13..fd1ca1c23 100644 --- a/packages/safe-ds-lang/src/language/codeActions/quickfixes/arguments.ts +++ b/packages/safe-ds-lang/src/language/codeActions/quickfixes/arguments.ts @@ -1,10 +1,11 @@ import { Diagnostic, TextEdit } from 'vscode-languageserver'; -import { AstUtils, LangiumDocument } from 'langium'; +import { LangiumDocument } from 'langium'; import { SafeDsServices } from '../../safe-ds-module.js'; -import { CodeActionAcceptor } from '../safe-ds-code-action-provider.js'; import { isSdsArgumentList, SdsArgument } from '../../generated/ast.js'; import { Argument, Parameter } from '../../helpers/nodeProperties.js'; import { SafeDsNodeMapper } from '../../helpers/safe-ds-node-mapper.js'; +import { CodeActionAcceptor } from '../safe-ds-code-action-provider.js'; +import { createQuickfixFromTextEditsToSingleDocument } from '../factories.js'; export const makeArgumentsAssignedToOptionalParametersNamed = (services: SafeDsServices) => { const locator = services.workspace.AstNodeLocator; @@ -12,29 +13,19 @@ export const makeArgumentsAssignedToOptionalParametersNamed = (services: SafeDsS return (diagnostic: Diagnostic, document: LangiumDocument, acceptor: CodeActionAcceptor) => { const node = locator.getAstNode(document.parseResult.value, diagnostic.data.path); - if (!node) { - return; - } - - const argumentList = AstUtils.getContainerOfType(node, isSdsArgumentList); - if (!argumentList) { - /* c8 ignore next 2 */ + if (!isSdsArgumentList(node)) { return; } - const edits: TextEdit[] = argumentList.arguments.flatMap((it) => ensureArgumentIsNamed(nodeMapper, it)); - - acceptor({ - title: 'Add names to arguments that are assigned to optional parameters.', - kind: 'quickfix', - diagnostics: [diagnostic], - edit: { - changes: { - [document.textDocument.uri]: edits, - }, - }, - isPreferred: true, - }); + acceptor( + createQuickfixFromTextEditsToSingleDocument( + 'Add names to arguments that are assigned to optional parameters.', + diagnostic, + document, + node.arguments.flatMap((it) => ensureArgumentIsNamed(nodeMapper, it)), + true, + ), + ); }; }; diff --git a/packages/safe-ds-lang/src/language/codeActions/quickfixes/safe-ds-quickfix-provider.ts b/packages/safe-ds-lang/src/language/codeActions/quickfixes/safe-ds-quickfix-provider.ts index 8ffaa84ca..8ad1d4aba 100644 --- a/packages/safe-ds-lang/src/language/codeActions/quickfixes/safe-ds-quickfix-provider.ts +++ b/packages/safe-ds-lang/src/language/codeActions/quickfixes/safe-ds-quickfix-provider.ts @@ -20,6 +20,7 @@ export class SafeDsQuickfixProvider { } const quickfixes = this.registry[diagnostic.code]; + if (Array.isArray(quickfixes)) { for (const quickfix of quickfixes) { quickfix(diagnostic, document, acceptor); @@ -31,7 +32,7 @@ export class SafeDsQuickfixProvider { } type QuickfixRegistry = { - [code: string | number]: Quickfix | Quickfix[]; + [code: string | number]: QuickfixCreator | QuickfixCreator[]; }; -type Quickfix = (diagnostic: Diagnostic, document: LangiumDocument, acceptor: CodeActionAcceptor) => void; +type QuickfixCreator = (diagnostic: Diagnostic, document: LangiumDocument, acceptor: CodeActionAcceptor) => void; diff --git a/packages/safe-ds-lang/src/language/codeActions/safe-ds-code-action-provider.ts b/packages/safe-ds-lang/src/language/codeActions/safe-ds-code-action-provider.ts index fefd710c5..759d4ce13 100644 --- a/packages/safe-ds-lang/src/language/codeActions/safe-ds-code-action-provider.ts +++ b/packages/safe-ds-lang/src/language/codeActions/safe-ds-code-action-provider.ts @@ -1,8 +1,9 @@ import { CodeActionProvider } from 'langium/lsp'; import { LangiumDocument, MaybePromise } from 'langium'; -import { CancellationToken, CodeAction, CodeActionParams, Command } from 'vscode-languageserver'; +import { CancellationToken, CodeAction, CodeActionParams } from 'vscode-languageserver'; import { SafeDsServices } from '../safe-ds-module.js'; import { SafeDsQuickfixProvider } from './quickfixes/safe-ds-quickfix-provider.js'; +import { isEmpty } from '../../helpers/collections.js'; export class SafeDsCodeActionProvider implements CodeActionProvider { private readonly quickfixProvider: SafeDsQuickfixProvider; @@ -15,7 +16,7 @@ export class SafeDsCodeActionProvider implements CodeActionProvider { document: LangiumDocument, params: CodeActionParams, _cancelToken?: CancellationToken, - ): MaybePromise | undefined> { + ): MaybePromise { const result: CodeAction[] = []; const acceptor = (action: CodeAction) => result.push(action); @@ -23,7 +24,7 @@ export class SafeDsCodeActionProvider implements CodeActionProvider { this.quickfixProvider.createQuickfixes(diagnostic, document, acceptor); } - return result; + return isEmpty(result) ? undefined : result; } } diff --git a/packages/safe-ds-lang/tests/helpers/diagnostics.ts b/packages/safe-ds-lang/tests/helpers/diagnostics.ts index 23a5b932c..ca16b1b55 100644 --- a/packages/safe-ds-lang/tests/helpers/diagnostics.ts +++ b/packages/safe-ds-lang/tests/helpers/diagnostics.ts @@ -97,15 +97,15 @@ export class SyntaxErrorsInCodeError extends TestDescriptionError { } /** - * The code contains syntax errors. + * The code contains errors. */ export class ErrorsInCodeError extends TestDescriptionError { constructor( readonly errors: Diagnostic[], uri: URI, ) { - const syntaxErrorsAsString = errors.map((e) => ` - ${e.message}`).join(`\n`); + const errorsAsString = errors.map((e) => ` - ${e.message}`).join(`\n`); - super(`Code has errors:\n${syntaxErrorsAsString}`, uri); + super(`Code has errors:\n${errorsAsString}`, uri); } } diff --git a/packages/safe-ds-lang/tests/language/codeActions/creator.ts b/packages/safe-ds-lang/tests/language/codeActions/creator.ts new file mode 100644 index 000000000..58a1a9ba4 --- /dev/null +++ b/packages/safe-ds-lang/tests/language/codeActions/creator.ts @@ -0,0 +1,156 @@ +import { + listTestFilesWithExtensions, + listTestSafeDsFilesGroupedByParentDirectory, + loadDocuments, + uriToShortenedTestResourceName, +} from '../../helpers/testResources.js'; +import path from 'path'; +import { getSyntaxErrors, SyntaxErrorsInCodeError } from '../../helpers/diagnostics.js'; +import { NodeFileSystem } from 'langium/node'; +import { TestDescription, TestDescriptionError } from '../../helpers/testDescription.js'; +import { URI } from 'langium'; +import { createSafeDsServices } from '../../../src/language/index.js'; +import { findTestComments } from '../../helpers/testComments.js'; +import { SAFE_DS_FILE_EXTENSIONS } from '../../../src/language/helpers/fileExtensions.js'; + +const services = (await createSafeDsServices(NodeFileSystem)).SafeDs; +const langiumDocuments = services.shared.workspace.LangiumDocuments; + +const rootResourceName = 'code actions'; +const commentRegex = /\s*apply\s*(?r)?"(?.*)"/gu; + +export const createCodeActionsTests = async (): Promise<CodeActionsTest[]> => { + const filesGroupedByParentDirectory = listTestSafeDsFilesGroupedByParentDirectory(rootResourceName); + const testCases = filesGroupedByParentDirectory.map((entry) => createCodeActionsTest(...entry)); + + return Promise.all(testCases); +}; + +const createCodeActionsTest = async (parentDirectory: URI, inputUris: URI[]): Promise<CodeActionsTest> => { + const outputRoot = URI.file(path.join(parentDirectory.fsPath, 'skip-output')); + const expectedOutputUris = listExpectedOutputFiles(outputRoot); + const inputs: CodeActionsInput[] = []; + + // Load all files, so they get linked + await loadDocuments(services, inputUris, { validation: true }); + + for (const uri of inputUris) { + const document = langiumDocuments.getDocument(uri)!; + const code = document.textDocument.getText(); + + // File must not contain syntax errors + const syntaxErrors = await getSyntaxErrors(services, code); + if (syntaxErrors.length > 0) { + return invalidTest('FILE', new SyntaxErrorsInCodeError(syntaxErrors, uri)); + } + + const testComments = findTestComments(code); + const codeActionTitles: (string | RegExp)[] = []; + + for (const comment of testComments) { + const match = commentRegex.exec(comment); + + // Comment must match the expected format + if (!match) { + return invalidTest('FILE', new InvalidCommentError(comment, uri)); + } + + const title = match.groups!.title!; + const titleIsRegex = match.groups!.titleIsRegex === 'r'; + + codeActionTitles.push(titleIsRegex ? new RegExp(title, 'gu') : title); + } + + inputs.push({ uri, codeActionTitles }); + } + + const shortenedResourceName = uriToShortenedTestResourceName(parentDirectory, rootResourceName); + return { + testName: `[${shortenedResourceName}]`, + inputs, + inputRoot: parentDirectory, + expectedOutputUris, + outputRoot, + }; +}; + +/** + * List all expected output files. + * + * @param outputRoot The directory, where output files are located. + */ +const listExpectedOutputFiles = (outputRoot: URI): URI[] => { + return listTestFilesWithExtensions(uriToShortenedTestResourceName(outputRoot), SAFE_DS_FILE_EXTENSIONS); +}; + +/** + * Report a test that has errors. + * + * @param level Whether a test file or a test suite is invalid. + * @param error The error that occurred. + */ +const invalidTest = (level: 'FILE' | 'SUITE', error: TestDescriptionError): CodeActionsTest => { + const shortenedResourceName = uriToShortenedTestResourceName(error.uri, rootResourceName); + const testName = `INVALID TEST ${level} [${shortenedResourceName}]`; + return { + testName, + inputs: [], + inputRoot: URI.file(''), + expectedOutputUris: [], + outputRoot: URI.file(''), + error, + }; +}; + +/** + * A description of a code actions test. + */ +interface CodeActionsTest extends TestDescription { + /** + * The original code. + */ + inputs: CodeActionsInput[]; + + /** + * The directory, where input files are located. + */ + inputRoot: URI; + + /** + * The expected output files. + */ + expectedOutputUris: URI[]; + + /** + * The directory, where output files are located. + */ + outputRoot: URI; +} + +/** + * A description of the input for code actions. + */ +interface CodeActionsInput { + /** + * The URI of the file. + */ + uri: URI; + + /** + * The titles of the code actions that should be applied. Strings must match exactly, regular expressions must match + * the entire string. + */ + codeActionTitles: (string | RegExp)[]; +} + +/** + * A test comment did not match the expected format. + */ +class InvalidCommentError extends TestDescriptionError { + constructor( + readonly comment: string, + uri: URI, + ) { + super(`Invalid test comment (refer to the documentation for guidance): ${comment}`, uri); + } +} diff --git a/packages/safe-ds-lang/tests/language/codeActions/safe-ds-code-action-provider.test.ts b/packages/safe-ds-lang/tests/language/codeActions/safe-ds-code-action-provider.test.ts new file mode 100644 index 000000000..5653c6a3b --- /dev/null +++ b/packages/safe-ds-lang/tests/language/codeActions/safe-ds-code-action-provider.test.ts @@ -0,0 +1,221 @@ +import { describe, expect, it } from 'vitest'; +import { NodeFileSystem } from 'langium/node'; +import { createCodeActionsTests } from './creator.js'; +import { loadDocuments } from '../../helpers/testResources.js'; +import { createSafeDsServices } from '../../../src/language/index.js'; +import { LangiumDocument, URI, UriUtils } from 'langium'; +import { + CodeAction, + CodeActionParams, + Command, + CreateFile, + DeleteFile, + RenameFile, + TextDocumentEdit, + WorkspaceEdit, +} from 'vscode-languageserver'; +import { TextDocument } from 'vscode-languageserver-textdocument'; + +const services = (await createSafeDsServices(NodeFileSystem)).SafeDs; +const langiumDocuments = services.shared.workspace.LangiumDocuments; +const codeActionProvider = services.lsp.CodeActionProvider!; + +const codeActionTests = createCodeActionsTests(); + +describe('code actions', async () => { + it.each(await codeActionTests)('$testName', async (test) => { + // Test is invalid + if (test.error) { + throw test.error; + } + + // Load all documents + const inputUris = test.inputs.map((input) => input.uri); + await loadDocuments(services, inputUris, { validation: true }); + + // Collect workspace edits + const edits: WorkspaceEdit[] = []; + + for (const input of test.inputs) { + const document = langiumDocuments.getDocument(input.uri)!; + const codeActions = (await getCodeActions(document)) ?? []; + + for (const action of codeActions) { + if (actionShouldBeApplied(action, input.codeActionTitles) && action.edit) { + edits.push(action.edit); + } + } + } + + // Compute actual output + const relevantUris = await applyWorkspaceEdits(edits, inputUris); + const actualOutputs = computeActualOutput(relevantUris, test.inputRoot, test.outputRoot); + + // File contents must match + for (const [uriString, code] of actualOutputs) { + const fsPath = uriString.fsPath; + await expect(code).toMatchFileSnapshot(fsPath); + } + + // File paths must match + const actualOutputPaths = Array.from(actualOutputs.keys()) + .map((uri) => uri.toString()) + .sort(); + const expectedOutputPaths = test.expectedOutputUris.map((uri) => uri.toString()).sort(); + expect(actualOutputPaths).toStrictEqual(expectedOutputPaths); + }); +}); + +const getCodeActions = async function getCodeActions(document: LangiumDocument) { + const params: CodeActionParams = { + textDocument: { + uri: document.textDocument.uri, + }, + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + }, + context: { + diagnostics: document.diagnostics ?? [], + }, + }; + + return codeActionProvider.getCodeActions(document, params); +}; + +const actionShouldBeApplied = (action: CodeAction | Command, titles: (string | RegExp)[]): action is CodeAction => { + if (!CodeAction.is(action)) { + return false; + } + + for (const title of titles) { + if (typeof title === 'string' && action.title === title) { + return true; + } else if (title instanceof RegExp && title.test(action.title)) { + return true; + } + } + + return false; +}; + +const applyWorkspaceEdits = async function (edits: WorkspaceEdit[], inputUris: URI[]): Promise<URI[]> { + const uris = [...inputUris]; + + for (const edit of edits) { + if (edit.documentChanges) { + await applyDocumentChanges(edit.documentChanges, uris); + } + } + + return uris; +}; + +const applyDocumentChanges = async function ( + changes: (TextDocumentEdit | CreateFile | RenameFile | DeleteFile)[], + uris: URI[], +) { + for (const change of changes) { + if (TextDocumentEdit.is(change)) { + await applyTextDocumentEdit(change); + } else if (CreateFile.is(change)) { + await applyCreateFile(change, uris); + } else if (RenameFile.is(change)) { + await applyRenameFile(change, uris); + } else if (DeleteFile.is(change)) { + await applyDeleteFile(change, uris); + } + } +}; + +const applyTextDocumentEdit = async function (change: TextDocumentEdit) { + const uri = URI.parse(change.textDocument.uri); + const document = langiumDocuments.getDocument(uri)!; + + const newCode = TextDocument.applyEdits(document.textDocument, change.edits); + langiumDocuments.deleteDocument(uri); + langiumDocuments.createDocument(uri, newCode); +}; + +const applyCreateFile = async function (change: CreateFile, uris: URI[]) { + const uri = URI.parse(change.uri); + + // Do nothing if the file exists already and should not be overwritten + const exists = langiumDocuments.hasDocument(uri); + if (exists && !change.options?.overwrite) { + return; + } + + // Apply the change + if (exists) { + langiumDocuments.deleteDocument(uri); + } + langiumDocuments.createDocument(uri, ''); + + // Update the list of URIs + if (uris.every((knownUri) => !UriUtils.equals(knownUri, uri))) { + uris.push(uri); + } +}; + +const applyRenameFile = async function (change: RenameFile, uris: URI[]) { + const oldUri = URI.parse(change.oldUri); + const newUri = URI.parse(change.newUri); + + // Do nothing if the old file does not exist + if (!langiumDocuments.hasDocument(oldUri)) { + return; + } + + // Do nothing if the new file exists already and should not be overwritten + const newExists = langiumDocuments.hasDocument(newUri); + if (newExists && !change.options?.overwrite) { + return; + } + + // Apply the change + const oldDocument = langiumDocuments.getDocument(oldUri)!; + langiumDocuments.deleteDocument(oldUri); + if (newExists) { + langiumDocuments.deleteDocument(newUri); + } + langiumDocuments.createDocument(newUri, oldDocument.textDocument.getText()); + + // Update the list of URIs + const index = uris.findIndex((knownUri) => UriUtils.equals(knownUri, oldUri)); + if (index >= 0) { + uris[index] = newUri; + } +}; + +const applyDeleteFile = async function (change: DeleteFile, uris: URI[]) { + const uri = URI.parse(change.uri); + + // Do nothing if the file does not exist + if (!langiumDocuments.hasDocument(uri)) { + return; + } + + // Apply the change + langiumDocuments.deleteDocument(uri); + + // Update the list of URIs + const index = uris.findIndex((knownUri) => UriUtils.equals(knownUri, uri)); + if (index >= 0) { + uris.splice(index, 1); + } +}; + +const computeActualOutput = (uris: URI[], inputRoot: URI, outputRoot: URI): Map<URI, string> => { + const result = new Map<URI, string>(); + + for (const uri of uris) { + const document = langiumDocuments.getDocument(uri)!; + const relativeUri = UriUtils.relative(inputRoot, uri); + const outputUri = UriUtils.resolvePath(outputRoot, relativeUri); + + result.set(outputUri, document.textDocument.getText()); + } + + return result; +}; diff --git a/packages/safe-ds-lang/tests/resources/code actions/quickfixes/make arguments assigned to optional parameters named/input.sdsdev b/packages/safe-ds-lang/tests/resources/code actions/quickfixes/make arguments assigned to optional parameters named/input.sdsdev new file mode 100644 index 000000000..b19f54957 --- /dev/null +++ b/packages/safe-ds-lang/tests/resources/code actions/quickfixes/make arguments assigned to optional parameters named/input.sdsdev @@ -0,0 +1,18 @@ +// $TEST$ apply "Add names to arguments that are assigned to optional parameters." + +package tests.codeActions.quickfixes.makeArgumentsAssignedToOptionalParametersNamed + +@Repeatable +annotation MyAnnotation(required: Int, optional1: Int = 0, optional2: Int = 0) + +@Pure +fun myFunction(required: Int, optional1: Int = 0, optional2: Int = 0) + +@MyAnnotation(1, 2, 3, 4) + +@MyAnnotation(1, 2, optional2 = 3) +pipeline testPipeline { + myFunction(1, 2, 3, 4); + + myFunction(1, 2, optional2 = 3); +} diff --git a/packages/safe-ds-lang/tests/resources/code actions/quickfixes/make arguments assigned to optional parameters named/skip-output/input.sdsdev b/packages/safe-ds-lang/tests/resources/code actions/quickfixes/make arguments assigned to optional parameters named/skip-output/input.sdsdev new file mode 100644 index 000000000..530de3006 --- /dev/null +++ b/packages/safe-ds-lang/tests/resources/code actions/quickfixes/make arguments assigned to optional parameters named/skip-output/input.sdsdev @@ -0,0 +1,18 @@ +// $TEST$ apply "Add names to arguments that are assigned to optional parameters." + +package tests.codeActions.quickfixes.makeArgumentsAssignedToOptionalParametersNamed + +@Repeatable +annotation MyAnnotation(required: Int, optional1: Int = 0, optional2: Int = 0) + +@Pure +fun myFunction(required: Int, optional1: Int = 0, optional2: Int = 0) + +@MyAnnotation(1, optional1 = 2, optional2 = 3, 4) + +@MyAnnotation(1, 2, optional2 = 3) +pipeline testPipeline { + myFunction(1, optional1 = 2, optional2 = 3, 4); + + myFunction(1, 2, optional2 = 3); +}