diff --git a/packages/safe-ds-lang/src/language/validation/other/expressions/arguments.ts b/packages/safe-ds-lang/src/language/validation/other/expressions/arguments.ts index 3d1434ce0..3dce5b037 100644 --- a/packages/safe-ds-lang/src/language/validation/other/expressions/arguments.ts +++ b/packages/safe-ds-lang/src/language/validation/other/expressions/arguments.ts @@ -1,7 +1,7 @@ import { SafeDsServices } from '../../../safe-ds-module.js'; -import type { SdsArgument } from '../../../generated/ast.js'; +import type { SdsArgumentList } from '../../../generated/ast.js'; import { ValidationAcceptor } from 'langium'; -import { Argument, Parameter } from '../../../helpers/nodeProperties.js'; +import { Argument, getArguments, Parameter } from '../../../helpers/nodeProperties.js'; export const CODE_ARGUMENT_POSITIONAL = 'argument/positional'; @@ -9,19 +9,30 @@ export const argumentMustBeNamedIfParameterIsOptional = (services: SafeDsService const locator = services.workspace.AstNodeLocator; const nodeMapper = services.helpers.NodeMapper; - return (node: SdsArgument, accept: ValidationAcceptor) => { - const parameter = nodeMapper.argumentToParameter(node); - if (!parameter || !Parameter.isOptional(parameter)) { - return; - } + return (node: SdsArgumentList, accept: ValidationAcceptor) => { + for (const argument of getArguments(node).reverse()) { + 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) }, + }); - if (!Argument.isNamed(node)) { - accept('error', 'Argument must be named if the parameter is optional.', { - node, - 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; + } } }; }; diff --git a/packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts b/packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts index 047fda5b4..7c3f7576b 100644 --- a/packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts +++ b/packages/safe-ds-lang/src/language/validation/safe-ds-validator.ts @@ -222,11 +222,11 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsArgument: [ argumentCorrespondingParameterShouldNotBeDeprecated(services), argumentCorrespondingParameterShouldNotBeExperimental(services), - argumentMustBeNamedIfParameterIsOptional(services), ], SdsArgumentList: [ argumentListMustNotHavePositionalArgumentsAfterNamedArguments, argumentListMustNotSetParameterMultipleTimes(services), + argumentMustBeNamedIfParameterIsOptional(services), ], SdsAssignee: [ assigneeAssignedResultShouldNotBeDeprecated(services), diff --git a/packages/safe-ds-lang/tests/resources/validation/other/expressions/arguments/must be named if parameter is optional/annotation calls.sdsdev b/packages/safe-ds-lang/tests/resources/validation/other/expressions/arguments/must be named if parameter is optional/annotation calls.sdsdev index f012b20e8..bd0243f71 100644 --- a/packages/safe-ds-lang/tests/resources/validation/other/expressions/arguments/must be named if parameter is optional/annotation calls.sdsdev +++ b/packages/safe-ds-lang/tests/resources/validation/other/expressions/arguments/must be named if parameter is optional/annotation calls.sdsdev @@ -1,19 +1,22 @@ package tests.validation.other.expressions.arguments.mustBeNamedIfParameterIsOptional @Repeatable -annotation MyAnnotation(required: Int, optional: Int = 0) +annotation MyAnnotation(required: Int, optional1: Int = 0, optional2: Int = 0) // $TEST$ no error "Argument must be named if the parameter is optional." // $TEST$ error "Argument must be named if the parameter is optional." @MyAnnotation(»1«, »2«) // $TEST$ no error "Argument must be named if the parameter is optional." // $TEST$ no error "Argument must be named if the parameter is optional." -@MyAnnotation(»required = 1«, »optional = 2«) +// $TEST$ error "Argument must be named if the parameter is optional." +// $TEST$ no error "Argument must be named if the parameter is optional." +@MyAnnotation(»1«, »2«, »3«, »4«) +// $TEST$ no error "Argument must be named if the parameter is optional." +// $TEST$ no error "Argument must be named if the parameter is optional." +@MyAnnotation(»required = 1«, »optional1 = 2«) // $TEST$ no error "Argument must be named if the parameter is optional." @Unresolved(»1«) // $TEST$ no error "Argument must be named if the parameter is optional." -@MyAnnotation(1, 2, »3«) -// $TEST$ no error "Argument must be named if the parameter is optional." @MyAnnotation(unresolved = »1«) pipeline testPipeline {} diff --git a/packages/safe-ds-lang/tests/resources/validation/other/expressions/arguments/must be named if parameter is optional/calls.sdsdev b/packages/safe-ds-lang/tests/resources/validation/other/expressions/arguments/must be named if parameter is optional/calls.sdsdev index f74f52f00..0b4e82fba 100644 --- a/packages/safe-ds-lang/tests/resources/validation/other/expressions/arguments/must be named if parameter is optional/calls.sdsdev +++ b/packages/safe-ds-lang/tests/resources/validation/other/expressions/arguments/must be named if parameter is optional/calls.sdsdev @@ -1,6 +1,6 @@ package tests.validation.other.expressions.arguments.mustBeNamedIfParameterIsOptional -@Pure fun myFunction(required: Int, optional: Int = 0) +@Pure fun myFunction(required: Int, optional1: Int = 0, optional2: Int = 0) pipeline testPipeline { // $TEST$ no error "Argument must be named if the parameter is optional." @@ -8,12 +8,15 @@ pipeline testPipeline { myFunction(»1«, »2«); // $TEST$ no error "Argument must be named if the parameter is optional." // $TEST$ no error "Argument must be named if the parameter is optional." - myFunction(»required = 1«, »optional = 2«); + // $TEST$ error "Argument must be named if the parameter is optional." + // $TEST$ no error "Argument must be named if the parameter is optional." + myFunction(»1«, »2«, »3«, »4«); + // $TEST$ no error "Argument must be named if the parameter is optional." + // $TEST$ no error "Argument must be named if the parameter is optional." + myFunction(»required = 1«, »optional1 = 2«); // $TEST$ no error "Argument must be named if the parameter is optional." unresolved(»1«); // $TEST$ no error "Argument must be named if the parameter is optional." - myFunction(1, 2, »3«); - // $TEST$ no error "Argument must be named if the parameter is optional." myFunction(unresolved = »1«); }