TS fails to understand guard for Array.prototype.pop()

[jaredh159]

TypeScript Version: 3.4.0-dev.201xxxxx

Search Terms:

• array.pop
• Object is possibly 'undefined'

Code

const arr: string[] = [];

if (Math.random() > 0.5) { // <-- simulate runtime unknown array length
    arr.push('hi');
}

if (arr.length > 0) {
    arr.pop().toLowerCase(); // <-- error `Object is possibly 'undefined'`
}

Expected behavior:
No error, because we tested that the array's length was greater than zero.

Actual behavior:
Error: Object is possibly 'undefined'

Playground Link:
Note: turn on all strict checks in "options" (the link doesn't preserve these settings)
http://www.typescriptlang.org/play/index.html#src=const%20arr%3A%20string%5B%5D%20%3D%20%5B%5D%3B%0D%0A%0D%0Aif%20(Math.random()%20%3E%200.5)%20%7B%0D%0A%20%20%20%20arr.push('hi')%3B%0D%0A%7D%0D%0A%0D%0Aif%20(arr.length%20%3E%200)%20%7B%0D%0A%20%20%20%20%2F%2F%20NOTE%3A%20turn%20on%20all%20strict%20checks!%0D%0A%20%20%20%20arr.pop().toLowerCase()%3B%0D%0A%7D%0D%0A

Related Issues:
#29642 -- maybe? 🤔

[jack-williams]

I think is essentially the same thing as #9619, or rather, it suffers from the same limitations.

[RyanCavanaugh]

This isn't something we're capable of tracking

[tadhgmister]

you just need an interface that represents a non-empty array and a type guard function:

const arr: string[] = [];
interface NonEmptyArray<T> extends Array<T> {
    // would need to implement all relevant functions.
    pop: () => T;
}

function isNonEmpty<T>(arr: Array<T>): arr is NonEmptyArray<T> {
    return arr.length > 0;
}
if (isNonEmpty(arr)) {
    // here we are sure arr.pop() will return a value!
    arr.pop().toLowerCase();
}

Edit: never mind, in the general case there really isn't any way to ensure pop is valid since if you pop more than once this would still act like each one was guaranteed not to return undefined, so this doesn't get caught by typescript:

if (isNonEmpty(arr)) {
    arr.clear();
    // typescript still under assertion that arr isn't empty!
    arr.pop().toLowerCase();
}

[MartinJohns]

@tadhgmister This would require something like #29346, #27568, #25679, #8655, #10421. A way to indicate that a function will mutate and change the type of an argument.

[basvanmeurs]

A simple way to let Typescript know that it can assume 'not undefined' is to use an explicit cast:

if (arr.length > 0) {
    (arr.pop() as string).toLowerCase();
}

[dragomirtitian]

@basvanmeurs A not null assertion:

if (arr.length > 0) {
    arr.pop()!.toLowerCase();
}

[roeniss]

Is this relevant?

class Stack<T> {
  private data: T[] = [];
  constructor() {}
  push(item: T): void {
    this.data.push(item);
  }
  pop(): T | 0 {
    if (this.data.length > 0) return this.data.pop(); // Error : Type 'undefined' is not assignable to type '0 | T'.
    else return 0;
  }
}

EDIT: below code has no problem. Weird i think.

class Stack<T> {
  private data: T[] = [];
  constructor() {}
  push(item: T): void {
    this.data.push(item);
  }
  pop(): T  {
     return this.data[this.data.length]; // no error but it could actually return 'undefined'
  }
}

[wbt]

TypeScript already recognizes an index signature property type which must be either ‘string’ or ‘number’. In addition to that, TypeScript could keep track of minLength and maxLength as part of an Array-like's type definition, which changes dynamically through operations like push/pop/shift/unshift/splice etc. Unless otherwise specified or guarded, the default is minLength 0 and maxLength of the largest UINT32. Then you could do things like this:

if (arr.length > 0) { //guard: arr.minLength is now 1
    arr.pop().toLowerCase(); //this would be fine; arr.minLength drops by 1 to 0
    arr.pop().toLowerCase(); //this second call would have a type error, fixed by setting 0 to 1 in check.
}

Though it is not a trivial implementation, I think Typescript would be much more useful with that enhancement.