From 9ba9d20b2f576237a32f9c3b647220577bd93191 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sat, 21 Oct 2023 15:07:49 +0200 Subject: [PATCH] feat: warning if literal types or union types have duplicate entries (#658) ### Summary of Changes Show a warning if a literal type or a union type has duplicate entries. --------- Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com> --- package.json | 2 +- src/language/typing/safe-ds-type-computer.ts | 3 ++ .../validation/other/types/literalTypes.ts | 28 ++++++++++++++-- .../validation/other/types/unionTypes.ts | 32 ++++++++++++++++--- src/language/validation/safe-ds-validator.ts | 10 ++++-- .../typing/types/type arguments/main.sdstest | 29 +++++++++++++++++ .../duplicate literals/empty list.sdstest | 7 ++++ .../duplicate literals/main.sdstest | 14 ++++++++ .../duplicate types/empty list.sdstest | 7 ++++ .../union types/duplicate types/main.sdstest | 14 ++++++++ .../must have type arguments/main.sdstest | 16 ---------- .../union types/must have types/main.sdstest | 16 ++++++++++ 12 files changed, 153 insertions(+), 25 deletions(-) create mode 100644 tests/resources/typing/types/type arguments/main.sdstest create mode 100644 tests/resources/validation/other/types/literal types/duplicate literals/empty list.sdstest create mode 100644 tests/resources/validation/other/types/literal types/duplicate literals/main.sdstest create mode 100644 tests/resources/validation/other/types/union types/duplicate types/empty list.sdstest create mode 100644 tests/resources/validation/other/types/union types/duplicate types/main.sdstest delete mode 100644 tests/resources/validation/other/types/union types/must have type arguments/main.sdstest create mode 100644 tests/resources/validation/other/types/union types/must have types/main.sdstest diff --git a/package.json b/package.json index 863774ccf..477079d07 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,7 @@ "configurationDefaults": { "[safe-ds]": { "editor.semanticHighlighting.enabled": true, - "editor.wordSeparators": "`~!@#$%^&*()-=+[]{}\\|;:'\",.<>/?»«", + "editor.wordSeparators": "`~!@#%^&*()-=+[]{}\\|;:'\",.<>/?»«", "files.trimTrailingWhitespace": true } } diff --git a/src/language/typing/safe-ds-type-computer.ts b/src/language/typing/safe-ds-type-computer.ts index 90b432b81..2e803e66d 100644 --- a/src/language/typing/safe-ds-type-computer.ts +++ b/src/language/typing/safe-ds-type-computer.ts @@ -51,6 +51,7 @@ import { isSdsSegment, isSdsTemplateString, isSdsType, + isSdsTypeArgument, isSdsTypeProjection, isSdsUnionType, isSdsYield, @@ -140,6 +141,8 @@ export class SafeDsTypeComputer { return this.computeTypeOfExpression(node); } else if (isSdsType(node)) { return this.computeTypeOfType(node); + } else if (isSdsTypeArgument(node)) { + return this.computeType(node.value); } else if (isSdsTypeProjection(node)) { return this.computeTypeOfType(node.type); } /* c8 ignore start */ else { diff --git a/src/language/validation/other/types/literalTypes.ts b/src/language/validation/other/types/literalTypes.ts index a30e400e4..f26db0154 100644 --- a/src/language/validation/other/types/literalTypes.ts +++ b/src/language/validation/other/types/literalTypes.ts @@ -2,17 +2,20 @@ import { isSdsList, isSdsMap, SdsLiteralType } from '../../../generated/ast.js'; import { ValidationAcceptor } from 'langium'; import { literalsOrEmpty } from '../../../helpers/nodeProperties.js'; import { isEmpty } from 'radash'; +import { SafeDsServices } from '../../../safe-ds-module.js'; +import { EvaluatedNode } from '../../../partialEvaluation/model.js'; -export const CODE_UNION_TYPE_MISSING_LITERALS = 'union-type/missing-literals'; +export const CODE_LITERAL_TYPE_DUPLICATE_LITERAL = 'literal-type/duplicate-literal'; export const CODE_LITERAL_TYPE_LIST_LITERAL = 'literal-type/list-literal'; export const CODE_LITERAL_TYPE_MAP_LITERAL = 'literal-type/map-literal'; +export const CODE_LITERAL_TYPE_MISSING_LITERALS = 'literal-type/missing-literals'; export const literalTypeMustHaveLiterals = (node: SdsLiteralType, accept: ValidationAcceptor): void => { if (isEmpty(literalsOrEmpty(node))) { accept('error', 'A literal type must have at least one literal.', { node, property: 'literalList', - code: CODE_UNION_TYPE_MISSING_LITERALS, + code: CODE_LITERAL_TYPE_MISSING_LITERALS, }); } }; @@ -38,3 +41,24 @@ export const literalTypeMustNotContainMapLiteral = (node: SdsLiteralType, accept } } }; + +export const literalTypeShouldNotHaveDuplicateLiteral = (services: SafeDsServices) => { + const partialEvaluator = services.evaluation.PartialEvaluator; + + return (node: SdsLiteralType, accept: ValidationAcceptor): void => { + const literals = literalsOrEmpty(node); + const constants: EvaluatedNode[] = []; + + for (const literal of literals) { + const constant = partialEvaluator.evaluate(literal); + if (constants.some((it) => it.equals(constant))) { + accept('warning', `The literal ${constant.toString()} was already listed.`, { + node: literal, + code: CODE_LITERAL_TYPE_DUPLICATE_LITERAL, + }); + } else { + constants.push(constant); + } + } + }; +}; diff --git a/src/language/validation/other/types/unionTypes.ts b/src/language/validation/other/types/unionTypes.ts index 7c2c3bd73..6249c24a1 100644 --- a/src/language/validation/other/types/unionTypes.ts +++ b/src/language/validation/other/types/unionTypes.ts @@ -2,15 +2,39 @@ import { SdsUnionType } from '../../../generated/ast.js'; import { ValidationAcceptor } from 'langium'; import { isEmpty } from 'radash'; import { typeArgumentsOrEmpty } from '../../../helpers/nodeProperties.js'; +import { SafeDsServices } from '../../../safe-ds-module.js'; +import { Type } from '../../../typing/model.js'; -export const CODE_UNION_TYPE_MISSING_TYPE_ARGUMENTS = 'union-type/missing-type-arguments'; +export const CODE_UNION_TYPE_DUPLICATE_TYPE = 'union-type/duplicate-type'; +export const CODE_UNION_TYPE_MISSING_TYPES = 'union-type/missing-types'; -export const unionTypeMustHaveTypeArguments = (node: SdsUnionType, accept: ValidationAcceptor): void => { +export const unionTypeMustHaveTypes = (node: SdsUnionType, accept: ValidationAcceptor): void => { if (isEmpty(typeArgumentsOrEmpty(node.typeArgumentList))) { - accept('error', 'A union type must have at least one type argument.', { + accept('error', 'A union type must have at least one type.', { node, property: 'typeArgumentList', - code: CODE_UNION_TYPE_MISSING_TYPE_ARGUMENTS, + code: CODE_UNION_TYPE_MISSING_TYPES, }); } }; + +export const unionTypeShouldNotHaveDuplicateTypes = (services: SafeDsServices) => { + const typeComputer = services.types.TypeComputer; + + return (node: SdsUnionType, accept: ValidationAcceptor): void => { + const typeArguments = typeArgumentsOrEmpty(node.typeArgumentList); + const knownTypes: Type[] = []; + + for (const typeArgument of typeArguments) { + const type = typeComputer.computeType(typeArgument); + if (knownTypes.some((it) => it.equals(type))) { + accept('warning', `The type '${type.toString()}' was already listed.`, { + node: typeArgument, + code: CODE_UNION_TYPE_DUPLICATE_TYPE, + }); + } else { + knownTypes.push(type); + } + } + }; +}; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index c752e28bb..15fd46a26 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -53,7 +53,7 @@ import { import { moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage } from './other/modules.js'; import { typeParameterConstraintLeftOperandMustBeOwnTypeParameter } from './other/declarations/typeParameterConstraints.js'; import { parameterListMustNotHaveRequiredParametersAfterOptionalParameters } from './other/declarations/parameterLists.js'; -import { unionTypeMustHaveTypeArguments } from './other/types/unionTypes.js'; +import { unionTypeMustHaveTypes, unionTypeShouldNotHaveDuplicateTypes } from './other/types/unionTypes.js'; import { callableTypeMustNotHaveOptionalParameters, callableTypeParameterMustNotHaveConstModifier, @@ -124,6 +124,7 @@ import { literalTypeMustHaveLiterals, literalTypeMustNotContainListLiteral, literalTypeMustNotContainMapLiteral, + literalTypeShouldNotHaveDuplicateLiteral, } from './other/types/literalTypes.js'; /** @@ -211,6 +212,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { literalTypeMustNotContainListLiteral, literalTypeMustNotContainMapLiteral, literalTypesShouldBeUsedWithCaution, + literalTypeShouldNotHaveDuplicateLiteral(services), ], SdsMap: [mapsShouldBeUsedWithCaution], SdsMemberAccess: [ @@ -261,7 +263,11 @@ export const registerValidationChecks = function (services: SafeDsServices) { SdsTemplateString: [templateStringMustHaveExpressionBetweenTwoStringParts], SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter], SdsTypeParameterList: [typeParameterListShouldNotBeEmpty], - SdsUnionType: [unionTypeMustHaveTypeArguments, unionTypeShouldNotHaveASingularTypeArgument], + SdsUnionType: [ + unionTypeMustHaveTypes, + unionTypeShouldNotHaveDuplicateTypes(services), + unionTypeShouldNotHaveASingularTypeArgument, + ], SdsYield: [yieldMustNotBeUsedInPipeline], }; registry.register(checks); diff --git a/tests/resources/typing/types/type arguments/main.sdstest b/tests/resources/typing/types/type arguments/main.sdstest new file mode 100644 index 000000000..61b3ddefa --- /dev/null +++ b/tests/resources/typing/types/type arguments/main.sdstest @@ -0,0 +1,29 @@ +package tests.typing.types.typeArguments + +class MyClass + +enum MyEnum { + MyEnumVariant +} + +fun myFunction( + // $TEST$ serialization Int + a: MyClass<»Int«>, + // $TEST$ serialization Int + b: MyClass<»T = Int«>, + + // $TEST$ serialization String + c: MyEnum.MyEnumVariant<»T = String«>, + // $TEST$ serialization String + d: MyEnum.MyEnumVariant<»T = String«>, + + // $TEST$ serialization Boolean + e: unresolved<»Boolean«>, + // $TEST$ serialization Boolean + f: unresolved<»T = Boolean«>, + + // $TEST$ serialization $Unknown + g: MyClass<»unresolved«>, + // $TEST$ serialization $Unknown + h: MyClass<»T = unresolved«>, +) diff --git a/tests/resources/validation/other/types/literal types/duplicate literals/empty list.sdstest b/tests/resources/validation/other/types/literal types/duplicate literals/empty list.sdstest new file mode 100644 index 000000000..ad8a02370 --- /dev/null +++ b/tests/resources/validation/other/types/literal types/duplicate literals/empty list.sdstest @@ -0,0 +1,7 @@ +package tests.validation.other.types.literalTypes.duplicateLiterals + +// $TEST$ no warning r"The literal .* was already listed." + +segment mySegment1( + p: literal<> +) {} diff --git a/tests/resources/validation/other/types/literal types/duplicate literals/main.sdstest b/tests/resources/validation/other/types/literal types/duplicate literals/main.sdstest new file mode 100644 index 000000000..7ecff256c --- /dev/null +++ b/tests/resources/validation/other/types/literal types/duplicate literals/main.sdstest @@ -0,0 +1,14 @@ +package tests.validation.other.types.literalTypes.duplicateLiterals + +segment mySegment( + // $TEST$ no warning r"The literal .* was already listed." + p: literal<»1«>, + q: literal< + // $TEST$ no warning r"The literal .* was already listed." + »1«, + // $TEST$ no warning r"The literal .* was already listed." + »2«, + // $TEST$ warning r"The literal 1 was already listed." + »1«, + >, +) {} diff --git a/tests/resources/validation/other/types/union types/duplicate types/empty list.sdstest b/tests/resources/validation/other/types/union types/duplicate types/empty list.sdstest new file mode 100644 index 000000000..59bd07f44 --- /dev/null +++ b/tests/resources/validation/other/types/union types/duplicate types/empty list.sdstest @@ -0,0 +1,7 @@ +package tests.validation.other.types.unionTypes.duplicateTypes + +// $TEST$ no warning r"The type .* was already listed." + +segment mySegment1( + p: union<> +) {} diff --git a/tests/resources/validation/other/types/union types/duplicate types/main.sdstest b/tests/resources/validation/other/types/union types/duplicate types/main.sdstest new file mode 100644 index 000000000..f0d7f0ef7 --- /dev/null +++ b/tests/resources/validation/other/types/union types/duplicate types/main.sdstest @@ -0,0 +1,14 @@ +package tests.validation.other.types.unionTypes.duplicateTypes + +segment mySegment( + // $TEST$ no warning r"The type .* was already listed." + p: union<»Int«>, + q: union< + // $TEST$ no warning r"The type .* was already listed." + »Int«, + // $TEST$ no warning r"The type .* was already listed." + »String«, + // $TEST$ warning r"The type 'Int' was already listed." + »Int«, + >, +) {} 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 deleted file mode 100644 index 17c8df750..000000000 --- a/tests/resources/validation/other/types/union types/must have type arguments/main.sdstest +++ /dev/null @@ -1,16 +0,0 @@ -package tests.validation.other.types.unionTypes.mustHaveTypeArguments - -// $TEST$ error "A union type must have at least one type argument." -segment mySegment1( - p: union»<>« -) {} - -// $TEST$ no error "A union type must have at least one type argument." -segment mySegment2( - p: union»« -) {} - -// $TEST$ no error "A union type must have at least one type argument." -segment mySegment3( - p: union»« -) {} diff --git a/tests/resources/validation/other/types/union types/must have types/main.sdstest b/tests/resources/validation/other/types/union types/must have types/main.sdstest new file mode 100644 index 000000000..54a5629ea --- /dev/null +++ b/tests/resources/validation/other/types/union types/must have types/main.sdstest @@ -0,0 +1,16 @@ +package tests.validation.other.types.unionTypes.mustHaveTypes + +// $TEST$ error "A union type must have at least one type." +segment mySegment1( + p: union»<>« +) {} + +// $TEST$ no error "A union type must have at least one type." +segment mySegment2( + p: union»« +) {} + +// $TEST$ no error "A union type must have at least one type." +segment mySegment3( + p: union»« +) {}