Skip to content

Commit

Permalink
fix: type checking of two union types (#949)
Browse files Browse the repository at this point in the history
### Summary of Changes

The quantifiers were in the wrong order when checking whether two union
types were subtypes of each other:

Previously, `isSubtypeOf` only returned `true`, if **there was** one
entry of `other` that was a supertype **of all** entries of `type`.

Now, it returns `true` if **for all** entries of `type` **there is** one
entry of `other` that is a supertype.
  • Loading branch information
lars-reimann authored Mar 29, 2024
1 parent ce01628 commit 21fc485
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@ export class SafeDsTypeChecker {
* Checks whether {@link type} is a subtype of {@link other}.
*/
isSubtypeOf = (type: Type, other: Type, options: TypeCheckOptions = {}): boolean => {
// Handle base cases
if (type.equals(this.coreTypes.Nothing) || other.equals(this.coreTypes.AnyOrNull)) {
return true;
} else if (type === UnknownType || other === UnknownType) {
return false;
}

// Handle type parameter types
if (other instanceof TypeParameterType) {
if (type.isExplicitlyNullable && !other.isExplicitlyNullable) {
return false;
Expand All @@ -68,10 +70,16 @@ export class SafeDsTypeChecker {

const otherLowerBound = this.coreTypes.Nothing.withExplicitNullability(other.isExplicitlyNullable);
return this.isSubtypeOf(type, otherLowerBound, options);
}

// Handle union types
if (type instanceof UnionType) {
return type.types.every((it) => this.isSubtypeOf(it, other, options));
} else if (other instanceof UnionType) {
return other.types.some((it) => this.isSubtypeOf(type, it, options));
}

// Handle other cases
if (type instanceof CallableType) {
return this.callableTypeIsSubtypeOf(type, other, options);
} else if (type instanceof ClassType) {
Expand All @@ -88,8 +96,6 @@ export class SafeDsTypeChecker {
return this.staticTypeIsSubtypeOf(type, other, options);
} else if (type instanceof TypeParameterType) {
return this.typeParameterTypeIsSubtypeOf(type, other, options);
} else if (type instanceof UnionType) {
return this.unionTypeIsSubtypeOf(type, other, options);
} /* c8 ignore start */ else {
throw new Error(`Unexpected type: ${type.constructor.name}`);
} /* c8 ignore stop */
Expand Down Expand Up @@ -312,10 +318,6 @@ export class SafeDsTypeChecker {
return this.isSubtypeOf(upperBound, other, options);
}

private unionTypeIsSubtypeOf(type: UnionType, other: Type, options: TypeCheckOptions): boolean {
return type.types.every((it) => this.isSubtypeOf(it, other, options));
}

// -----------------------------------------------------------------------------------------------------------------
// Special cases
// -----------------------------------------------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ const basic = async (): Promise<IsSubOrSupertypeOfTest[]> => {
class Class2() sub Class1
class Class3
class Class4
enum Enum1 {
Variant1(p: Int)
Variant2
Expand Down Expand Up @@ -654,7 +652,23 @@ const basic = async (): Promise<IsSubOrSupertypeOfTest[]> => {
type2: enumType1,
expected: false,
},
// Union type to X
// Union type to union type
{
type1: factory.createUnionType(classType1, classType2),
type2: factory.createUnionType(classType1, classType2),
expected: true,
},
{
type1: factory.createUnionType(classType1, classType2),
type2: factory.createUnionType(classType1, classType3),
expected: true,
},
{
type1: factory.createUnionType(classType1, classType3),
type2: factory.createUnionType(classType1, classType2),
expected: false,
},
// Union type to other
{
type1: factory.createUnionType(),
type2: classType1,
Expand Down

0 comments on commit 21fc485

Please sign in to comment.