Skip to content

Commit

Permalink
feat: only show this error for the last argument
Browse files Browse the repository at this point in the history
  • Loading branch information
lars-reimann committed Jan 8, 2025
1 parent a45382a commit c2c75f7
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 23 deletions.
Original file line number Diff line number Diff line change
@@ -1,27 +1,38 @@
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';

export const argumentMustBeNamedIfParameterIsOptional = (services: SafeDsServices) => {
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;
}
}
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {}
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
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."
// $TEST$ error "Argument must be named if the parameter is optional."
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«);
}

0 comments on commit c2c75f7

Please sign in to comment.