From b0b44a873cb40a25ee3b0f8d6eb6dc45e65b37d9 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sun, 22 Oct 2023 21:42:25 +0200 Subject: [PATCH] feat: check context of union types --- .../experimentalLanguageFeatures.ts | 24 +++++++- .../validation/other/types/unionTypes.ts | 41 ++++++++++++- src/language/validation/safe-ds-validator.ts | 7 ++- .../indexed access/main.sdstest | 6 ++ .../maps/main.sdstest | 3 + .../union types/main.sdstest | 9 ++- .../types/union types/context/main.sdstest | 61 +++++++++++++++++++ .../types/union types/context/nested.sdstest | 16 +++++ .../duplicate types/empty list.sdstest | 4 +- .../union types/duplicate types/main.sdstest | 4 +- .../union types/must have types/main.sdstest | 12 ++-- 11 files changed, 171 insertions(+), 16 deletions(-) create mode 100644 tests/resources/validation/other/types/union types/context/main.sdstest create mode 100644 tests/resources/validation/other/types/union types/context/nested.sdstest diff --git a/src/language/validation/experimentalLanguageFeatures.ts b/src/language/validation/experimentalLanguageFeatures.ts index 0ecbc0659..21daa0681 100644 --- 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,6 +30,10 @@ 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, @@ -25,6 +41,10 @@ export const mapsShouldBeUsedWithCaution = (node: SdsMap, accept: ValidationAcce }; 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/other/types/unionTypes.ts b/src/language/validation/other/types/unionTypes.ts index 3a8ca39ed..d99afa93f 100644 --- 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 index 62c32d440..988c725bc 100644 --- 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 index 4c4a858a9..4ae8e1b56 100644 --- 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 index 24a13dc14..9addb92a9 100644 --- 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 index 1fe244879..fec09d29d 100644 --- 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«, + + // $TEST$ no warning "Union types are experimental and may change without prior notice." + q: union<»union«, 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«) -> (r: »union«), 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 new file mode 100644 index 000000000..57c7d6abe --- /dev/null +++ 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«, +) + +class MyClass( + // $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions." + p: »union«, +) { + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + attr a: »union« +} + +enum MyEnum { + MyEnumVariant( + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + p: »union«, + ) +} + +fun myFunction( + // $TEST$ no error "Union types must only be used for parameters of annotations, classes, and functions." + p: »union«, +) -> ( + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + r: »union«, +) + +segment mySegment1( + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + p: »union«, +) -> ( + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + r: »union«, +) {} + +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«) -> (r: »union«), +) { + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + ( + p: »union«, + ) {}; + + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + ( + p: »union«, + ) -> 1; +} + +segment mySegment3( + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + p1: MyClass<»union«>, + + // $TEST$ error "Union types must only be used for parameters of annotations, classes, and functions." + p2: MyEnum.MyEnumVariant<»union«>, +) {} diff --git a/tests/resources/validation/other/types/union types/context/nested.sdstest b/tests/resources/validation/other/types/union types/context/nested.sdstest new file mode 100644 index 000000000..3c0707f3f --- /dev/null +++ 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«> +} + +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«) -> (r: »union«)> +} 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 index 59bd07f44..cd81b4d1f 100644 --- 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 index f0d7f0ef7..bc3931b9d 100644 --- 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 index 54a5629ea..b4779c26f 100644 --- 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»« -) {} +) // $TEST$ no error "A union type must have at least one type." -segment mySegment3( +fun myFunction3( p: union»« -) {} +)