Code flow analysis isn't smart enough

[pleerock]

typescript version: Version 1.9.0-dev.20160424

first looks like a bug:

if (options.leftJoin)
    Object.keys(options.leftJoin).forEach(key => {
        qb.leftJoin(options.leftJoin[key], key);
    });

Gives TS2531: Object is possibly 'null' or 'undefined'., however error is not expected because of the if (options.leftJoin) check above.

second:

connect(options?: ConnectionOptions) {
    if (!options)
        options = {} as ConnectionOptions;

    console.log(options.url);
}

Gives same TS2531: Object is possibly 'null' or 'undefined'.. Looks like expected behaviour because of original type of the options object. Solution is to define variable with a new type and without undefined. Maybe there is a smarter way to do it?

[basarat]

I did a quick test with alm. Looks good to me 🌹:

type ConnectionOptions = {url:string}
function connect(options?: ConnectionOptions) {
    if (!options)
        options = {} as ConnectionOptions;

    console.log(options.url);
}

Proof

[tinganho]

I'm also experiencing the same error as the second example using ||:

export function patch<R>(path: string, options?: HTTPOptions): Promise<HTTPResponse<R>> {
    options = options || {};
    options.method = 'PATCH'; // error options can be null or undefined

    let req = new HttpRequest<R>(path, options);
    return req.request();
}

In addition, the options is HTTPOptions | undefined when I inspect it. Though it is giving me the error that it can be null or undefined. Slight incorrect since it cannot be null.

[ahejlsberg]

@pleerock Regarding the first issue, control flow analysis doesn't consider a type guard to hold across a callback (because the callback could happen at any time, including outside the guarded block). This is different and more conservative from what we've previously done, and we're certainly open to feedback about whether it is too conservative. Meanwhile, you probably want to capture the guarded variable in a local and then reference that in the callback:

if (options.leftJoin) {
    const leftJoin = options.leftJoin;
    Object.keys(leftJoin).forEach(key => {
        qb.leftJoin(leftJoin[key], key);
    });
    ...
}

Regarding your second example, I can't tell how ConnectionOptions is defined and in particular whether it is a union type that includes null and undefined. However, assuming ConnectionOptions is just a plain object type, your example compiles with no error as expected. Looks like that is what @basarat is seeing too.

@tinganho Your example compiles fine for me (but I suspect you have an older nightly build).

[pleerock]

@basarat are you on the same typescript version as me? Because it looks weird that I get this error and you are not. I used same code as you and I get an error. Proof:

[pleerock]

@ahejlsberg thank you, got the problem of the first issue. For the second issue I have same code as basarat which does not compile

[tinganho]

@pleerock I can also compile @basarat 's example correctly. Actually forgot the strictNullChecks flag below. But getting the same error as @pleerock now.

$ tsc -v
Version 1.9.0-dev.20160424

tinganho at Tiens-MacBook-Pro in ~/Development/Mount on master [!?]
$ cat test.ts
type ConnectionOptions = {url: string}
function connect(options?: ConnectionOptions) {
    if (!options) {
        options = {} as ConnectionOptions;
    }

    console.log(options.url);
}
tinganho at Tiens-MacBook-Pro in ~/Development/Mount on master [!?]
$ tsc --strictNullChecks test.ts
test.ts(7,17): error TS2531: Object is possibly 'null' or 'undefined'.

The strange thing is that it doesn't work on my IDE VSCode Insider:

I have defined in my workspace settings:

{
    "typescript.tsdk": "/usr/local/lib/node_modules/typescript/lib",
}

[ahejlsberg]

It appears the current typescript@next (1.9.0-dev.20160424) still doesn't contain the control flow type analysis logic (there's no getFlowTypeAtReference function in the tsc.js file). Don't know why, something must have gone awry on the nightly build server.

@mhegazy Mohamed, any ideas?

[ahejlsberg]

@tinganho You need to specify --strictNullChecks on the command-line to repro @pleerock's issue.

[tinganho]

Yes got it now.

[mhegazy]

Looks like we have not published a nightly since 4/12. the script is publishing without updating. looking into it.

[basarat]

are you on the same typescript version as me

