Typescript: Function body is not checked against assertion signature

[andersk]

TypeScript Version: 3.7.0-dev.20191002

Search Terms: assertion signatures, asserts, not checked, unchecked, unsound

Code

function assertIsString(x: number | string): asserts x is string {
}

function test(x: number | string) {
    assertIsString(x);
    x.toUpperCase();
}

test(4);

Expected behavior:

This incorrect empty implementation of assertIsString should not type check, because there are execution paths through assertIsString that don’t narrow the type of x to string as the type of assertIsString should require.

Actual behavior:

No TS errors. Compiled JS code fails at runtime: TypeError: x.toUpperCase is not a function.

Playground Link: Playground nightly isn’t new enough, but maybe it will be by the time you read this: link.

Related Issues:

[andersk]

FTR, the existing type guards feature suffers from the same soundness hole (which, as far as I can tell, isn’t called out at all in the documentation!):

function isString(x: number | string): x is string {
    return true;
}

but I’m filing this against the new feature in the hope that there’s still time to give this more thought before it becomes stable. I can file another report for type guards if desired.

Perhaps these unchecked features were justified with the rationale of allowing one to explain invariants that the checker wouldn’t derive by itself. But I would argue that they are also useful for encapsulating checks that the checker would derive if they weren’t encapsulated, and that these purposes should be separate: it should be possible to encapsulate checks without bypassing the type system.

If one does need to bypass the type system, one can already do it explicitly with casts. Bypassing the type system should be explicit.

const assertIsString: (x: number | string) => asserts x is string =
  ((x: number | string): void => {}) as
  (x: number | string) => asserts x is string;

[RyanCavanaugh]

It's unclear how we'd do this in a way that would actually be desirable. There are a lot of ways you could cause an error and a lot of ways you could plausibly check for a type; at the point that you're writing an assertion function it's not like you could just plum forget to add a body. And if you did write a body that correctly asserted the value, but TypeScript didn't believe you for whatever reason, how would you convince it otherwise (if it was a function declaration)? You can't type-assert the control flow graph.

e.g. a short and entirely plausible implementation of this function would be

function assertIsString(x: number | string): asserts x is string {
  (x as any).substr()
}

which we have no hope of correctly analyzing.

[andersk]

There are a lot of ways you could cause an error and a lot of ways you could plausibly check for a type;

TypeScript already understands most of them. I just want it to apply its existing knowledge instead of ignoring it. This, for example, should just work:

function assertIsString(x: number | string): asserts x is string {
    if (typeof x === "number")
        throw new Error("x is a number, not a string");
}

since TypeScript already understands that x has type string in any execution path where this function returns normally.

For the ones it doesn’t understand:

And if you did write a body that correctly asserted the value, but TypeScript didn't believe you for whatever reason, how would you convince it otherwise (if it was a function declaration)?

Like I suggested above:

const assertIsString: (x: number | string) => asserts x is string =
  ((x: number | string): void => {
    (x as any).substr()
  }) as
  (x: number | string) => asserts x is string;

It’s not pretty (which is perhaps a feature rather than a bug!), but it only needs to be done once. You can write this helper:

const uncheckedTypeAssert: <T>(x: unknown) => asserts x is T =
  ((x: unknown): void => {}) as
  <T>(x: unknown) => asserts x is T;

that can be reused wherever you need an unchecked type assertion:

function assertIsString(x: number | string): asserts x is string {
  (x as any).substr();
  uncheckedTypeAssert<string>(x);
}

Or, if we wanted to add a prettier syntax, how about one that reuses the familiar principle that casts are unchecked:

function assertIsString(x: number | string): asserts x is string {
  (x as any).substr();
  void 0 as asserts x is string;
}

or just

function assertIsString(x: number | string): asserts x is string {
  (x as any).substr() as asserts x is string;
}

[gnprice]

at the point that you're writing an assertion function it's not like you could just plum forget to add a body.

In practice, the bug that I'd expect to happen here is:

• You write an assertion function that's correct.
• A month later, someone extends one of the types involved in a way that makes the assertion function incorrect.

From my experience with large codebases that lack type-checking, this doesn't seem unlikely at all -- on the contrary, it feels like exactly the kind of bug that untyped codebases tend to be full of.

So I'd suggest reading @andersk's example with the empty body as a boiled-down stand-in for a lengthier example where the body looks plausible, but is missing a case.

[gnprice]

A month later, someone extends one of the types involved in a way that makes the assertion function incorrect.

Here's a fully-worked example of how that happens. Inevitably it's rather longer than the boiled-down example @andersk gave at the top. I hope it's nevertheless helpful for priming one's intuition for how a bug of exactly this kind happens in a real codebase.

The example starts from exactly the types the documentation uses as an example for type guards. I've just adapted the code slightly to use assertion functions instead.

First, the types:

interface Bird {
    fly();
    layEggs();
}

interface Fish {
    swim();
    layEggs();
}

type Pet = Bird | Fish

These are verbatim from the docs' example, except I've added an alias for the union type that the example mentions several times.

Now an assertion function, which is a direct translation of the type guard in the docs' example:

function assertIsFish(pet: Pet): asserts pet is Fish {
    if ((pet as Fish).swim === undefined) {
        throw new Error("not a fish")
    }
}

Then here's an example of using the assertion function:

function f() {
    // (... a bunch of app logic ...)

    // Because (... some invariant ...), this should in fact be a Fish.
    const fish = nextPet();
    // Assert that just to be sure.
    assertIsFish(fish);

    // Great, definitely a Fish.  We'll go do some delicate logic that
    // relies on the assumption this is a Fish.
    // (...)
    fish.swim();
    // (...)
    fish.layEggs();
    // (...)
}

At this stage, the assertion function is 100% correct, and all this code is quite reasonable.

---

Now, someone comes along and adds another type of pet, extending the union:

interface Amoeba {
    swim(): void;
    divide(): void;
}

type Pet = Bird | Fish | Amoeba

This is a perfectly reasonable change. Naturally they go on to update all the code that needs to change to handle the new Amoeba case, and they lean on the type-checker to help them be sure they've covered all of it.

They don't make any change to assertIsFish because they don't notice it -- it might be in a different file, or even a completely different project. And the type-checker doesn't give a peep about it.

But, in fact, assertIsFish is now wrong! It'll accept a pet that's an Amoeba, as well as one that's a Fish.

As a result, f can blow right past the assertIsFish call even though its fish isn't a fish at all, but an Amoeba. It'll run through the subsequent logic with that assumption violated, and e.g. blow up with a runtime type error at fish.layEggs().

Here's fully stitched-together versions of this example on the playground: before; after.

---

Unlike the Amoeba author, the type-checker looks at the definition of assertIsFish. And it has all the information it needs to see that the assertion function is now wrong -- it's not fundamentally different from the type checking that it does everywhere else. So it should be able to spot this bug in a straightforward assertion function like this one.

OTOH if I do write an assertion function in a way the type-checker doesn't understand, then as @andersk says, the bypass should be something explicit like a cast.

PS: One might notice in this example that things only actually go wrong once there's also a bug somewhere to break the invariant that the fish in f will actually be a Fish. But that's just the nature of assertions -- they only do anything when there's already a bug somewhere -- so it doesn't really mitigate this issue. And in fact an analogous example with a type guard instead (if (isFish(pet)) fish.layEggs();) will already go wrong once Amoeba is added, with no exogenous bug involved at all.