feat: optional parameters must be passed by name (#1307)

Closes #1163

### Summary of Changes

Optional parameters must now be passed by name. This allows reordering
them in the stubs and inserting new optional parameters anywhere instead
of just at the end. We can now also deprecate all optional parameters
without introducing a new function.

The error has an associated quickfix (our first). This PR also adds a
data-driven test system for quickfixes and other code actions.

.github/workflows/main.yml

@@ -12,7 +12,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [ 18.x ]
+        node-version: [ 20.x ]
 
     steps:
       - name: Checkout source

.github/workflows/pr.yml

@@ -17,7 +17,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [ 18.x ]
+        node-version: [ 20.x ]
 
     steps:
       - name: Checkout source

docs/development/testing/code-actions-testing.md

@@ -0,0 +1,49 @@
+# Code Actions Testing
+
+Code actions tests are data-driven instead of being specified explicitly. This document explains how to add a new
+code action test.
+
+## Adding a code action test
+
+1. Create a new **folder** (not just a file!) in the `tests/resources/code actions` directory or any subdirectory. Give
+   the folder a descriptive name, since the folder name becomes part of the test name.
+
+    !!! tip "Skipping a test"
+
+        If you want to skip a test, add the prefix `skip-` to the folder name.
+
+2. Add files with the extension `.sdsdev`, `.sds`, or `.sdsstub` **directly inside the folder**. All files in a
+   folder will be loaded into the same workspace, so they can reference each other. Files in different folders are
+   loaded into different workspaces, so they cannot reference each other.
+3. Add the Safe-DS code that you want to test to the files.
+4. Specify the code actions to apply using test comments (see [below](#format-of-test-comments)) at the top of the file.
+5. Run the tests. The test runner will automatically pick up the new test, and create a snapshot of the current output
+   after applying the selected code actions.
+6. Verify that the snapshot is correct, modify it if needed, and commit it.
+
+## Format of test comments
+
+1. As usual, test comments are single-line comments that start with `$TEST$`.
+2. Then, the keyword `apply` follows.
+3. Finally, you must specify the title of the code action enclosed in double-quotes. You can also add an `r` before the
+   opening double-quote to indicate that the title should be interpreted as a regular expression that must match the
+   entire actual title.
+
+Here are some examples:
+
+```ts
+// $TEST$ apply "Remove statement."
+```
+
+We apply all code actions with the exact title `Remove statement.`.
+
+```ts
+// $TEST$ apply r"^Remove.*"
+```
+
+We apply all code actions with a title that starts with `Remove`.
+
+
+## Updating the snapshots
+
+To quickly update the snapshots after changes to the code generator, run `vitest` with the `--update` flag.

docs/development/testing/generation-testing.md

@@ -28,14 +28,12 @@ document explains how to add a new generation test.
     ```ts
     // $TEST$ target
     ```
-5. Add another folder called `generated` inside the folder that you created in step 1. Place folders and Python files
-   inside the `generated` folder to specify the expected output of the program. The relative paths to the Python files
-   and the contents of the Python files will be compared to the actual generation output.
-6. Run the tests. The test runner will automatically pick up the new test.
+5. Run the tests. The test runner will automatically pick up the new test, and create a snapshot of the current output of the generator.
+6. Verify that the snapshot is correct, modify it if needed, and commit it.
 
-### Updating the expected output
+### Updating the snapshots
 
-To quickly update the expected output after changes to the code generator, run `vitest` with the `--update` flag.
+To quickly update the snapshots after changes to the code generator, run `vitest` with the `--update` flag.
 
 ## Markdown Generation
 
@@ -53,11 +51,9 @@ To quickly update the expected output after changes to the code generator, run `
    loaded into different workspaces, so they cannot reference each other. Generation will be triggered for all files in
    the folder.
 3. Add the Safe-DS code that you want to test to the files.
-4. Add another folder called `generated` inside the folder that you created in step 1. Place folders and Python files
-   inside the `generated` folder to specify the expected output of the program. The relative paths to the Python files
-   and the contents of the Python files will be compared to the actual generation output.
-5. Run the tests. The test runner will automatically pick up the new test.
+4. Run the tests. The test runner will automatically pick up the new test, and create a snapshot of the current output of the generator.
+5. Verify that the snapshot is correct, modify it if needed, and commit it.
 
-### Updating the expected output
+### Updating the snapshots
 
-To quickly update the expected output after changes to the code generator, run `vitest` with the `--update` flag.
+To quickly update the snapshots after changes to the code generator, run `vitest` with the `--update` flag.

docs/development/testing/validation-testing.md

@@ -32,7 +32,7 @@ validation test.
 ## Format of test comments
 
 1. As usual, test comments are single-line comments that start with `$TEST$`.
-2. Then, you specify whether the issue should be absent by writing `no` or present by writing nothing.
+2. Then, you specify whether the issue should be absent by writing `no`, or present by writing nothing.
 3. Next, you specify the severity of the issue by writing `error`, `warning`, `info`, or `hint`.
 4. Finally, you can optionally specify the message of the issue enclosed in double-quotes. You can also add an `r`
    before the opening double-quote to indicate that the expected message should be interpreted as a regular expression

docs/mkdocs.yml

@@ -62,6 +62,7 @@ nav:
   - Development:
     - Testing:
       - Call Graph Testing: development/testing/call-graph-testing.md
+      - Code Actions Testing: development/testing/code-actions-testing.md
       - Formatting Testing: development/testing/formatting-testing.md
       - Generation Testing: development/testing/generation-testing.md
       - Grammar Testing: development/testing/grammar-testing.md

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,
+    };
+};

packages/safe-ds-lang/src/language/codeActions/quickfixes/arguments.ts

@@ -0,0 +1,50 @@
+import { Diagnostic, TextEdit } from 'vscode-languageserver';
+import { LangiumDocument } from 'langium';
+import { SafeDsServices } from '../../safe-ds-module.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;
+    const nodeMapper = services.helpers.NodeMapper;
+
+    return (diagnostic: Diagnostic, document: LangiumDocument, acceptor: CodeActionAcceptor) => {
+        const node = locator.getAstNode(document.parseResult.value, diagnostic.data.path);
+        if (!isSdsArgumentList(node)) {
+            /* c8 ignore next 2 */
+            return;
+        }
+
+        acceptor(
+            createQuickfixFromTextEditsToSingleDocument(
+                'Add names to arguments that are assigned to optional parameters.',
+                diagnostic,
+                document,
+                node.arguments.flatMap((it) => ensureArgumentIsNamed(nodeMapper, it)),
+                true,
+            ),
+        );
+    };
+};
+
+const ensureArgumentIsNamed = (nodeMapper: SafeDsNodeMapper, argument: SdsArgument): TextEdit[] | TextEdit => {
+    const cstNode = argument.$cstNode;
+    if (!cstNode || Argument.isNamed(argument)) {
+        return [];
+    }
+
+    const parameter = nodeMapper.argumentToParameter(argument);
+    if (!parameter || Parameter.isRequired(parameter)) {
+        return [];
+    }
+
+    const text = argument.$cstNode.text;
+
+    return {
+        range: cstNode.range,
+        newText: `${parameter.name} = ${text}`,
+    };
+};

packages/safe-ds-lang/src/language/codeActions/quickfixes/safe-ds-quickfix-provider.ts

@@ -0,0 +1,34 @@
+import { Diagnostic } from 'vscode-languageserver';
+import { LangiumDocument } from 'langium';
+import { CODE_ARGUMENT_POSITIONAL } from '../../validation/other/expressions/arguments.js';
+import { SafeDsServices } from '../../safe-ds-module.js';
+import { makeArgumentsAssignedToOptionalParametersNamed } from './arguments.js';
+import { CodeActionAcceptor } from '../safe-ds-code-action-provider.js';
+
+export class SafeDsQuickfixProvider {
+    private readonly registry: QuickfixRegistry;
+
+    constructor(services: SafeDsServices) {
+        this.registry = {
+            [CODE_ARGUMENT_POSITIONAL]: [makeArgumentsAssignedToOptionalParametersNamed(services)],
+        };
+    }
+
+    createQuickfixes(diagnostic: Diagnostic, document: LangiumDocument, acceptor: CodeActionAcceptor) {
+        if (!diagnostic.code) {
+            /* c8 ignore next 2 */
+            return;
+        }
+
+        const quickfixes = this.registry[diagnostic.code] ?? [];
+        for (const quickfix of quickfixes) {
+            quickfix(diagnostic, document, acceptor);
+        }
+    }
+}
+
+type QuickfixRegistry = {
+    [code: string | number]: QuickfixCreator[];
+};
+
+type QuickfixCreator = (diagnostic: Diagnostic, document: LangiumDocument, acceptor: CodeActionAcceptor) => void;

packages/safe-ds-lang/src/language/codeActions/safe-ds-code-action-provider.ts

@@ -0,0 +1,31 @@
+import { CodeActionProvider } from 'langium/lsp';
+import { LangiumDocument, MaybePromise } from 'langium';
+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;
+
+    constructor(services: SafeDsServices) {
+        this.quickfixProvider = services.codeActions.QuickfixProvider;
+    }
+
+    getCodeActions(
+        document: LangiumDocument,
+        params: CodeActionParams,
+        _cancelToken?: CancellationToken,
+    ): MaybePromise<CodeAction[] | undefined> {
+        const result: CodeAction[] = [];
+        const acceptor = (action: CodeAction) => result.push(action);
+
+        for (const diagnostic of params.context.diagnostics) {
+            this.quickfixProvider.createQuickfixes(diagnostic, document, acceptor);
+        }
+
+        return isEmpty(result) ? undefined : result;
+    }
+}
+
+export type CodeActionAcceptor = (action: CodeAction) => void;

packages/safe-ds-lang/src/language/safe-ds-module.ts

@@ -57,6 +57,8 @@ import { SafeDsPythonServer } from './runtime/safe-ds-python-server.js';
 import { SafeDsSlicer } from './flow/safe-ds-slicer.js';
 import { SafeDsSyntheticProperties } from './helpers/safe-ds-synthetic-properties.js';
 import { SafeDsLinker } from './scoping/safe-ds-linker.js';
+import { SafeDsCodeActionProvider } from './codeActions/safe-ds-code-action-provider.js';
+import { SafeDsQuickfixProvider } from './codeActions/quickfixes/safe-ds-quickfix-provider.js';
 
 /**
  * Declaration of custom services - add your own service classes here.
@@ -68,6 +70,9 @@ export type SafeDsAddedServices = {
         Enums: SafeDsEnums;
         ImpurityReasons: SafeDsImpurityReasons;
     };
+    codeActions: {
+        QuickfixProvider: SafeDsQuickfixProvider;
+    };
     communication: {
         MessagingProvider: SafeDsMessagingProvider;
     };
@@ -139,6 +144,9 @@ export const SafeDsModule: Module<SafeDsServices, PartialLangiumServices & SafeD
         Enums: (services) => new SafeDsEnums(services),
         ImpurityReasons: (services) => new SafeDsImpurityReasons(services),
     },
+    codeActions: {
+        QuickfixProvider: (services) => new SafeDsQuickfixProvider(services),
+    },
     communication: {
         MessagingProvider: (services) => new SafeDsMessagingProvider(services),
     },
@@ -163,6 +171,7 @@ export const SafeDsModule: Module<SafeDsServices, PartialLangiumServices & SafeD
     },
     lsp: {
         CallHierarchyProvider: (services) => new SafeDsCallHierarchyProvider(services),
+        CodeActionProvider: (services) => new SafeDsCodeActionProvider(services),
         CodeLensProvider: (services) => new SafeDsCodeLensProvider(services),
         CompletionProvider: (services) => new SafeDsCompletionProvider(services),
         DocumentSymbolProvider: (services) => new SafeDsDocumentSymbolProvider(services),

packages/safe-ds-lang/src/language/validation/other/declarations/annotationCalls.ts

@@ -18,7 +18,7 @@ import {
 } from '../../../helpers/nodeProperties.js';
 import type { SafeDsServices } from '../../../safe-ds-module.js';
 
-export const CODE_ANNOTATION_CALL_CONSTANT_ARGUMENT = 'annotation-call/constant-argument';
+export const CODE_ANNOTATION_CALL_CONSTANT_ARGUMENT = 'annotation-call/non-constant-argument';
 export const CODE_ANNOTATION_CALL_MISSING_ARGUMENT_LIST = 'annotation-call/missing-argument-list';
 export const CODE_ANNOTATION_CALL_TARGET_PARAMETER = 'annotation-call/target-parameter';
 export const CODE_ANNOTATION_CALL_TARGET_RESULT = 'annotation-call/target-result';

packages/safe-ds-lang/src/language/validation/other/expressions/arguments.ts

@@ -0,0 +1,38 @@
+import { SafeDsServices } from '../../../safe-ds-module.js';
+import type { SdsArgumentList } from '../../../generated/ast.js';
+import { ValidationAcceptor } from 'langium';
+import { Argument, getArguments, Parameter } from '../../../helpers/nodeProperties.js';
+
+export const CODE_ARGUMENT_POSITIONAL = 'argument/positional';
+
+export const argumentMustBeNamedIfParameterIsOptional = (services: SafeDsServices) => {
+    const locator = services.workspace.AstNodeLocator;
+    const nodeMapper = services.helpers.NodeMapper;
+
+    return (node: SdsArgumentList, accept: ValidationAcceptor) => {
+        for (const argument of getArguments(node).toReversed()) {
+            const parameter = nodeMapper.argumentToParameter(argument);
+            if (!parameter) {
+                // Still keep going if there are extra arguments.
+                continue;
+            }
+            if (Parameter.isRequired(parameter)) {
+                // Required parameters must appear before optional parameters.
+                return;
+            }
+
+            if (!Argument.isNamed(argument)) {
+                accept('error', 'Argument must be named if the parameter is optional.', {
+                    node: argument,
+                    property: 'value',
+                    code: CODE_ARGUMENT_POSITIONAL,
+                    data: { path: locator.getAstNodePath(node) },
+                });
+
+                // Only show the error for the last argument. If users added names starting in the middle, we would no
+                // longer be able to assign the arguments to the correct parameters.
+                return;
+            }
+        }
+    };
+};

packages/safe-ds-lang/src/language/validation/other/expressions/calls.ts

@@ -3,7 +3,7 @@ import { type SdsCall } from '../../../generated/ast.js';
 import { getArguments, Parameter } from '../../../helpers/nodeProperties.js';
 import { SafeDsServices } from '../../../safe-ds-module.js';
 
-export const CODE_CALL_CONSTANT_ARGUMENT = 'call/constant-argument';
+export const CODE_CALL_CONSTANT_ARGUMENT = 'call/non-constant-argument';
 export const CODE_CALL_INFINITE_RECURSION = 'call/infinite-recursion';
 
 export const callArgumentMustBeConstantIfParameterIsConstant = (services: SafeDsServices) => {