Skip to content

Commit

Permalink
feat: error if parent type is nullable (#891)
Browse files Browse the repository at this point in the history
### Summary of Changes

Only allow non-nullable class types as the parent types of a class.
  • Loading branch information
lars-reimann authored Feb 17, 2024
1 parent bb0c94b commit add650d
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<SdsClass>([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;
}
Expand All @@ -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;
}
Expand Down
18 changes: 13 additions & 5 deletions packages/safe-ds-lang/src/language/validation/inheritance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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«
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package tests.validation.inheritance.mustInheritOnlyClasses

// $TEST$ no error "A class must only inherit classes."
class TestClass
class TestClass7
Original file line number Diff line number Diff line change
@@ -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?«

0 comments on commit add650d

Please sign in to comment.