From add650dd8864770bb1b6e24c24c34e7a97c43217 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sat, 17 Feb 2024 19:05:37 +0100 Subject: [PATCH] feat: error if parent type is nullable (#891) ### Summary of Changes Only allow non-nullable class types as the parent types of a class. --- .../language/typing/safe-ds-type-computer.ts | 6 ++-- .../src/language/validation/inheritance.ts | 18 +++++++--- .../type computer/streamSupertypes.test.ts | 4 +-- .../class with parent types.sdstest | 12 +++---- .../class without parent types.sdstest | 2 +- .../class with parent types.sdstest | 34 +++++++++++++++++++ 6 files changed, 58 insertions(+), 18 deletions(-) create mode 100644 packages/safe-ds-lang/tests/resources/validation/inheritance/must not be nullable/class with parent types.sdstest diff --git a/packages/safe-ds-lang/src/language/typing/safe-ds-type-computer.ts b/packages/safe-ds-lang/src/language/typing/safe-ds-type-computer.ts index f8ea9cb26..424cb8d1c 100644 --- a/packages/safe-ds-lang/src/language/typing/safe-ds-type-computer.ts +++ b/packages/safe-ds-lang/src/language/typing/safe-ds-type-computer.ts @@ -1110,17 +1110,15 @@ export class SafeDsTypeComputer { // Compared to `ClassHierarchy.properSuperclassesGenerator`, we already include the given type in the list of // visited elements, since this method here is not used to detect cycles. const visited = new Set([type.declaration]); - let isNullable = type.isNullable; let current = this.parentClassType(type); while (current && !visited.has(current.declaration)) { yield current; visited.add(current.declaration); - isNullable ||= current.isNullable; current = this.parentClassType(current); } - const Any = this.coreTypes.Any.updateNullability(isNullable); + const Any = this.coreTypes.Any.updateNullability(type.isNullable); if (Any instanceof ClassType && !visited.has(Any.declaration)) { yield Any; } @@ -1136,7 +1134,7 @@ export class SafeDsTypeComputer { const firstParentType = this.computeType(firstParentTypeNode, type.substitutions); if (firstParentType instanceof ClassType) { - return firstParentType.updateNullability(type.isNullable || firstParentType.isNullable); + return firstParentType.updateNullability(type.isNullable); } return undefined; } diff --git a/packages/safe-ds-lang/src/language/validation/inheritance.ts b/packages/safe-ds-lang/src/language/validation/inheritance.ts index 0f7c29f40..5f3b20c09 100644 --- a/packages/safe-ds-lang/src/language/validation/inheritance.ts +++ b/packages/safe-ds-lang/src/language/validation/inheritance.ts @@ -10,6 +10,7 @@ export const CODE_INHERITANCE_MULTIPLE_INHERITANCE = 'inheritance/multiple-inher export const CODE_INHERITANCE_IDENTICAL_TO_OVERRIDDEN_MEMBER = 'inheritance/identical-to-overridden-member'; export const CODE_INHERITANCE_INCOMPATIBLE_TO_OVERRIDDEN_MEMBER = 'inheritance/incompatible-to-overridden-member'; export const CODE_INHERITANCE_NOT_A_CLASS = 'inheritance/not-a-class'; +export const CODE_INHERITANCE_NULLABLE = 'inheritance/nullable'; export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services: SafeDsServices) => { const builtinAnnotations = services.builtins.Annotations; @@ -115,11 +116,18 @@ export const classMustOnlyInheritASingleClass = (services: SafeDsServices) => { // First parent type must be a class const computedType = computeType(firstParentType); - if (computedType !== UnknownType && !(computedType instanceof ClassType)) { - accept('error', 'A class must only inherit classes.', { - node: firstParentType!, - code: CODE_INHERITANCE_NOT_A_CLASS, - }); + if (computedType !== UnknownType) { + if (!(computedType instanceof ClassType)) { + accept('error', 'A class must only inherit classes.', { + node: firstParentType!, + code: CODE_INHERITANCE_NOT_A_CLASS, + }); + } else if (computedType.isNullable) { + accept('error', 'The parent type must not be nullable.', { + node: firstParentType!, + code: CODE_INHERITANCE_NULLABLE, + }); + } } // Must have only one parent type diff --git a/packages/safe-ds-lang/tests/language/typing/type computer/streamSupertypes.test.ts b/packages/safe-ds-lang/tests/language/typing/type computer/streamSupertypes.test.ts index f5ffda435..53bce186a 100644 --- a/packages/safe-ds-lang/tests/language/typing/type computer/streamSupertypes.test.ts +++ b/packages/safe-ds-lang/tests/language/typing/type computer/streamSupertypes.test.ts @@ -106,7 +106,7 @@ describe('streamProperSupertypes', async () => { expected: ['B', 'Any'], }, { - testName: 'should consider the nullability of the parent type', + testName: 'should disregard the nullability of the parent type', code: ` class Test { attr a: A @@ -115,7 +115,7 @@ describe('streamProperSupertypes', async () => { class A sub B? class B `, - expected: ['B?', 'Any?'], + expected: ['B', 'Any'], }, { testName: 'should consider the nullability of the start type', diff --git a/packages/safe-ds-lang/tests/resources/validation/inheritance/must inherit only classes/class with parent types.sdstest b/packages/safe-ds-lang/tests/resources/validation/inheritance/must inherit only classes/class with parent types.sdstest index 9312e2b24..fcb6c6c08 100644 --- a/packages/safe-ds-lang/tests/resources/validation/inheritance/must inherit only classes/class with parent types.sdstest +++ b/packages/safe-ds-lang/tests/resources/validation/inheritance/must inherit only classes/class with parent types.sdstest @@ -12,23 +12,23 @@ enum MyEnum { // $TEST$ no error "A class must only inherit classes." // $TEST$ no error "A class must only inherit classes." // $TEST$ no error "A class must only inherit classes." -class TestClass sub »MyClass«, +class TestClass1 sub »MyClass«, »MyClass.MyNestedClass«, »MyEnum«, »MyEnum.MyEnumVariant«, »Unresolved« // $TEST$ no error "A class must only inherit classes." -class TestClass sub »MyClass« +class TestClass2 sub »MyClass« // $TEST$ no error "A class must only inherit classes." -class TestClass sub »MyClass.MyNestedClass« +class TestClass3 sub »MyClass.MyNestedClass« // $TEST$ error "A class must only inherit classes." -class TestClass sub »MyEnum« +class TestClass4 sub »MyEnum« // $TEST$ error "A class must only inherit classes." -class TestClass sub »MyEnum.MyEnumVariant« +class TestClass5 sub »MyEnum.MyEnumVariant« // $TEST$ no error "A class must only inherit classes." -class TestClass sub »Unresolved« +class TestClass6 sub »Unresolved« diff --git a/packages/safe-ds-lang/tests/resources/validation/inheritance/must inherit only classes/class without parent types.sdstest b/packages/safe-ds-lang/tests/resources/validation/inheritance/must inherit only classes/class without parent types.sdstest index e0b4c3574..eb1fc7ecd 100644 --- a/packages/safe-ds-lang/tests/resources/validation/inheritance/must inherit only classes/class without parent types.sdstest +++ b/packages/safe-ds-lang/tests/resources/validation/inheritance/must inherit only classes/class without parent types.sdstest @@ -1,4 +1,4 @@ package tests.validation.inheritance.mustInheritOnlyClasses // $TEST$ no error "A class must only inherit classes." -class TestClass +class TestClass7 diff --git a/packages/safe-ds-lang/tests/resources/validation/inheritance/must not be nullable/class with parent types.sdstest b/packages/safe-ds-lang/tests/resources/validation/inheritance/must not be nullable/class with parent types.sdstest new file mode 100644 index 000000000..78a28da24 --- /dev/null +++ b/packages/safe-ds-lang/tests/resources/validation/inheritance/must not be nullable/class with parent types.sdstest @@ -0,0 +1,34 @@ +package tests.validation.inheritance.mustNotBeNullable + +class MyClass { + class MyNestedClass +} +enum MyEnum { + MyEnumVariant +} + +// $TEST$ no error "The parent type must not be nullable." +// $TEST$ no error "The parent type must not be nullable." +// $TEST$ no error "The parent type must not be nullable." +// $TEST$ no error "The parent type must not be nullable." +// $TEST$ no error "The parent type must not be nullable." +class TestClass1 sub »MyClass«, + »MyClass.MyNestedClass?«, + »MyEnum?«, + »MyEnum.MyEnumVariant?«, + »Unresolved?« + +// $TEST$ error "The parent type must not be nullable." +class TestClass2 sub »MyClass?« + +// $TEST$ error "The parent type must not be nullable." +class TestClass3 sub »MyClass.MyNestedClass?« + +// $TEST$ no error "The parent type must not be nullable." +class TestClass4 sub »MyEnum?« + +// $TEST$ no error "The parent type must not be nullable." +class TestClass5 sub »MyEnum.MyEnumVariant?« + +// $TEST$ no error "The parent type must not be nullable." +class TestClass6 sub »Unresolved?«