feat: check context of union types (#677)

Closes #675 ### Summary of Changes Union types are now only allowed as parameters of annotations, classes, and functions. This way, no values with a union type appear inside pipelines or segments.
Safe-DS · Oct 22, 2023 · e846b59 · e846b59
1 parent 4656c25
commit e846b59
Show file tree

Hide file tree

Showing 12 changed files with 181 additions and 22 deletions.
diff --git a/src/language/validation/experimentalLanguageFeatures.ts b/src/language/validation/experimentalLanguageFeatures.ts
@@ -1,9 +1,21 @@
-import { SdsIndexedAccess, SdsLiteralType, SdsMap, SdsUnionType } from '../generated/ast.js';
-import { ValidationAcceptor } from 'langium';
+import {
+    isSdsIndexedAccess,
+    isSdsMap,
+    isSdsUnionType,
+    SdsIndexedAccess,
+    SdsLiteralType,
+    SdsMap,
+    SdsUnionType,
+} from '../generated/ast.js';
+import { hasContainerOfType, ValidationAcceptor } from 'langium';
 
 export const CODE_EXPERIMENTAL_LANGUAGE_FEATURE = 'experimental/language-feature';
 
 export const indexedAccessesShouldBeUsedWithCaution = (node: SdsIndexedAccess, accept: ValidationAcceptor): void => {
+    if (hasContainerOfType(node.$container, isSdsIndexedAccess)) {
+        return;
+    }
+
     accept('warning', 'Indexed accesses are experimental and may change without prior notice.', {
         node,
         code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
@@ -18,13 +30,21 @@ export const literalTypesShouldBeUsedWithCaution = (node: SdsLiteralType, accept
 };
 
 export const mapsShouldBeUsedWithCaution = (node: SdsMap, accept: ValidationAcceptor): void => {
+    if (hasContainerOfType(node.$container, isSdsMap)) {
+        return;
+    }
+
     accept('warning', 'Map literals are experimental and may change without prior notice.', {
         node,
         code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,
     });
 };
 
 export const unionTypesShouldBeUsedWithCaution = (node: SdsUnionType, accept: ValidationAcceptor): void => {
+    if (hasContainerOfType(node.$container, isSdsUnionType)) {
+        return;
+    }
+
     accept('warning', 'Union types are experimental and may change without prior notice.', {
         node,
         code: CODE_EXPERIMENTAL_LANGUAGE_FEATURE,

diff --git a/src/language/validation/names.ts b/src/language/validation/names.ts
@@ -17,19 +17,19 @@ import {
 } from '../generated/ast.js';
 import { getDocument, ValidationAcceptor } from 'langium';
 import {
-    streamBlockLambdaResults,
-    getMatchingClassMembers,
     getColumns,
     getEnumVariants,
     getImportedDeclarations,
     getImports,
-    isStatic,
+    getMatchingClassMembers,
     getModuleMembers,
     getPackageName,
     getParameters,
-    streamPlaceholders,
     getResults,
     getTypeParameters,
+    isStatic,
+    streamBlockLambdaResults,
+    streamPlaceholders,
 } from '../helpers/nodeProperties.js';
 import { duplicatesBy } from '../../helpers/collectionUtils.js';
 import { isInPipelineFile, isInStubFile, isInTestFile } from '../helpers/fileExtensions.js';
@@ -217,16 +217,20 @@ export const moduleMemberMustHaveNameThatIsUniqueInPackage = (services: SafeDsSe
     const builtinUris = new Set(listBuiltinFiles().map((it) => it.toString()));
 
     return (node: SdsModule, accept: ValidationAcceptor): void => {
+        const moduleUri = getDocument(node).uri?.toString();
+        if (builtinUris.has(moduleUri)) {
+            return;
+        }
+
         for (const member of getModuleMembers(node)) {
             const packageName = getPackageName(member) ?? '';
             const declarationsInPackage = packageManager.getDeclarationsInPackage(packageName);
-            const memberUri = getDocument(member).uri?.toString();
 
             if (
                 declarationsInPackage.some(
                     (it) =>
                         it.name === member.name &&
-                        it.documentUri.toString() !== memberUri &&
+                        it.documentUri.toString() !== moduleUri &&
                         !builtinUris.has(it.documentUri.toString()),
                 )
             ) {

diff --git a/src/language/validation/other/types/unionTypes.ts b/src/language/validation/other/types/unionTypes.ts
@@ -1,13 +1,50 @@
-import { SdsUnionType } from '../../../generated/ast.js';
-import { ValidationAcceptor } from 'langium';
+import {
+    isSdsAnnotation,
+    isSdsCallable,
+    isSdsClass,
+    isSdsFunction,
+    isSdsParameter,
+    isSdsUnionType,
+    SdsUnionType,
+} from '../../../generated/ast.js';
+import { getContainerOfType, hasContainerOfType, ValidationAcceptor } from 'langium';
 import { getTypeArguments } from '../../../helpers/nodeProperties.js';
 import { SafeDsServices } from '../../../safe-ds-module.js';
 import { Type } from '../../../typing/model.js';
 import { isEmpty } from '../../../../helpers/collectionUtils.js';
 
+export const CODE_UNION_TYPE_CONTEXT = 'union-type/context';
 export const CODE_UNION_TYPE_DUPLICATE_TYPE = 'union-type/duplicate-type';
 export const CODE_UNION_TYPE_MISSING_TYPES = 'union-type/missing-types';
 
+export const unionTypeMustBeUsedInCorrectContext = (node: SdsUnionType, accept: ValidationAcceptor): void => {
+    if (!contextIsCorrect(node)) {
+        accept('error', 'Union types must only be used for parameters of annotations, classes, and functions.', {
+            node,
+            code: CODE_UNION_TYPE_CONTEXT,
+        });
+    }
+};
+
+const contextIsCorrect = (node: SdsUnionType): boolean => {
+    if (hasContainerOfType(node.$container, isSdsUnionType)) {
+        return true;
+    }
+
+    const container = node.$container;
+    if (!isSdsParameter(container)) {
+        return false;
+    }
+
+    const containingCallable = getContainerOfType(container, isSdsCallable);
+    return (
+        !containingCallable ||
+        isSdsAnnotation(containingCallable) ||
+        isSdsClass(containingCallable) ||
+        isSdsFunction(containingCallable)
+    );
+};
+
 export const unionTypeMustHaveTypes = (node: SdsUnionType, accept: ValidationAcceptor): void => {
     if (isEmpty(getTypeArguments(node.typeArgumentList))) {
         accept('error', 'A union type must have at least one type.', {

diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts
@@ -57,7 +57,11 @@ import {
 } from './other/modules.js';
 import { typeParameterConstraintLeftOperandMustBeOwnTypeParameter } from './other/declarations/typeParameterConstraints.js';
 import { parameterListMustNotHaveRequiredParametersAfterOptionalParameters } from './other/declarations/parameterLists.js';
-import { unionTypeMustHaveTypes, unionTypeShouldNotHaveDuplicateTypes } from './other/types/unionTypes.js';
+import {
+    unionTypeMustBeUsedInCorrectContext,
+    unionTypeMustHaveTypes,
+    unionTypeShouldNotHaveDuplicateTypes,
+} from './other/types/unionTypes.js';
 import {
     callableTypeMustNotHaveOptionalParameters,
     callableTypeParameterMustNotHaveConstModifier,
@@ -273,6 +277,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
         SdsTypeParameterConstraint: [typeParameterConstraintLeftOperandMustBeOwnTypeParameter],
         SdsTypeParameterList: [typeParameterListShouldNotBeEmpty],
         SdsUnionType: [
+            unionTypeMustBeUsedInCorrectContext,
             unionTypeMustHaveTypes,
             unionTypesShouldBeUsedWithCaution,
             unionTypeShouldNotHaveDuplicateTypes(services),

diff --git a/tests/resources/validation/experimental language feature/indexed access/main.sdstest b/tests/resources/validation/experimental language feature/indexed access/main.sdstest
@@ -6,4 +6,10 @@ pipeline myPipeline {
 
     // $TEST$ warning "Indexed accesses are experimental and may change without prior notice."
     »{"a": "b"}["a"]«;
+
+    // $TEST$ no warning "Indexed accesses are experimental and may change without prior notice."
+    [1, 2][»[1, 2][1]«];
+
+    // $TEST$ no warning "Indexed accesses are experimental and may change without prior notice."
+    {"a": "b"}[»{"a": "b"}["a"]«];
 }
diff --git a/tests/resources/validation/experimental language feature/maps/main.sdstest b/tests/resources/validation/experimental language feature/maps/main.sdstest
@@ -3,4 +3,7 @@ package tests.validation.experimentalLanguageFeature.maps
 pipeline myPipeline {
     // $TEST$ warning "Map literals are experimental and may change without prior notice."
     »{"a": "b"}«;
+
+    // $TEST$ no warning "Map literals are experimental and may change without prior notice."
+    {"a": »{}«};
 }
diff --git a/tests/resources/validation/experimental language feature/union types/main.sdstest b/tests/resources/validation/experimental language feature/union types/main.sdstest
@@ -2,5 +2,12 @@ package tests.validation.experimentalLanguageFeature.unionTypes
 
 fun myFunction(
     // $TEST$ warning "Union types are experimental and may change without prior notice."
-    p: »union<>«
+    p: »union<Int, Float>«,
+
+    // $TEST$ no warning "Union types are experimental and may change without prior notice."
+    q: union<»union<Int, Float>«, Int>,
+
+    // $TEST$ no warning "Union types are experimental and may change without prior notice."
+    // $TEST$ no warning "Union types are experimental and may change without prior notice."
+    r: union<(p: »union<Int, Float>«) -> (r: »union<Int, Float>«), Int>,
 )
diff --git a/tests/resources/validation/other/types/union types/context/main.sdstest b/tests/resources/validation/other/types/union types/context/main.sdstest
@@ -0,0 +1,61 @@
+package tests.validation.other.types.unionTypes.context
+
+annotation MyAnnotation(
+    // $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
+    p: »union<Int>«,
+)
+
+class MyClass<T>(
+    // $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
+    p: »union<Int>«,
+) {
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    attr a: »union<Int>«
+}
+
+enum MyEnum {
+    MyEnumVariant<T>(
+        // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+        p: »union<Int>«,
+    )
+}
+
+fun myFunction(
+    // $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
+    p: »union<Int>«,
+) -> (
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    r: »union<Int>«,
+)
+
+segment mySegment1(
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    p: »union<Int>«,
+) -> (
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    r: »union<Int>«,
+) {}
+
+segment mySegment2(
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    c: (p: »union<Int>«) -> (r: »union<Int>«),
+) {
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    (
+        p: »union<Int>«,
+    ) {};
+
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    (
+        p: »union<Int>«,
+    ) -> 1;
+}
+
+segment mySegment3(
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    p1: MyClass<»union<Int>«>,
+
+    // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions."
+    p2: MyEnum.MyEnumVariant<»union<Int>«>,
+) {}
diff --git a/tests/resources/validation/other/types/union types/context/nested.sdstest b/tests/resources/validation/other/types/union types/context/nested.sdstest
@@ -0,0 +1,16 @@
+package tests.validation.other.types.unionTypes.context
+
+/*
+ * We already show an error for the outer union type, if it's used in the wrong context.
+ */
+
+class MyClass1 {
+    // $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
+    attr a: union<Int, »union<Int>«>
+}
+
+class MyClass2 {
+    // $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
+    // $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions."
+    attr a: union<Int, (p: »union<Int>«) -> (r: »union<Int>«)>
+}
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
@@ -2,6 +2,6 @@ package tests.validation.other.types.unionTypes.duplicateTypes
 
 // $TEST$ no warning r"The type .* was already listed."
 
-segment mySegment1(
+fun myFunction1(
     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
@@ -1,6 +1,6 @@
 package tests.validation.other.types.unionTypes.duplicateTypes
 
-segment mySegment(
+fun myFunction2(
     // $TEST$ no warning r"The type .* was already listed."
     p: union<»Int«>,
     q: union<
@@ -11,4 +11,4 @@ segment mySegment(
         // $TEST$ warning r"The type 'Int' was already listed."
         »Int«,
     >,
-) {}
+)
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
@@ -1,16 +1,16 @@
 package tests.validation.other.types.unionTypes.mustHaveTypes
 
 // $TEST$ error "A union type must have at least one type."
-segment mySegment1(
+fun myFunction1(
     p: union»<>«
-) {}
+)
 
 // $TEST$ no error "A union type must have at least one type."
-segment mySegment2(
+fun myFunction2(
     p: union»<Int>«
-) {}
+)
 
 // $TEST$ no error "A union type must have at least one type."
-segment mySegment3(
+fun myFunction3(
     p: union»<Int, Float>«
-) {}
+)