Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: validation for type arguments of named types #632

Merged
merged 5 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/language/helpers/stringUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* Based on the given count, returns the singular or plural form of the given word.
*/
export const pluralize = (count: number, singular: string, plural: string = `${singular}s`): string => {
return count === 1 ? singular : plural;
};
69 changes: 69 additions & 0 deletions src/language/validation/other/types/namedTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { SdsNamedType } from '../../../generated/ast.js';
import { ValidationAcceptor } from 'langium';
import { SafeDsServices } from '../../../safe-ds-module.js';
import { typeArgumentsOrEmpty, typeParametersOrEmpty } from '../../../helpers/nodeProperties.js';
import { duplicatesBy } from '../../../helpers/collectionUtils.js';
import { pluralize } from '../../../helpers/stringUtils.js';

export const CODE_NAMED_TYPE_DUPLICATE_TYPE_PARAMETER = 'named-type/duplicate-type-parameter';
export const CODE_NAMED_TYPE_POSITIONAL_AFTER_NAMED = 'named-type/positional-after-named';
export const CODE_NAMED_TYPE_TOO_MANY_TYPE_ARGUMENTS = 'named-type/too-many-type-arguments';

export const namedTypeMustNotSetTypeParameterMultipleTimes = (services: SafeDsServices) => {
const nodeMapper = services.helpers.NodeMapper;
const typeArgumentToTypeParameterOrUndefined = nodeMapper.typeArgumentToTypeParameterOrUndefined.bind(nodeMapper);

return (node: SdsNamedType, accept: ValidationAcceptor): void => {
const typeArguments = typeArgumentsOrEmpty(node.typeArgumentList);
const duplicates = duplicatesBy(typeArguments, typeArgumentToTypeParameterOrUndefined);

for (const duplicate of duplicates) {
const correspondingTypeParameter = typeArgumentToTypeParameterOrUndefined(duplicate)!;
accept('error', `The type parameter '${correspondingTypeParameter.name}' is already set.`, {
node: duplicate,
code: CODE_NAMED_TYPE_DUPLICATE_TYPE_PARAMETER,
});
}
};
};

export const namedTypeTypeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments = (
node: SdsNamedType,
accept: ValidationAcceptor,
): void => {
const typeArgumentList = node.typeArgumentList;
if (!typeArgumentList) {
return;
}

let foundNamed = false;
for (const typeArgument of typeArgumentList.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_NAMED_TYPE_POSITIONAL_AFTER_NAMED,
});
}
}
};

export const namedTypeMustNotHaveTooManyTypeArguments = (node: SdsNamedType, accept: ValidationAcceptor): void => {
// If the declaration is unresolved, we already show another error
if (!node.declaration.ref) {
return;
}

const typeParameters = typeParametersOrEmpty(node.declaration.ref);
const typeArguments = typeArgumentsOrEmpty(node.typeArgumentList);

if (typeArguments.length > typeParameters.length) {
const kind = pluralize(typeParameters.length, 'type argument');
accept('error', `Expected ${typeParameters.length} ${kind} but got ${typeArguments.length}.`, {
node,
property: 'typeArgumentList',
code: CODE_NAMED_TYPE_TOO_MANY_TYPE_ARGUMENTS,
});
}
};
21 changes: 0 additions & 21 deletions src/language/validation/other/types/typeArgumentLists.ts

This file was deleted.

