From 6f0abd01741c8c064cee8c34db8d07a781abe45a Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 22 Sep 2023 13:53:25 +0200 Subject: [PATCH 1/9] feat: error if type argument list of union type is empty --- .../validation/other/types/unionTypes.ts | 16 ++++++++++++++++ src/language/validation/safe-ds-validator.ts | 3 ++- .../must have type arguments/main.sdstest | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/language/validation/other/types/unionTypes.ts create mode 100644 tests/resources/validation/other/types/union types/must have type arguments/main.sdstest diff --git a/src/language/validation/other/types/unionTypes.ts b/src/language/validation/other/types/unionTypes.ts new file mode 100644 index 000000000..677b31a6a --- /dev/null +++ b/src/language/validation/other/types/unionTypes.ts @@ -0,0 +1,16 @@ +import { SdsUnionType } from '../../../generated/ast.js'; +import { ValidationAcceptor } from 'langium'; +import { typeArgumentsOrEmpty } from '../../../ast/shortcuts.js'; +import {isEmpty} from "radash"; + +export const CODE_UNION_TYPE_MISSING_TYPE_ARGUMENTS = 'union-type/missing-type-arguments'; + +export const unionTypeMustHaveTypeArguments = (node: SdsUnionType, accept: ValidationAcceptor): void => { + if (isEmpty(typeArgumentsOrEmpty(node.typeArgumentList))) { + accept('error', 'A union type must have least one type argument.', { + node, + property: 'typeArgumentList', + code: CODE_UNION_TYPE_MISSING_TYPE_ARGUMENTS, + }); + } +}; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index 80e417303..bfa40927f 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -38,6 +38,7 @@ import { parameterListVariadicParameterMustBeLast, } from './other/declarations/parameterLists.js'; import { importAliasMustNotBeUsedForWildcardImports } from './imports.js'; +import {unionTypeMustHaveTypeArguments} from "./other/types/unionTypes.js"; /** * Register custom validation checks. @@ -74,7 +75,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts], SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter], SdsTypeParameterList: [typeParameterListShouldNotBeEmpty], - SdsUnionType: [unionTypeShouldNotHaveASingularTypeArgument], + SdsUnionType: [unionTypeMustHaveTypeArguments, unionTypeShouldNotHaveASingularTypeArgument], SdsYield: [yieldMustNotBeUsedInPipeline], }; registry.register(checks, validator); diff --git a/tests/resources/validation/other/types/union types/must have type arguments/main.sdstest b/tests/resources/validation/other/types/union types/must have type arguments/main.sdstest new file mode 100644 index 000000000..363bbd7d9 --- /dev/null +++ b/tests/resources/validation/other/types/union types/must have type arguments/main.sdstest @@ -0,0 +1,16 @@ +package tests.numberOfTypeArguments + +// $TEST$ error "A union type must have least one type argument." +segment mySegment1( + p: union»<>« +) {} + +// $TEST$ no error "A union type must have least one type argument." +segment mySegment2( + p: union»« +) {} + +// $TEST$ no error "A union type must have least one type argument." +segment mySegment3( + p: union»« +) {} From 359b236d95c2dc1a59330837e0568d6a914dda3c Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 22 Sep 2023 14:03:07 +0200 Subject: [PATCH 2/9] fix: don't show error 'parameter-list/optional-and-variadic' on parameters that are variadic and optional We show another error for this case. --- .../other/declarations/parameterLists.ts | 4 ++-- .../main.sdstest | 16 ++++++++-------- .../must have type arguments/main.sdstest | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/language/validation/other/declarations/parameterLists.ts b/src/language/validation/other/declarations/parameterLists.ts index 2635fbe8c..f7851aabb 100644 --- a/src/language/validation/other/declarations/parameterLists.ts +++ b/src/language/validation/other/declarations/parameterLists.ts @@ -11,9 +11,9 @@ export const parameterListMustNotHaveOptionalAndVariadicParameters = ( ) => { const hasOptional = node.parameters.find((p) => p.defaultValue); if (hasOptional) { - const variadicParameters = node.parameters.filter((p) => p.variadic); + const variadicRequiredParameters = node.parameters.filter((p) => p.variadic && !p.defaultValue); - for (const variadic of variadicParameters) { + for (const variadic of variadicRequiredParameters) { accept('error', 'A callable with optional parameters must not have a variadic parameter.', { node: variadic, property: 'name', diff --git a/tests/resources/validation/other/declarations/parameter lists/must not mix optional and variadic/main.sdstest b/tests/resources/validation/other/declarations/parameter lists/must not mix optional and variadic/main.sdstest index f321b8ab8..d497fd3ff 100644 --- a/tests/resources/validation/other/declarations/parameter lists/must not mix optional and variadic/main.sdstest +++ b/tests/resources/validation/other/declarations/parameter lists/must not mix optional and variadic/main.sdstest @@ -9,7 +9,7 @@ annotation MyAnnotation1(»a«: Int, »b«: Int = 1, »c«: Int, »d«: Int = 2, // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." -annotation MyAnnotation2(»a«: Int, vararg »b«: Int) +annotation MyAnnotation2(»a«: Int, vararg »b«: Int = 1) // $TEST$ error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." @@ -26,7 +26,7 @@ class MyClass1(»a«: Int, »b«: Int = 1, »c«: Int, »d«: Int = 2, vararg » // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." -class MyClass2(»a«: Int, vararg »b«: Int) +class MyClass2(»a«: Int, vararg »b«: Int = 1) // $TEST$ error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." @@ -44,7 +44,7 @@ enum MyEnum { // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." - MyEnumVariant2(»a«: Int, vararg »b«: Int) + MyEnumVariant2(»a«: Int, vararg »b«: Int = 1) // $TEST$ error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." @@ -62,7 +62,7 @@ fun myFunction1(»a«: Int, »b«: Int = 1, »c«: Int, »d«: Int = 2, vararg // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." -fun myFunction2(»a«: Int, vararg »b«: Int) +fun myFunction2(»a«: Int, vararg »b«: Int = 1) // $TEST$ error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." @@ -79,7 +79,7 @@ segment mySegment1(»a«: Int, »b«: Int = 1, »c«: Int, »d«: Int = 2, varar // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." -segment mySegment2(»a«: Int, vararg »b«: Int) {} +segment mySegment2(»a«: Int, vararg »b«: Int = 1) {} // $TEST$ error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." @@ -97,7 +97,7 @@ pipeline myPipeline { // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." - (»a«: Int, vararg »b«: Int) {}; + (»a«: Int, vararg »b«: Int = 1) {}; // $TEST$ error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." @@ -114,7 +114,7 @@ pipeline myPipeline { // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." - (»a«: Int, vararg »b«: Int) -> 1; + (»a«: Int, vararg »b«: Int = 1) -> 1; // $TEST$ error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." @@ -133,7 +133,7 @@ fun myFunction4( // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." - p2: (»a«: Int, vararg »b«: Int) -> (), + p2: (»a«: Int, vararg »b«: Int = 1) -> (), // $TEST$ error "A callable with optional parameters must not have a variadic parameter." // $TEST$ no error "A callable with optional parameters must not have a variadic parameter." diff --git a/tests/resources/validation/other/types/union types/must have type arguments/main.sdstest b/tests/resources/validation/other/types/union types/must have type arguments/main.sdstest index 363bbd7d9..faface2c1 100644 --- a/tests/resources/validation/other/types/union types/must have type arguments/main.sdstest +++ b/tests/resources/validation/other/types/union types/must have type arguments/main.sdstest @@ -1,4 +1,4 @@ -package tests.numberOfTypeArguments +package tests.validation.types.unionTypes.mustHaveTypeArguments // $TEST$ error "A union type must have least one type argument." segment mySegment1( From c40384966e8a97154827d4198c8aa324b4be18cb Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 22 Sep 2023 14:08:04 +0200 Subject: [PATCH 3/9] feat: error if callable type has optional parameters --- .../validation/other/types/callableTypes.ts | 17 +++++++++++++++++ src/language/validation/safe-ds-validator.ts | 3 ++- .../optional.sdstest | 8 ++++++++ .../required.sdstest | 8 ++++++++ .../variadic optional.sdstest | 8 ++++++++ .../variadic required.sdstest | 8 ++++++++ 6 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 src/language/validation/other/types/callableTypes.ts create mode 100644 tests/resources/validation/other/types/callable types/must not have optional parameters/optional.sdstest create mode 100644 tests/resources/validation/other/types/callable types/must not have optional parameters/required.sdstest create mode 100644 tests/resources/validation/other/types/callable types/must not have optional parameters/variadic optional.sdstest create mode 100644 tests/resources/validation/other/types/callable types/must not have optional parameters/variadic required.sdstest diff --git a/src/language/validation/other/types/callableTypes.ts b/src/language/validation/other/types/callableTypes.ts new file mode 100644 index 000000000..4a27db433 --- /dev/null +++ b/src/language/validation/other/types/callableTypes.ts @@ -0,0 +1,17 @@ +import { SdsCallableType } from '../../../generated/ast.js'; +import { ValidationAcceptor } from 'langium'; +import { parametersOrEmpty } from '../../../ast/shortcuts.js'; + +export const CODE_CALLABLE_TYPE_NO_OPTIONAL_PARAMETERS = 'callable-type/no-optional-parameters'; + +export const callableTypeMustNotHaveOptionalParameters = (node: SdsCallableType, accept: ValidationAcceptor): void => { + for (const parameter of parametersOrEmpty(node.parameterList)) { + if (parameter.defaultValue && !parameter.variadic) { + accept('error', 'A callable type must not have optional parameters.', { + node: parameter, + property: 'defaultValue', + code: CODE_CALLABLE_TYPE_NO_OPTIONAL_PARAMETERS, + }); + } + } +}; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index bfa40927f..72e3095f2 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -39,6 +39,7 @@ import { } from './other/declarations/parameterLists.js'; import { importAliasMustNotBeUsedForWildcardImports } from './imports.js'; import {unionTypeMustHaveTypeArguments} from "./other/types/unionTypes.js"; +import {callableTypeMustNotHaveOptionalParameters} from "./other/types/callableTypes.js"; /** * Register custom validation checks. @@ -51,7 +52,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsAnnotation: [annotationMustContainUniqueNames, annotationParameterListShouldNotBeEmpty], SdsAttribute: [attributeMustHaveTypeHint], SdsBlockLambda: [blockLambdaMustContainUniqueNames], - SdsCallableType: [callableTypeMustContainUniqueNames], + SdsCallableType: [callableTypeMustContainUniqueNames, callableTypeMustNotHaveOptionalParameters], SdsClass: [classMustContainUniqueNames], SdsClassBody: [classBodyShouldNotBeEmpty], SdsConstraintList: [constraintListShouldNotBeEmpty], diff --git a/tests/resources/validation/other/types/callable types/must not have optional parameters/optional.sdstest b/tests/resources/validation/other/types/callable types/must not have optional parameters/optional.sdstest new file mode 100644 index 000000000..a8709f658 --- /dev/null +++ b/tests/resources/validation/other/types/callable types/must not have optional parameters/optional.sdstest @@ -0,0 +1,8 @@ +package tests.validation.types.callableTypes.mustNotHaveOptionalParameters + +fun f( + g: ( + // $TEST$ error "A callable type must not have optional parameters." + p: Int = »1«, + ) -> () +) diff --git a/tests/resources/validation/other/types/callable types/must not have optional parameters/required.sdstest b/tests/resources/validation/other/types/callable types/must not have optional parameters/required.sdstest new file mode 100644 index 000000000..794be82b1 --- /dev/null +++ b/tests/resources/validation/other/types/callable types/must not have optional parameters/required.sdstest @@ -0,0 +1,8 @@ +package tests.validation.types.callableTypes.mustNotHaveOptionalParameters + +fun f( + g: ( + // $TEST$ no error "A callable type must not have optional parameters." + p: Int, + ) -> () +) diff --git a/tests/resources/validation/other/types/callable types/must not have optional parameters/variadic optional.sdstest b/tests/resources/validation/other/types/callable types/must not have optional parameters/variadic optional.sdstest new file mode 100644 index 000000000..d7972bdc6 --- /dev/null +++ b/tests/resources/validation/other/types/callable types/must not have optional parameters/variadic optional.sdstest @@ -0,0 +1,8 @@ +package tests.validation.types.callableTypes.mustNotHaveOptionalParameters + +fun f( + g: ( + // $TEST$ no error "A callable type must not have optional parameters." + vararg p: Int = 1, + ) -> () +) diff --git a/tests/resources/validation/other/types/callable types/must not have optional parameters/variadic required.sdstest b/tests/resources/validation/other/types/callable types/must not have optional parameters/variadic required.sdstest new file mode 100644 index 000000000..614ea95e0 --- /dev/null +++ b/tests/resources/validation/other/types/callable types/must not have optional parameters/variadic required.sdstest @@ -0,0 +1,8 @@ +package tests.validation.types.callableTypes.mustNotHaveOptionalParameters + +fun f( + g: ( + // $TEST$ no error "A callable type must not have optional parameters." + vararg p: Int, + ) -> () +) From 9eb9dd5bbb89194f25a6154cb221cb34c96d7524 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 22 Sep 2023 14:20:37 +0200 Subject: [PATCH 4/9] fix: allow test markers in type argument lists --- src/language/grammar/safe-ds.langium | 20 +++++++++---------- ...ed type argument with test markers.sdstest | 5 +++++ ...l type argument with tests markers.sdstest | 5 +++++ 3 files changed, 20 insertions(+), 10 deletions(-) create mode 100644 tests/resources/grammar/expressions/calls/good-named type argument with test markers.sdstest create mode 100644 tests/resources/grammar/expressions/calls/good-positional type argument with tests markers.sdstest diff --git a/src/language/grammar/safe-ds.langium b/src/language/grammar/safe-ds.langium index 400522119..f96eeb57f 100644 --- a/src/language/grammar/safe-ds.langium +++ b/src/language/grammar/safe-ds.langium @@ -1045,16 +1045,16 @@ terminal TEMPLATE_STRING_END returns string: TEMPLATE_EXPRESSION_END STRING_TEXT terminal CALL_TYPE_ARGUMENT_LIST_START: '<' (?= - /\s*/ - ( '*' // Star projection as positional type argument - | 'in' // Contravariant type projection as positional type argument - | 'out' // Covariant type projection as positional type argument - | 'literal' // Invariant literal type as positional type argument - | 'union' // Invariant union type as positional type argument - | '>' // Empty type argument list - | ID /\s*/ - ( '=' // Named type argument - | ('.' /\s*/ ID /\s*/)* (',' | '>') // Invariant type projection as positional type argument + /[\s»«]*/ + ( '*' // Star projection as positional type argument + | 'in' // Contravariant type projection as positional type argument + | 'out' // Covariant type projection as positional type argument + | 'literal' // Invariant literal type as positional type argument + | 'union' // Invariant union type as positional type argument + | '>' // Empty type argument list + | ID /[\s»«]*/ + ( '=' // Named type argument + | ('.' /[\s»«]*/ ID /[\s»«]*/)* (',' | '>') // Invariant type projection as positional type argument ) ) ) diff --git a/tests/resources/grammar/expressions/calls/good-named type argument with test markers.sdstest b/tests/resources/grammar/expressions/calls/good-named type argument with test markers.sdstest new file mode 100644 index 000000000..1233c71f7 --- /dev/null +++ b/tests/resources/grammar/expressions/calls/good-named type argument with test markers.sdstest @@ -0,0 +1,5 @@ +// $TEST$ no_syntax_error + +pipeline myPipeline { + f<»T = Int«>(); +} diff --git a/tests/resources/grammar/expressions/calls/good-positional type argument with tests markers.sdstest b/tests/resources/grammar/expressions/calls/good-positional type argument with tests markers.sdstest new file mode 100644 index 000000000..748f7fe60 --- /dev/null +++ b/tests/resources/grammar/expressions/calls/good-positional type argument with tests markers.sdstest @@ -0,0 +1,5 @@ +// $TEST$ no_syntax_error + +pipeline myPipeline { + f<»Int«>(); +} From 0faa5fbef5604120a5bb7a9f92efe0660be8e3dd Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 22 Sep 2023 14:26:40 +0200 Subject: [PATCH 5/9] feat: error if positional type argument occurs after named type argument --- .../other/types/typeArgumentLists.ts | 21 ++++++++++++ src/language/validation/safe-ds-validator.ts | 9 +++-- .../main.sdstest | 33 +++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 src/language/validation/other/types/typeArgumentLists.ts create mode 100644 tests/resources/validation/other/types/type argument lists/must not have positional type argument after named type argument/main.sdstest diff --git a/src/language/validation/other/types/typeArgumentLists.ts b/src/language/validation/other/types/typeArgumentLists.ts new file mode 100644 index 000000000..aae56f5ce --- /dev/null +++ b/src/language/validation/other/types/typeArgumentLists.ts @@ -0,0 +1,21 @@ +import { SdsTypeArgumentList } from '../../../generated/ast.js'; +import { ValidationAcceptor } from 'langium'; + +export const CODE_TYPE_ARGUMENT_LIST_POSITIONAL_AFTER_NAMED = 'type-argument-list/positional-after-named'; + +export const typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments = ( + node: SdsTypeArgumentList, + accept: ValidationAcceptor, +): void => { + let foundNamed = false; + for (const typeArgument of node.typeArguments) { + if (typeArgument.typeParameter) { + foundNamed = true; + } else if (foundNamed) { + accept('error', 'After the first named type argument all type arguments must be named.', { + node: typeArgument, + code: CODE_TYPE_ARGUMENT_LIST_POSITIONAL_AFTER_NAMED, + }); + } + } +}; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index 72e3095f2..bcd4f5ff0 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -38,8 +38,9 @@ import { parameterListVariadicParameterMustBeLast, } from './other/declarations/parameterLists.js'; import { importAliasMustNotBeUsedForWildcardImports } from './imports.js'; -import {unionTypeMustHaveTypeArguments} from "./other/types/unionTypes.js"; -import {callableTypeMustNotHaveOptionalParameters} from "./other/types/callableTypes.js"; +import { unionTypeMustHaveTypeArguments } from './other/types/unionTypes.js'; +import { callableTypeMustNotHaveOptionalParameters } from './other/types/callableTypes.js'; +import { typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/types/typeArgumentLists.js'; /** * Register custom validation checks. @@ -74,6 +75,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsResult: [resultMustHaveTypeHint], SdsSegment: [segmentMustContainUniqueNames, segmentResultListShouldNotBeEmpty], SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts], + SdsTypeArgumentList: [typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments], SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter], SdsTypeParameterList: [typeParameterListShouldNotBeEmpty], SdsUnionType: [unionTypeMustHaveTypeArguments, unionTypeShouldNotHaveASingularTypeArgument], @@ -85,4 +87,5 @@ export const registerValidationChecks = function (services: SafeDsServices) { /** * Implementation of custom validations. */ -export class SafeDsValidator {} +export class SafeDsValidator { +} diff --git a/tests/resources/validation/other/types/type argument lists/must not have positional type argument after named type argument/main.sdstest b/tests/resources/validation/other/types/type argument lists/must not have positional type argument after named type argument/main.sdstest new file mode 100644 index 000000000..9bbc778a2 --- /dev/null +++ b/tests/resources/validation/other/types/type argument lists/must not have positional type argument after named type argument/main.sdstest @@ -0,0 +1,33 @@ +package tests.validation.types.typeArgumentLists.mustNotHavePositionalTypeArgumentAfterNamedTypeArgument + +// $TEST$ no error "After the first named type argument all type arguments must be named." +// $TEST$ no error "After the first named type argument all type arguments must be named." +// $TEST$ error "After the first named type argument all type arguments must be named." +// $TEST$ no error "After the first named type argument all type arguments must be named." +segment mySegment1( + f: MyClass<»Int«, »A = Int«, »Int«, »B = Int«> +) {} + +// $TEST$ no error "After the first named type argument all type arguments must be named." +segment mySegment2( + f: MyClass<»Int«> +) {} + +// $TEST$ no error "After the first named type argument all type arguments must be named." +segment mySegment2( + f: MyClass<»A = Int«> +) {} + +pipeline myPipeline { + // $TEST$ no error "After the first named type argument all type arguments must be named." + // $TEST$ no error "After the first named type argument all type arguments must be named." + // $TEST$ error "After the first named type argument all type arguments must be named." + // $TEST$ no error "After the first named type argument all type arguments must be named." + call<»Int«, »A = Int«, »Int«, »B = Int«>(); + + // $TEST$ no error "After the first named type argument all type arguments must be named." + call<»Int«>(); + + // $TEST$ no error "After the first named type argument all type arguments must be named." + call<»A = Int«>(); +} From c2de24829bba6d26a7277f2dca2c9457c5af93bd Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 22 Sep 2023 14:34:38 +0200 Subject: [PATCH 6/9] feat: error if positional argument occurs after named argument --- .../validation/other/argumentLists.ts | 21 +++++++ src/language/validation/safe-ds-validator.ts | 5 +- .../main.sdstest | 63 +++++++++++++++++++ 3 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 src/language/validation/other/argumentLists.ts create mode 100644 tests/resources/validation/other/argument lists/must not have positional argument after named argument/main.sdstest diff --git a/src/language/validation/other/argumentLists.ts b/src/language/validation/other/argumentLists.ts new file mode 100644 index 000000000..ccd50e5bb --- /dev/null +++ b/src/language/validation/other/argumentLists.ts @@ -0,0 +1,21 @@ +import { SdsArgumentList } from '../../generated/ast.js'; +import { ValidationAcceptor } from 'langium'; + +export const CODE_ARGUMENT_LIST_POSITIONAL_AFTER_NAMED = 'argument-list/positional-after-named'; + +export const argumentListMustNotHavePositionalArgumentsAfterNamedArguments = ( + node: SdsArgumentList, + accept: ValidationAcceptor, +): void => { + let foundNamed = false; + for (const argument of node.arguments) { + if (argument.parameter) { + foundNamed = true; + } else if (foundNamed) { + accept('error', 'After the first named argument all arguments must be named.', { + node: argument, + code: CODE_ARGUMENT_LIST_POSITIONAL_AFTER_NAMED, + }); + } + } +}; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index bcd4f5ff0..9ff136531 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -41,6 +41,7 @@ import { importAliasMustNotBeUsedForWildcardImports } from './imports.js'; import { unionTypeMustHaveTypeArguments } from './other/types/unionTypes.js'; import { callableTypeMustNotHaveOptionalParameters } from './other/types/callableTypes.js'; import { typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/types/typeArgumentLists.js'; +import { argumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/argumentLists.js'; /** * Register custom validation checks. @@ -51,6 +52,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { const checks: ValidationChecks = { SdsAssignment: [assignmentShouldHaveMoreThanWildcardsAsAssignees], SdsAnnotation: [annotationMustContainUniqueNames, annotationParameterListShouldNotBeEmpty], + SdsArgumentList: [argumentListMustNotHavePositionalArgumentsAfterNamedArguments], SdsAttribute: [attributeMustHaveTypeHint], SdsBlockLambda: [blockLambdaMustContainUniqueNames], SdsCallableType: [callableTypeMustContainUniqueNames, callableTypeMustNotHaveOptionalParameters], @@ -87,5 +89,4 @@ export const registerValidationChecks = function (services: SafeDsServices) { /** * Implementation of custom validations. */ -export class SafeDsValidator { -} +export class SafeDsValidator {} diff --git a/tests/resources/validation/other/argument lists/must not have positional argument after named argument/main.sdstest b/tests/resources/validation/other/argument lists/must not have positional argument after named argument/main.sdstest new file mode 100644 index 000000000..234cd6572 --- /dev/null +++ b/tests/resources/validation/other/argument lists/must not have positional argument after named argument/main.sdstest @@ -0,0 +1,63 @@ +package tests.validation.other.argumentLists.mustNotHavePositionalArgumentAfterNamedArgument + +annotation MyAnnotation(a: Int, b: Int = 0, c: Int = 0, d: Int = 0, vararg e: Int) + +// $TEST$ no error "After the first named argument all arguments must be named." +// $TEST$ no error "After the first named argument all arguments must be named." +// $TEST$ error "After the first named argument all arguments must be named." +// $TEST$ no error "After the first named argument all arguments must be named." +// $TEST$ error "After the first named argument all arguments must be named." +@MyAnnotation(»0«, »a = 1«, »2«, »b = 3«, »4«) +class MyClass1 + +// $TEST$ no error "After the first named argument all arguments must be named." +@MyAnnotation(»0«) +class MyClass2 + +// $TEST$ no error "After the first named argument all arguments must be named." +@MyAnnotation(»a = 0«) +class MyClass3 + +// $TEST$ no error "After the first named argument all arguments must be named." +// $TEST$ no error "After the first named argument all arguments must be named." +// $TEST$ error "After the first named argument all arguments must be named." +// $TEST$ no error "After the first named argument all arguments must be named." +@UnresolvedAnnotation(»0«, »a = 1«, »2«, »b = 3«) +class MyClass4 + +// $TEST$ no error "After the first named argument all arguments must be named." +@UnresolvedAnnotation(»0«) +class MyClass5 + +// $TEST$ no error "After the first named argument all arguments must be named." +@UnresolvedAnnotation(»a = 0«) +class MyClass3 + +fun f(a: Int, b: Int = 0, c: Int = 0, d: Int = 0, vararg e: Int) + +pipeline myPipeline { + // $TEST$ no error "After the first named argument all arguments must be named." + // $TEST$ no error "After the first named argument all arguments must be named." + // $TEST$ error "After the first named argument all arguments must be named." + // $TEST$ no error "After the first named argument all arguments must be named." + // $TEST$ error "After the first named argument all arguments must be named." + f(»0«, »a = 1«, »2«, »b = 3«, »4«); + + // $TEST$ no error "After the first named argument all arguments must be named." + f(»0«); + + // $TEST$ no error "After the first named argument all arguments must be named." + f(»a = 0«); + + // $TEST$ no error "After the first named argument all arguments must be named." + // $TEST$ no error "After the first named argument all arguments must be named." + // $TEST$ error "After the first named argument all arguments must be named." + // $TEST$ no error "After the first named argument all arguments must be named." + unresolvedCallable(»0«, »a = 1«, »2«, »b = 3«); + + // $TEST$ no error "After the first named argument all arguments must be named." + unresolvedCallable(»0«); + + // $TEST$ no error "After the first named argument all arguments must be named." + unresolvedCallable(»a = 0«); +} From bbec9a02c619e1df2f18a78274462b0382813815 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 22 Sep 2023 14:48:55 +0200 Subject: [PATCH 7/9] feat: error if parameter is optional and variadic --- .../validation/other/declarations/parameters.ts | 14 ++++++++++++++ src/language/validation/{ => other}/imports.ts | 4 ++-- src/language/validation/safe-ds-validator.ts | 5 +++-- src/language/validation/value.ts | 3 +++ .../variadic must not be optional/main.sdstest | 12 ++++++++++++ 5 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 src/language/validation/other/declarations/parameters.ts rename src/language/validation/{ => other}/imports.ts (82%) create mode 100644 src/language/validation/value.ts create mode 100644 tests/resources/validation/other/declarations/parameter/variadic must not be optional/main.sdstest diff --git a/src/language/validation/other/declarations/parameters.ts b/src/language/validation/other/declarations/parameters.ts new file mode 100644 index 000000000..064fd74bd --- /dev/null +++ b/src/language/validation/other/declarations/parameters.ts @@ -0,0 +1,14 @@ +import { SdsParameter } from '../../../generated/ast.js'; +import { ValidationAcceptor } from 'langium'; + +export const CODE_PARAMETER_VARIADIC_AND_OPTIONAL = 'parameter/variadic-and-optional'; + +export const parameterMustNotBeVariadicAndOptional = (node: SdsParameter, accept: ValidationAcceptor) => { + if (node.variadic && node.defaultValue) { + accept('error', 'Variadic parameters must not be optional.', { + node, + property: 'name', + code: CODE_PARAMETER_VARIADIC_AND_OPTIONAL, + }); + } +}; diff --git a/src/language/validation/imports.ts b/src/language/validation/other/imports.ts similarity index 82% rename from src/language/validation/imports.ts rename to src/language/validation/other/imports.ts index 9cf781d81..99210eba1 100644 --- a/src/language/validation/imports.ts +++ b/src/language/validation/other/imports.ts @@ -1,6 +1,6 @@ import { ValidationAcceptor } from 'langium'; -import { SdsImportAlias } from '../generated/ast.js'; -import { isWildcardImport } from '../ast/checks.js'; +import { SdsImportAlias } from '../../generated/ast.js'; +import { isWildcardImport } from '../../ast/checks.js'; export const CODE_IMPORT_WILDCARD_IMPORT_WITH_ALIAS = 'import/wildcard-import-with-alias'; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index 9ff136531..7109b13a7 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -37,11 +37,12 @@ import { parameterListMustNotHaveRequiredParametersAfterOptionalParameters, parameterListVariadicParameterMustBeLast, } from './other/declarations/parameterLists.js'; -import { importAliasMustNotBeUsedForWildcardImports } from './imports.js'; +import { importAliasMustNotBeUsedForWildcardImports } from './other/imports.js'; import { unionTypeMustHaveTypeArguments } from './other/types/unionTypes.js'; import { callableTypeMustNotHaveOptionalParameters } from './other/types/callableTypes.js'; import { typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/types/typeArgumentLists.js'; import { argumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/argumentLists.js'; +import {parameterMustNotBeVariadicAndOptional} from "./other/declarations/parameters.js"; /** * Register custom validation checks. @@ -67,7 +68,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsFunction: [functionMustContainUniqueNames, functionResultListShouldNotBeEmpty], SdsImportAlias: [importAliasMustNotBeUsedForWildcardImports], SdsModule: [moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage], - SdsParameter: [parameterMustHaveTypeHint], + SdsParameter: [parameterMustHaveTypeHint, parameterMustNotBeVariadicAndOptional], SdsParameterList: [ parameterListMustNotHaveOptionalAndVariadicParameters, parameterListMustNotHaveRequiredParametersAfterOptionalParameters, diff --git a/src/language/validation/value.ts b/src/language/validation/value.ts new file mode 100644 index 000000000..8ec488838 --- /dev/null +++ b/src/language/validation/value.ts @@ -0,0 +1,3 @@ +// TODO: implement value-related checks here, including: +// - division by zero +// - must be constant diff --git a/tests/resources/validation/other/declarations/parameter/variadic must not be optional/main.sdstest b/tests/resources/validation/other/declarations/parameter/variadic must not be optional/main.sdstest new file mode 100644 index 000000000..ae90eaae3 --- /dev/null +++ b/tests/resources/validation/other/declarations/parameter/variadic must not be optional/main.sdstest @@ -0,0 +1,12 @@ +package tests.validation.declarations.parameters.variadicMustNotBeOptional + +fun f( + // $TEST$ no error "Variadic parameters must not be optional." + »a«: Int, + // $TEST$ no error "Variadic parameters must not be optional." + »b«: Int = 1, + // $TEST$ no error "Variadic parameters must not be optional." + vararg »c«: Int, + // $TEST$ error "Variadic parameters must not be optional." + vararg »d«: Int = 2 +) From 38593c4bcf963eca677d27ad780d341db7a5facb Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Fri, 22 Sep 2023 15:07:57 +0200 Subject: [PATCH 8/9] test: load builtin library before each validation test --- tests/language/validation/testValidation.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/language/validation/testValidation.test.ts b/tests/language/validation/testValidation.test.ts index 5b19437d2..0b8175ed1 100644 --- a/tests/language/validation/testValidation.test.ts +++ b/tests/language/validation/testValidation.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, it } from 'vitest'; +import { afterEach, beforeEach, describe, it } from 'vitest'; import { createSafeDsServices } from '../../../src/language/safe-ds-module.js'; import { URI } from 'vscode-uri'; import { NodeFileSystem } from 'langium/node'; @@ -11,6 +11,11 @@ import { locationToString } from '../../helpers/location.js'; const services = createSafeDsServices(NodeFileSystem).SafeDs; describe('validation', async () => { + beforeEach(async () => { + // Load the builtin library + await services.shared.workspace.WorkspaceManager.initializeWorkspace([]); + }); + afterEach(async () => { await clearDocuments(services); }); From 5500f257b387d00dc1f226e33a1ad7e868a39769 Mon Sep 17 00:00:00 2001 From: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com> Date: Fri, 22 Sep 2023 13:27:06 +0000 Subject: [PATCH 9/9] style: apply automated linter fixes --- src/language/validation/other/types/unionTypes.ts | 2 +- src/language/validation/safe-ds-validator.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/language/validation/other/types/unionTypes.ts b/src/language/validation/other/types/unionTypes.ts index 677b31a6a..5056ee0d7 100644 --- a/src/language/validation/other/types/unionTypes.ts +++ b/src/language/validation/other/types/unionTypes.ts @@ -1,7 +1,7 @@ import { SdsUnionType } from '../../../generated/ast.js'; import { ValidationAcceptor } from 'langium'; import { typeArgumentsOrEmpty } from '../../../ast/shortcuts.js'; -import {isEmpty} from "radash"; +import { isEmpty } from 'radash'; export const CODE_UNION_TYPE_MISSING_TYPE_ARGUMENTS = 'union-type/missing-type-arguments'; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index 7109b13a7..f71886c6c 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -42,7 +42,7 @@ import { unionTypeMustHaveTypeArguments } from './other/types/unionTypes.js'; import { callableTypeMustNotHaveOptionalParameters } from './other/types/callableTypes.js'; import { typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/types/typeArgumentLists.js'; import { argumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/argumentLists.js'; -import {parameterMustNotBeVariadicAndOptional} from "./other/declarations/parameters.js"; +import { parameterMustNotBeVariadicAndOptional } from './other/declarations/parameters.js'; /** * Register custom validation checks.