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>
Safe-DS · Oct 21, 2023 · 9ba9d20 · 9ba9d20
1 parent 1775705
commit 9ba9d20
Show file tree

Hide file tree

Showing 12 changed files with 153 additions and 25 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,29 @@
+package tests.typing.types.typeArguments
+
+class MyClass<T>
+
+enum MyEnum {
+    MyEnumVariant<T>
+}
+
+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
@@ -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
@@ -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
@@ -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
@@ -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
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
@@ -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»<Int>«
+) {}
+
+// $TEST$ no error "A union type must have at least one type."
+segment mySegment3(
+    p: union»<Int, Float>«
+) {}