strictNullChecks + non-null flow control + higher-order function = spurious "Object is possibly 'undefined'"

[brandonbloom]

I'm basically reopenning
#10642 since the justification provided for the expected behavior doesn't apply in my case.

Here's a playground, be sure to turn on strictNullChecks:

https://www.typescriptlang.org/play/index.html#src=const%20map%20=%20new%20Map%3Cstring,%20%7Battr:%20number%7D%3E();%0Alet%20obj%20=%20map.get('key');%0Aif%20(!obj)%20%7B%0A%20%20obj%20=%20%7B%20attr:%201%20%7D;%0A%7D%0A[].map(elem%20=%3E%20%7B%0A%20%20console.log(obj.attr);%0A%7D)

TypeScript Version: 3.3.0-dev.201xxxxx

Search Terms:

• strictNullChecks
• control-flow
• undefined
• higher-order function (map, filter, forEach)
• "Object is possibly 'undefined'"

Code

const map = new Map<string, {attr: number}>();
let obj = map.get('key');
if (!obj) {
  obj = { attr: 1 };
}
[].map(elem => {
  console.log(obj.attr); // error occurs on this line
})

Expected behavior:

TypeScript should detect that there's no way obj can become undefined during the evaluation of the map call.

[AnyhowStep]

This has to do with typescript not knowing if the callback will be called immediately (it will in map()) or if it will be invoked asynchronously. (or if it won't be invoked at all)

In general, we can't know that. If it is asynchronous, then obj could, indeed, be undefined when the function is invoked.

declare let x: number | undefined;
declare function maybeSyncMaybeAsync(f: () => void): void;

if (x == undefined) {
    x = 2;
}
maybeSyncMaybeAsync(() => {
    //--strictNullChecks
    //Object is possibly 'undefined'.
    return x + 1;
});

Some people have requested some way to annotate that a callback will be invoked sync/async. The issues are probably somewhere on this tracker

[kitsonk]

Dupe of #11498?

[brandonbloom]

Asynchrony is irrelevant to my code example. The obj variable is made not-undefined before any of the higher order fuctions are called and there are no further writes to obj that may make it undefined, indeed no further writes at all. It is impossible for obj to become undefined and no annotation of the callback is required to prove that.

[kitsonk]

It is the ability to annotate immediately invoked functions. Because of #9998 all functions can either be executed immediately, or somehow not invoked during the same turn. Currently TypeScript has no way of modelling that, so it assumes all functions could be invoked out of turn (the safer option). If TypeScript had #11498 it would solve this problem. This isn't some new feature request/issue.

[brandonbloom]

First: If the work to address "immediately invoked functions" addresses my problem, that'd be great!

Second: I've gotten more than a half-dozen people here and on Twitter explaining this to me that this has to do with asynchrony, but it clearly does not. I understand why people think that, and I understand why the first step in developing the type checker was to be conservative with respect to higher-order functions, but I'm already regretting filing this bug because I just don't care enough to engage with the community about it, so I'm going to say my piece one last time, then mute this thread.

However, the minimal reproduction I provided is a much simpler case, requiring no new annotations to prove safe. No amount of asynchrony could make a non-exported variable change if there is no code path that mutates the variable below the point of the potentially asynchronous call. Detecting the vast majority of such case can be done lexically, even without any type annotations at all! Add in the existing control-flow / loop-aware type analysis, and you can handle many cases involving safe-writes, even some conditionally. To me, this is a dramatically simpler solution for a much more common problem than to consider synchronous vs asynchronous callbacks.

[AnyhowStep]

Your original post didn't elaborate on the point that no more writes to the obj variable are made after the callback is created. Ever.

Your code was in the global scope. It's reasonable to assume there may be further writes to the obj variable in other pieces of code not shown.

It would have been better to give examples where it is more obvious that no further writes are happening, like using a function's scope, or if-statement's scope, or any other local scope.

And comments explaining your point would also help.

declare function foo () : number | undefined;

function bar() {
    let f = foo();
    if (f == undefined) {
        f = 999;
    }
    //No more writes to variable `f`
    return () => {
        /*
            --strictNullChecks
            Object is possibly 'undefined'.

            Should not be possible for `f` to be 'undefined'
            because no more writes to `f` happen
            in `bar()` or this callback.

            Doesn't matter if the call to this callback
            is synchronous or asynchronous.
        */
        return f + 1;
    };
}

function bar2() {
    let f = foo();
    if (f == undefined) {
        f = 999;
    }
    //No more writes to variable `f`
    const anonymousFunc = () => {
        /*
            --strictNullChecks
            Object is possibly 'undefined'.

            Should not be possible for `f` to be 'undefined'
            because no more writes to `f` happen
            in `bar2()` or `anonymousFunc()`.

            We invoke anonymousFunc() immediately
            and never use it again.
        */
        return f + 1;
    };
    const result = anonymousFunc();
    return result;
}

function bar3() {
    let f = foo();
    if (f == undefined) {
        f = 999;
    }
    //No more writes to variable `f`
    [].map(() => {
        /*
            --strictNullChecks
            Object is possibly 'undefined'.

            Should not be possible for `f` to be 'undefined'
            because no more writes to `f` happen
            in `bar3()` or this callback.

            Doesn't matter if the call to this callback
            is synchronous or asynchronous.
        */
        return f + 1;
    });
}

function baz() {
    let f = foo();
    if (f == undefined) {
        f = 999;
    }
    
    const result = () => {
        /*
            --strictNullChecks
            Object is possibly 'undefined'.

            This is OK because we set `f` to `undefined`
            in `baz()` before it is executed.
        */
        return f + 1;
    };
    f = undefined;
    return result;
}

function buzz() {
    let f = foo();
    if (f == undefined) {
        f = 999;
    }
    
    const result = () => {
        f = undefined;
        /*
            --strictNullChecks
            Object is possibly 'undefined'.

            This is OK because we set `f` to `undefined`
            in the callback before using it.
        */
        return f + 1;
    };
    return result;
}

[weswigham]

#9998 has some words on why we bound the control flow checks where we do today.

[typescript-bot]

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.