Failure to warn of possible numeric comparison with an optional function parameter

[lll000111]

TypeScript Version: 2.1.4

I had an issue with Flow that it warned for no reason because I had an "if" to guard against undefined (facebook/flow#2984) - so I tried TypeScript. TypeScript didn't warn - but it also didn't warn when I removed the if-guards!

Here is the code I used to check with TypeScript 2.1.4. Strict null checks are "true", see tsconfig.json below. I only get a warning about the unused const test.

Just to cover those cases I added a test at the top of the function to be outside the inner promise callback function - same result.

Code

'use strict';

const imap: {search(criteria: string[], cb: Function): void} = {
    search: (criteria, cb) => {}
};

const search = (from?: number, to?: number): Promise<number[]> => {
    // 1) NOT SAFE, "from" could be undefined:
    const test: boolean = from >= 4;

    return new Promise((resolve, reject) => {
        imap.search(
            ['ALL'],
            (err: Error, uids: number[]) => {

                // 2) NONE of this is safe, "from" could be undefined:
                const test: boolean = uids[0] >= from;
                uids = uids.filter((uid: number) => uid >= from);
                uids = uids.filter(function (uid: number) {
                    return uid >= from;
                });

            }
        )
    });
};

tsconfig.json:

{
    "compilerOptions": {
        "strictNullChecks": true,
        "alwaysStrict": true,
        "module": "commonjs",
        "noImplicitAny": true,
        "noImplicitReturns": true,
        "noImplicitThis": true,
        "newLine": "lf",
        "noLib": false,
        "noUnusedLocals": true,
        "removeComments": true,
        "preserveConstEnums": true,
        "sourceMap": true,
        "lib": ["ES2017"],
        "target": "ESNext"
    },
    "include": [
        "lib/*"
    ],
    "exclude": [
        "node_modules",
        "**/*.spec.ts"
    ]
}

Expected behavior:

Error: from, used in a numeric comparison, could be undefined.

Actual behavior:

No warnings or errors other than the one about the unused variable test.

---

The reason Flow correctly complains (although it over-complains) is that comparisons of a number against something that is not a number is useless. If you end up with an undefined on one side and your type system didn't warn you of that potential problem it didn't do its job as far as I'm concerned (and boy am I glad to be able to point to the Flow guys to show that I'm not alone). An undefined is an error should it occur in that position. If the code simply runs through, because the Javascript sure won't tell you about it, you may be in some trouble. Likely much later in the code, hard to debug.

So no (or yes), simply avoiding the problem by including an additional check for undefined but not doing anything about it is probably useless because it's just another way to turn off the alarm bell.

In the case of the above code example there should probably be an Error thrown, because comparing against something that isn't there should not have a legitimate result.

So the purpose of the type system warning the developer here would not be so that the developer then goes ahead and promptly ignores the warning actively, but is forced to raise the alarm if it happens during runtime - which the type system cannot do for him/her.

[ahejlsberg]

You get the expected errors with the current nightly drop. We tightened up checking of null and undefined in expression operators in #13483. It will be in the TypeScript 2.2 release.

[ahejlsberg]

Duplicate of #12795.

[lll000111]

Perfect, thanks!