18 changes: 15 additions & 3 deletions src/language/validation/safe-ds-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ import {
} from './style.js';
import { templateStringMustHaveExpressionBetweenTwoStringParts } from './other/expressions/templateStrings.js';
import { assignmentAssigneeMustGetValue, yieldMustNotBeUsedInPipeline } from './other/statements/assignments.js';
import { attributeMustHaveTypeHint, parameterMustHaveTypeHint, resultMustHaveTypeHint } from './types.js';
import {
attributeMustHaveTypeHint,
namedTypeMustSetAllTypeParameters,
parameterMustHaveTypeHint,
resultMustHaveTypeHint,
} from './types.js';
import { moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage } from './other/modules.js';
import { typeParameterConstraintLeftOperandMustBeOwnTypeParameter } from './other/declarations/typeParameterConstraints.js';
import { parameterListMustNotHaveRequiredParametersAfterOptionalParameters } from './other/declarations/parameterLists.js';
Expand All @@ -43,7 +48,6 @@ import {
callableTypeMustNotHaveOptionalParameters,
callableTypeParameterMustNotHaveConstModifier,
} from './other/types/callableTypes.js';
import { typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/types/typeArgumentLists.js';
import { argumentListMustNotHavePositionalArgumentsAfterNamedArguments } from './other/argumentLists.js';
import {
referenceMustNotBeFunctionPointer,
Expand Down Expand Up @@ -77,6 +81,11 @@ import {
import { memberAccessMustBeNullSafeIfReceiverIsNullable } from './other/expressions/memberAccesses.js';
import { importPackageMustExist, importPackageShouldNotBeEmpty } from './other/imports.js';
import { singleUseAnnotationsMustNotBeRepeated } from './builtins/repeatable.js';
import {
namedTypeMustNotHaveTooManyTypeArguments,
namedTypeMustNotSetTypeParameterMultipleTimes,
namedTypeTypeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments,
} from './other/types/namedTypes.js';

/**
* Register custom validation checks.
Expand Down Expand Up @@ -135,7 +144,11 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsNamedType: [
namedTypeDeclarationShouldNotBeDeprecated(services),
namedTypeDeclarationShouldNotBeExperimental(services),
namedTypeMustNotHaveTooManyTypeArguments,
namedTypeMustNotSetTypeParameterMultipleTimes(services),
namedTypeMustSetAllTypeParameters(services),
namedTypeTypeArgumentListShouldBeNeeded,
namedTypeTypeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments,
],
SdsParameter: [
parameterMustHaveTypeHint,
Expand All @@ -159,7 +172,6 @@ export const registerValidationChecks = function (services: SafeDsServices) {
segmentResultListShouldNotBeEmpty,
],
SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts],
SdsTypeArgumentList: [typeArgumentListMustNotHavePositionalArgumentsAfterNamedArguments],
SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter],
SdsTypeParameterList: [typeParameterListShouldNotBeEmpty],
SdsUnionType: [unionTypeMustHaveTypeArguments, unionTypeShouldNotHaveASingularTypeArgument],
Expand Down
47 changes: 46 additions & 1 deletion src/language/validation/types.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,53 @@
import { getContainerOfType, ValidationAcceptor } from 'langium';
import { isSdsCallable, isSdsLambda, SdsAttribute, SdsParameter, SdsResult } from '../generated/ast.js';
import { isSdsCallable, isSdsLambda, SdsAttribute, SdsNamedType, SdsParameter, SdsResult } from '../generated/ast.js';
import { typeArgumentsOrEmpty, typeParametersOrEmpty } from '../helpers/nodeProperties.js';
import { isEmpty } from 'radash';
import { SafeDsServices } from '../safe-ds-module.js';
import { pluralize } from '../helpers/stringUtils.js';

export const CODE_TYPE_MISSING_TYPE_ARGUMENTS = 'type/missing-type-arguments';
export const CODE_TYPE_MISSING_TYPE_HINT = 'type/missing-type-hint';

// -----------------------------------------------------------------------------
// Missing type arguments
// -----------------------------------------------------------------------------

export const namedTypeMustSetAllTypeParameters =
(services: SafeDsServices) =>
(node: SdsNamedType, accept: ValidationAcceptor): void => {
const expectedTypeParameters = typeParametersOrEmpty(node.declaration.ref);
if (isEmpty(expectedTypeParameters)) {
return;
}

if (node.typeArgumentList) {
const actualTypeParameters = typeArgumentsOrEmpty(node.typeArgumentList).map((it) =>
services.helpers.NodeMapper.typeArgumentToTypeParameterOrUndefined(it),
);

const missingTypeParameters = expectedTypeParameters.filter((it) => !actualTypeParameters.includes(it));
if (!isEmpty(missingTypeParameters)) {
const kind = pluralize(missingTypeParameters.length, 'type parameter');
const missingTypeParametersString = missingTypeParameters.map((it) => `'${it.name}'`).join(', ');

accept('error', `The ${kind} ${missingTypeParametersString} must be set here.`, {
node,
property: 'typeArgumentList',
code: CODE_TYPE_MISSING_TYPE_ARGUMENTS,
});
}
} else {
accept(
'error',
`The type '${node.declaration.$refText}' is parameterized, so a type argument list must be added.`,
{
node,
code: CODE_TYPE_MISSING_TYPE_ARGUMENTS,
},
);
}
};

// -----------------------------------------------------------------------------
// Missing type hints
// -----------------------------------------------------------------------------
Expand Down
42 changes: 42 additions & 0 deletions tests/language/helpers/stringUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { describe, expect, it } from 'vitest';
import { pluralize } from '../../../src/language/helpers/stringUtils.js';

describe('pluralize', () => {
it.each([
{
count: 0,
singular: 'apple',
plural: 'apples',
expected: 'apples',
},
{
count: 1,
singular: 'apple',
plural: 'apple',
expected: 'apple',
},
{
count: 2,
singular: 'apple',
plural: 'apples',
expected: 'apples',
},
{
count: 0,
singular: 'apple',
expected: 'apples',
},
{
count: 1,
singular: 'apple',
expected: 'apple',
},
{
count: 2,
singular: 'apple',
expected: 'apples',
},
])('should return the singular or plural form based on the count', ({ count, singular, plural, expected }) => {
expect(pluralize(count, singular, plural)).toBe(expected);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ segment mySegment() -> (
// $TEST$ error "No value is assigned to this assignee."
»val k«, »val l« = unresolved();


// $TEST$ error "No value is assigned to this assignee."
»yield r1« = noResults();
// $TEST$ no error "No value is assigned to this assignee."
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package tests.validation.other.types.typeArgumentLists.duplicateTypeParameters

class MyClass<A, B>

fun myFunction(
f: MyClass<
// $TEST$ no error r"The type parameter '\w+' is already set\."
»Int«,
// $TEST$ error "The type parameter 'A' is already set."
»A = Int«
>,
g: MyClass<
// $TEST$ no error r"The type parameter '\w+' is already set\."
»B = Int«,
// $TEST$ error "The type parameter 'B' is already set."
»B = Int«
>,
h: MyClass<
// $TEST$ no error r"The type parameter '\w+' is already set\."
»A = Int«,
// $TEST$ no error r"The type parameter '\w+' is already set\."
»B = Int«
>,
i: MyClass<
// $TEST$ no error r"The type parameter '\w+' is already set\."
»Int«,
// $TEST$ no error r"The type parameter '\w+' is already set\."
»Int«
>,
j: MyClass<
// $TEST$ no error r"The type parameter '\w+' is already set\."
»Unresolved = Int«,
// $TEST$ no error r"The type parameter '\w+' is already set\."
»Unresolved = Int«
>
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package tests.validation.other.types.typeArgumentLists.tooManyTypeArguments

class MyClass1<T>
class MyClass2<A, B>

fun myFunction(
// $TEST$ no error r"Expected \d* type arguments? but got \d*\."
f: MyClass1»<>«,
// $TEST$ no error r"Expected \d* type arguments? but got \d*\."
g: MyClass1»<Int>«,
// $TEST$ error "Expected 1 type argument but got 2."
h: MyClass1»<Int, Int>«,
// $TEST$ error "Expected 2 type arguments but got 3."
i: MyClass2»<Int, Int, Int>«,
// $TEST$ no error r"Expected \d* type arguments? but got \d*\."
j: Unresolved»<Int, Int>«
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package tests.validation.types.namedTypes.missingTypeArgumentList

class MyClassWithoutTypeParameters
class MyClassWithTypeParameters<T>

enum MyEnum {
MyEnumVariantWithoutTypeParameters
MyEnumVariantWithTypeParameters<T>
}

fun myFunction(
// $TEST$ no error r"The type '\w*' is parameterized, so a type argument list must be added\."
a1: »MyClassWithoutTypeParameters«,
// $TEST$ no error r"The type '\w*' is parameterized, so a type argument list must be added\."
b1: »MyClassWithoutTypeParameters«<>,

// $TEST$ error "The type 'MyClassWithTypeParameters' is parameterized, so a type argument list must be added."
c1: »MyClassWithTypeParameters«,
// $TEST$ no error r"The type '\w*' is parameterized, so a type argument list must be added\."
d1: »MyClassWithTypeParameters«<>,


// $TEST$ no error r"The type '\w*' is parameterized, so a type argument list must be added\."
a2: MyEnum.»MyEnumVariantWithoutTypeParameters«,
// $TEST$ no error r"The type '\w*' is parameterized, so a type argument list must be added\."
b2: MyEnum.»MyEnumVariantWithoutTypeParameters«<>,

// $TEST$ error "The type 'MyEnumVariantWithTypeParameters' is parameterized, so a type argument list must be added."
c2: MyEnum.»MyEnumVariantWithTypeParameters«,
// $TEST$ no error r"The type '\w*' is parameterized, so a type argument list must be added\."
d2: MyEnum.»MyEnumVariantWithTypeParameters«<>,


// $TEST$ no error r"The type '\w*' is parameterized, so a type argument list must be added\."
e: »UnresolvedClass«,
// $TEST$ no error r"The type '\w*' is parameterized, so a type argument list must be added\."
f: »UnresolvedClass«<>,
)
Loading
Loading