Skip to content

Commit

Permalink
feat: validation for type arguments of named types (#632)
Browse files Browse the repository at this point in the history
Closes partially #543

### Summary of Changes

Show an error if
* the entire type argument list is missing
* a type argument is missing
* a type parameter is set multiple times
* too many type arguments are given.

---------

Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com>
  • Loading branch information
lars-reimann and megalinter-bot authored Oct 11, 2023
1 parent 17a5b7a commit b72768c
Show file tree
Hide file tree
Showing 11 changed files with 334 additions and 26 deletions.
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

0 comments on commit b72768c

Please sign in to comment.