feat: various checks for argument lists (#648)

Closes partially #543 ### Summary of Changes Show an error if an argument list of an annotation call or a call * does not set required parameters, * sets a parameter multiple times, * passes too many arguments. --------- Co-authored-by: megalinter-bot <[email protected]>
Safe-DS · Oct 18, 2023 · d76e597 · d76e597
1 parent 2d2ccc6
commit d76e597
Show file tree

Hide file tree

Showing 12 changed files with 770 additions and 43 deletions.
diff --git a/esbuild.mjs b/esbuild.mjs
@@ -1,6 +1,7 @@
 //@ts-check
 import * as esbuild from 'esbuild';
 import { copy } from 'esbuild-plugin-copy';
+import fs from 'fs/promises';
 
 const watch = process.argv.includes('--watch');
 const minify = process.argv.includes('--minify');
@@ -10,20 +11,18 @@ const success = watch ? 'Watch build succeeded' : 'Build succeeded';
 const getTime = function () {
     const date = new Date();
     return `[${`${padZeroes(date.getHours())}:${padZeroes(date.getMinutes())}:${padZeroes(date.getSeconds())}`}] `;
-}
+};
 
 const padZeroes = function (i) {
     return i.toString().padStart(2, '0');
-}
+};
 
 const plugins = [
     {
-        name: 'watch-plugin',
+        name: 'clean-old-builtins',
         setup(build) {
-            build.onEnd(result => {
-                if (result.errors.length === 0) {
-                    console.log(getTime() + success);
-                }
+            build.onStart(async () => {
+                await fs.rm('./out/resources', { force: true, recursive: true });
             });
         },
     },
@@ -34,26 +33,36 @@ const plugins = [
         },
         watch,
     }),
+    {
+        name: 'watch-plugin',
+        setup(build) {
+            build.onEnd((result) => {
+                if (result.errors.length === 0) {
+                    console.log(getTime() + success);
+                }
+            });
+        },
+    },
 ];
 
 const ctx = await esbuild.context({
     // Entry points for the vscode extension and the language server
     entryPoints: ['src/cli/main.ts', 'src/extension/main.ts', 'src/language/main.ts'],
     outdir: 'out',
     bundle: true,
-    target: "ES2017",
+    target: 'ES2017',
     // VSCode's extension host is still using cjs, so we need to transform the code
     format: 'cjs',
     // To prevent confusing node, we explicitly use the `.cjs` extension
     outExtension: {
-        '.js': '.cjs'
+        '.js': '.cjs',
     },
-    loader: {'.ts': 'ts'},
+    loader: { '.ts': 'ts' },
     external: ['vscode'],
     platform: 'node',
     sourcemap: !minify,
     minify,
-    plugins
+    plugins,
 });
 
 if (watch) {

diff --git a/src/language/helpers/nodeProperties.ts b/src/language/helpers/nodeProperties.ts
@@ -1,5 +1,6 @@
 import {
     isSdsAnnotation,
+    isSdsArgumentList,
     isSdsAssignment,
     isSdsAttribute,
     isSdsBlockLambda,
@@ -23,6 +24,7 @@ import {
     SdsAnnotation,
     SdsAnnotationCall,
     SdsArgument,
+    SdsArgumentList,
     SdsAssignee,
     SdsAssignment,
     SdsBlock,
@@ -161,12 +163,18 @@ export const findFirstAnnotationCallOf = (
     });
 };
 
-export const argumentsOrEmpty = (node: SdsAbstractCall | undefined): SdsArgument[] => {
-    return node?.argumentList?.arguments ?? [];
+export const argumentsOrEmpty = (node: SdsArgumentList | SdsAbstractCall | undefined): SdsArgument[] => {
+    if (isSdsArgumentList(node)) {
+        return node.arguments;
+    } else {
+        return node?.argumentList?.arguments ?? [];
+    }
 };
+
 export const assigneesOrEmpty = (node: SdsAssignment | undefined): SdsAssignee[] => {
     return node?.assigneeList?.assignees ?? [];
 };
+
 export const blockLambdaResultsOrEmpty = (node: SdsBlockLambda | undefined): SdsBlockLambdaResult[] => {
     return stream(statementsOrEmpty(node?.body))
         .filter(isSdsAssignment)

diff --git a/src/language/validation/builtins/repeatable.ts b/src/language/validation/builtins/repeatable.ts
@@ -16,6 +16,7 @@ export const singleUseAnnotationsMustNotBeRepeated =
         for (const duplicate of duplicatesBy(callsOfSingleUseAnnotations, (it) => it.annotation?.ref)) {
             accept('error', `The annotation '${duplicate.annotation.$refText}' is not repeatable.`, {
                 node: duplicate,
+                property: 'annotation',
                 code: CODE_ANNOTATION_NOT_REPEATABLE,
             });
         }

diff --git a/src/language/validation/other/argumentLists.ts b/src/language/validation/other/argumentLists.ts
@@ -1,7 +1,15 @@
-import { SdsArgumentList } from '../../generated/ast.js';
-import { ValidationAcceptor } from 'langium';
+import { isSdsAnnotation, isSdsCall, SdsAbstractCall, SdsArgumentList } from '../../generated/ast.js';
+import { getContainerOfType, ValidationAcceptor } from 'langium';
+import { SafeDsServices } from '../../safe-ds-module.js';
+import { argumentsOrEmpty, isRequiredParameter, parametersOrEmpty } from '../../helpers/nodeProperties.js';
+import { duplicatesBy } from '../../helpers/collectionUtils.js';
+import { isEmpty } from 'radash';
+import { pluralize } from '../../helpers/stringUtils.js';
 
+export const CODE_ARGUMENT_LIST_DUPLICATE_PARAMETER = 'argument-list/duplicate-parameter';
+export const CODE_ARGUMENT_LIST_MISSING_REQUIRED_PARAMETER = 'argument-list/missing-required-parameter';
 export const CODE_ARGUMENT_LIST_POSITIONAL_AFTER_NAMED = 'argument-list/positional-after-named';
+export const CODE_ARGUMENT_LIST_TOO_MANY_ARGUMENTS = 'argument-list/too-many-arguments';
 
 export const argumentListMustNotHavePositionalArgumentsAfterNamedArguments = (
     node: SdsArgumentList,
@@ -19,3 +27,107 @@ export const argumentListMustNotHavePositionalArgumentsAfterNamedArguments = (
         }
     }
 };
+
+export const argumentListMustNotHaveTooManyArguments = (services: SafeDsServices) => {
+    const nodeMapper = services.helpers.NodeMapper;
+
+    return (node: SdsAbstractCall, accept: ValidationAcceptor): void => {
+        const actualArgumentCount = argumentsOrEmpty(node).length;
+
+        // We can never have too many arguments in this case
+        if (actualArgumentCount === 0) {
+            return;
+        }
+
+        // We already report other errors in those cases
+        const callable = nodeMapper.callToCallableOrUndefined(node);
+        if (!callable || (isSdsCall(node) && isSdsAnnotation(callable))) {
+            return;
+        }
+
+        const parameters = parametersOrEmpty(callable);
+        const maxArgumentCount = parameters.length;
+
+        // All is good
+        if (actualArgumentCount <= maxArgumentCount) {
+            return;
+        }
+
+        const minArgumentCount = parameters.filter((it) => isRequiredParameter(it)).length;
+        const kind = pluralize(Math.max(minArgumentCount, maxArgumentCount), 'argument');
+        if (minArgumentCount === maxArgumentCount) {
+            accept('error', `Expected exactly ${minArgumentCount} ${kind} but got ${actualArgumentCount}.`, {
+                node,
+                property: 'argumentList',
+                code: CODE_ARGUMENT_LIST_TOO_MANY_ARGUMENTS,
+            });
+        } else {
+            accept(
+                'error',
+                `Expected between ${minArgumentCount} and ${maxArgumentCount} ${kind} but got ${actualArgumentCount}.`,
+                {
+                    node,
+                    property: 'argumentList',
+                    code: CODE_ARGUMENT_LIST_TOO_MANY_ARGUMENTS,
+                },
+            );
+        }
+    };
+};
+
+export const argumentListMustNotSetParameterMultipleTimes = (services: SafeDsServices) => {
+    const nodeMapper = services.helpers.NodeMapper;
+    const argumentToParameterOrUndefined = nodeMapper.argumentToParameterOrUndefined.bind(nodeMapper);
+
+    return (node: SdsArgumentList, accept: ValidationAcceptor): void => {
+        // We already report other errors in this case
+        const containingCall = getContainerOfType(node, isSdsCall);
+        const callable = nodeMapper.callToCallableOrUndefined(containingCall);
+        if (isSdsAnnotation(callable)) {
+            return;
+        }
+
+        const args = argumentsOrEmpty(node);
+        const duplicates = duplicatesBy(args, argumentToParameterOrUndefined);
+
+        for (const duplicate of duplicates) {
+            const correspondingParameter = argumentToParameterOrUndefined(duplicate)!;
+            accept('error', `The parameter '${correspondingParameter.name}' is already set.`, {
+                node: duplicate,
+                code: CODE_ARGUMENT_LIST_DUPLICATE_PARAMETER,
+            });
+        }
+    };
+};
+
+export const argumentListMustSetAllRequiredParameters = (services: SafeDsServices) => {
+    const nodeMapper = services.helpers.NodeMapper;
+
+    return (node: SdsAbstractCall, accept: ValidationAcceptor): void => {
+        const callable = nodeMapper.callToCallableOrUndefined(node);
+
+        // We already report other errors in those cases
+        if (!callable || (isSdsCall(node) && isSdsAnnotation(callable))) {
+            return;
+        }
+
+        const expectedParameters = parametersOrEmpty(callable).filter((it) => isRequiredParameter(it));
+        if (isEmpty(expectedParameters)) {
+            return;
+        }
+
+        const actualParameters = argumentsOrEmpty(node).map((it) => nodeMapper.argumentToParameterOrUndefined(it));
+
+        const missingTypeParameters = expectedParameters.filter((it) => !actualParameters.includes(it));
+        if (!isEmpty(missingTypeParameters)) {
+            const kind = pluralize(missingTypeParameters.length, 'parameter');
+            const missingParametersString = missingTypeParameters.map((it) => `'${it.name}'`).join(', ');
+
+            accept('error', `The ${kind} ${missingParametersString} must be set here.`, {
+                node,
+                property: 'argumentList',
+                code: CODE_ARGUMENT_LIST_MISSING_REQUIRED_PARAMETER,
+            });
+        }
+    };
+};
diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts
@@ -58,7 +58,12 @@ import {
     callableTypeMustNotHaveOptionalParameters,
     callableTypeParameterMustNotHaveConstModifier,
 } from './other/types/callableTypes.js';
-import { argumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/argumentLists.js';
+import {
+    argumentListMustNotHavePositionalArgumentsAfterNamedArguments,
+    argumentListMustNotHaveTooManyArguments,
+    argumentListMustNotSetParameterMultipleTimes,
+    argumentListMustSetAllRequiredParameters,
+} from './other/argumentLists.js';
 import {
     referenceMustNotBeFunctionPointer,
     referenceMustNotBeStaticClassOrEnumReference,
@@ -127,6 +132,10 @@ export const registerValidationChecks = function (services: SafeDsServices) {
             assignmentShouldNotImplicitlyIgnoreResult(services),
             assignmentShouldHaveMoreThanWildcardsAsAssignees,
         ],
+        SdsAbstractCall: [
+            argumentListMustNotHaveTooManyArguments(services),
+            argumentListMustSetAllRequiredParameters(services),
+        ],
         SdsAnnotation: [
             annotationMustContainUniqueNames,
             annotationParameterListShouldNotBeEmpty,
@@ -143,7 +152,10 @@ export const registerValidationChecks = function (services: SafeDsServices) {
             argumentCorrespondingParameterShouldNotBeDeprecated(services),
             argumentCorrespondingParameterShouldNotBeExperimental(services),
         ],
-        SdsArgumentList: [argumentListMustNotHavePositionalArgumentsAfterNamedArguments],
+        SdsArgumentList: [
+            argumentListMustNotHavePositionalArgumentsAfterNamedArguments,
+            argumentListMustNotSetParameterMultipleTimes(services),
+        ],
         SdsAttribute: [attributeMustHaveTypeHint],
         SdsBlockLambda: [blockLambdaMustContainUniqueNames],
         SdsCall: [

diff --git a/src/language/validation/types.ts b/src/language/validation/types.ts
@@ -46,7 +46,7 @@ export const callReceiverMustBeCallable = (services: SafeDsServices) => {
         }
 
         const callable = nodeMapper.callToCallableOrUndefined(node);
-        if (!callable) {
+        if (!callable || isSdsAnnotation(callable)) {
             accept('error', 'This expression is not callable.', {
                 node: node.receiver,
                 code: CODE_TYPE_CALLABLE_RECEIVER,

diff --git a/tests/resources/validation/builtins/annotations/repeatable/main.sdstest b/tests/resources/validation/builtins/annotations/repeatable/main.sdstest
@@ -6,19 +6,19 @@ annotation SingleUse
 annotation MultiUse
 
 // $TEST$ no error r"The annotation '\w*' is not repeatable\."
-»@SingleUse«
+@»SingleUse«
 // $TEST$ no error r"The annotation '\w*' is not repeatable\."
-»@MultiUse«
+@»MultiUse«
 // $TEST$ no error r"The annotation '\w*' is not repeatable\."
-»@MultiUse«
+@»MultiUse«
 // $TEST$ no error r"The annotation '\w*' is not repeatable\."
-»@UnresolvedAnnotation«
+@»UnresolvedAnnotation«
 // $TEST$ no error r"The annotation '\w*' is not repeatable\."
-»@UnresolvedAnnotation«
+@»UnresolvedAnnotation«
 class CorrectUse
 
 // $TEST$ no error r"The annotation '\w*' is not repeatable\."
-»@SingleUse«
+@»SingleUse«
 // $TEST$ error "The annotation 'SingleUse' is not repeatable."
-»@SingleUse«
+@»SingleUse«
 class IncorrectUse