@pleerock I am actually on nTypeScript https://github.com/TypeStrong/ntypescript which is just a build of TypeScript/master with some automation I did before official typescript@next but continue to use because I like to experiment with non public / non supported / cutting edge APIs 🌹

[ahejlsberg]

We've identified and fixed the issue with the nightly build server. Tonight's build should be good and will contain the control flow type analysis logic.

[pleerock]

confirming that it works with the last build

[yuit]

@pleerock thanks for the confirmation.

[RReverser]

@ahejlsberg @yuit This doesn't work in nightly TypeScript - is it a related issue or should I file as a new bug?

let maybeNumber: number | undefined;
(function () {
    maybeNumber = 1;
})();
if (maybeNumber) {
    maybeNumber++; // -> says that type of `maybeNumber` is always `nothing`, not even `number | undefined` anymore
}

[mhegazy]

@RReverser this is not covered in OP. control flow analysis is localized, and does not track side effects from other functions. feel free to file a new issue though.

[RReverser]

@mhegazy Well, I don't expect it to be smart enough to refine maybeNumber to number, but I also don't see why compiler might assume that maybeNumber becomes nothing after such call - preserving number | undefined seems more reasonable to me.

[RReverser]

Filed at #8381.

[mhegazy]

the issue is it does not know about the side effects. you would want it to catch something like:

let maybeNumber: number | undefined;
if (maybeNumber) {
    maybeNumber++; // there is no way the control flow would reach here.
}

[RReverser]

@mhegazy Hmm, can't it assume that declared variable type was preserved on unknown side effects?

[mhegazy]

the declared variable type is always there. but then control flow "narrows" it down. so:

let maybeNumber: number | undefined;

maybeNumber // it is undefined here, though it is defined as a number | undefined, there are no initialization to a non-undefined value.

maybeNumber  = 1;  // it is number definitely


maybeNumber  ++; // allowed, as it is definitely a number

if (maybeNumber === undefined) {
    maybeNumber  // here it is nothing, i.e. there is no way for the control flow to reach here, as it was a number, and number does not include undefined.
}

[RReverser]

@mhegazy Yeah, I understand why in:

let maybeNumber: number | undefined;
if (maybeNumber) {
    maybeNumber++; // there is no way the control flow would reach here.
}

it fails - it's obvious that control flow won't reach here because maybeNumber is undefined at that point.

However, I would expect compiler to assume number | undefined in my initial example (this actually "broke" my working code) - if it's unsure about side effects, then treat variable type after the call as per its initial declared type and not keep the type as it was before the call:

let maybeNumber: number | undefined;
// Control flow figured that: maybeNumber === undefined
(function () {
    maybeNumber = 1;
})(); // --> oops, unknown side effects
// Revert maybeNumber to initial `number | undefined` to cover all possibilities due to the unknown side effects in the flow
if (maybeNumber) {
    // Control flow: can refine to `number` again here
    maybeNumber++;
}

[mhegazy]

yup, i think we are saying the same thing. and that is what #8381 is tracking.

[RReverser]

@mhegazy Cool, thanks. Just wanted to be sure we're on the same page / I'm not getting anything wrong.

[gcnew]

Hi, I hit this error too. I understand the reasoning behind not carrying the reduced type over closures but non the less it looks very counter intuitive.

My case is with returning a closure, which should be safe.

function getLengthLambda(x: any[]|null) {
    if (!x) {
        return () => 0;
    }

    return () => x.length + 1; // <--- error TS2531: Object is possibly 'null'
}

getLengthLambda([]);

[108adams]

I have just encountered the problem with _.isEmpty (underscore safe check, will shortcut on undefined and null):

    if (_.isEmpty(childrenDescription)) {
        return null;
    }

    // Here I get TS2531: Object is possibly 'null' or 'undefined`, which is wrong
    return childrenDescription.map(createChild);

As for now I'm switching from elegant underscore to sth like

if (!childrenDescription || childrenDescription.length === 0) // blehhhh

tsc 2.0.3

[RReverser]

@108adams It's just a problem of poor definition, not of TypeScript itself.

For example, if I define helper like

declare function isEmpty(x: any): x is null | undefined;

it can already catch these cases and control flow will know that anything after the check is at least non-null.