Skip to content

Commit

Permalink
feat: port additional validation checks to `Langium (#576)
Browse files Browse the repository at this point in the history
Closes partially #543

### Summary of Changes

Show an error if:
* A union type has no type arguments
* A callable type has optional parameters
* Positional type arguments occur after named type arguments
* Positional arguments occur after named arguments
* A parameter is variadic and optional

---------

Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com>
  • Loading branch information
lars-reimann and megalinter-bot authored Sep 22, 2023
1 parent 47ce782 commit 8f5d57a
Show file tree
Hide file tree
Showing 22 changed files with 297 additions and 27 deletions.
20 changes: 10 additions & 10 deletions src/language/grammar/safe-ds.langium
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
)
)
Expand Down
21 changes: 21 additions & 0 deletions src/language/validation/other/argumentLists.ts
Original file line number Diff line number Diff line change
@@ -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,
});
}
}
};
4 changes: 2 additions & 2 deletions src/language/validation/other/declarations/parameterLists.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
14 changes: 14 additions & 0 deletions src/language/validation/other/declarations/parameters.ts
Original file line number Diff line number Diff line change
@@ -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,
});
}
};
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
17 changes: 17 additions & 0 deletions src/language/validation/other/types/callableTypes.ts
Original file line number Diff line number Diff line change
@@ -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,
});
}
}
};
21 changes: 21 additions & 0 deletions src/language/validation/other/types/typeArgumentLists.ts
Original file line number Diff line number Diff line change
@@ -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,
});
}
}
};
16 changes: 16 additions & 0 deletions src/language/validation/other/types/unionTypes.ts
Original file line number Diff line number Diff line change
@@ -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,
});
}
};
15 changes: 11 additions & 4 deletions src/language/validation/safe-ds-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +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.
Expand All @@ -48,9 +53,10 @@ export const registerValidationChecks = function (services: SafeDsServices) {
const checks: ValidationChecks<SafeDsAstType> = {
SdsAssignment: [assignmentShouldHaveMoreThanWildcardsAsAssignees],
SdsAnnotation: [annotationMustContainUniqueNames, annotationParameterListShouldNotBeEmpty],
SdsArgumentList: [argumentListMustNotHavePositionalArgumentsAfterNamedArguments],
SdsAttribute: [attributeMustHaveTypeHint],
SdsBlockLambda: [blockLambdaMustContainUniqueNames],
SdsCallableType: [callableTypeMustContainUniqueNames],
SdsCallableType: [callableTypeMustContainUniqueNames, callableTypeMustNotHaveOptionalParameters],
SdsClass: [classMustContainUniqueNames],
SdsClassBody: [classBodyShouldNotBeEmpty],
SdsConstraintList: [constraintListShouldNotBeEmpty],
Expand All @@ -62,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,
Expand All @@ -72,9 +78,10 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsResult: [resultMustHaveTypeHint],
SdsSegment: [segmentMustContainUniqueNames, segmentResultListShouldNotBeEmpty],
SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts],
SdsTypeArgumentList: [typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments],
SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter],
SdsTypeParameterList: [typeParameterListShouldNotBeEmpty],
SdsUnionType: [unionTypeShouldNotHaveASingularTypeArgument],
SdsUnionType: [unionTypeMustHaveTypeArguments, unionTypeShouldNotHaveASingularTypeArgument],
SdsYield: [yieldMustNotBeUsedInPipeline],
};
registry.register(checks, validator);
Expand Down
3 changes: 3 additions & 0 deletions src/language/validation/value.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// TODO: implement value-related checks here, including:
// - division by zero
// - must be constant
7 changes: 6 additions & 1 deletion tests/language/validation/testValidation.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// $TEST$ no_syntax_error

pipeline myPipeline {
f<»T = Int«>();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// $TEST$ no_syntax_error

pipeline myPipeline {
f<»Int«>();
}
Original file line number Diff line number Diff line change
@@ -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«);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand All @@ -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."
Expand All @@ -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."
Expand All @@ -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."
Expand All @@ -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."
Expand All @@ -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."
Expand All @@ -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."
Expand All @@ -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."
Expand Down
Original file line number Diff line number Diff line change
@@ -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
)
Original file line number Diff line number Diff line change
@@ -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«,
) -> ()
)
Original file line number Diff line number Diff line change
@@ -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,
) -> ()
)
Original file line number Diff line number Diff line change
@@ -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,
) -> ()
)
Loading

0 comments on commit 8f5d57a

Please sign in to comment.