Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When strictNullChecks is true, enable parser to detect null checking further than the current depth #11190

Closed
szahn opened this issue Sep 27, 2016 · 11 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@szahn
Copy link

szahn commented Sep 27, 2016

For example, both methods logNullableNameCustomNullCheck and logNullableName check if the name variable is null before usage, but logNullableNameCustomNullCheck fails because the null check occurs in another method. If I have a custom helper method designed to check for nulls, I can no longer call it when strict null checking is enabled.

function isNull(a: any) {
    return a == null;
}

class Demo {
    name: string | null;

    logNullableNameCustomNullCheck() {
        if (isNull(this.name)) {
            return;
        }

        this.log(this.name);
    }

    logNullableName() {
        if (this.name == null) {
            return;
        }

        this.log(this.name);
    }

    log(message: string) {
        console.log(message);
    }

}
@RyanCavanaugh
Copy link
Member

All you have to do is add a type predicate return type:

function isNull(a: any): a is null {
    return a == null;
}

and this example works as you'd like it to.

@szahn
Copy link
Author

szahn commented Sep 27, 2016

Thanks, is this documented anywhere?

@szahn
Copy link
Author

szahn commented Sep 27, 2016

It would be great if typescript could automatically detect this. For example calling lodash methods that check for null now must be updated.

Regarding LoDash and related libraries, you'd have to fix it in this manner:

Old Lodash d.ts:

    //_.isNull
    interface LoDashStatic {
        isNull(value?: any): boolean;
    }

New Lodash d.ts:

    //_.isNull
    interface LoDashStatic {
        isNull(value?: any): value is null;
    }

@RyanCavanaugh
Copy link
Member

See "User-Defined Type Guards" at https://www.typescriptlang.org/docs/handbook/advanced-types.html

@szahn
Copy link
Author

szahn commented Sep 27, 2016

How would this work for assertions? Since JS/TS does not have assert (it would be great if it did). Sometimes, it's best to fail early with an exception rather than returning. However, asserting with a null check fails during compilation

function assert(condition: boolean, message: string) {
    if (!condition) {
        throw new Error(message);
    }
}

class Demo {
    name: string | null;

    logNullableNameWithAssert() {
        assert(this.name !== null, "Name cannot be null");

        this.log(this.name);
    }

    log(message: string) {
        console.log(message);
    }

}

Am I correct to assume the best solution would be this:

    logNullableNameWithAssert() {
        assert(this.name !== null, "Name cannot be null");
        this.log(<string>this.name);
    }

This is were I wish TypeScript was more implicit when it comes to null checks. Adding the type explicitly kind of feels like defeating the purpose of the the strict null check. If I'm forcing the compiler to use a particular type, I now loose the benefit of strict null checking.

@RyanCavanaugh
Copy link
Member

A slightly different pattern that works much better:

function fail(message: string): never {
    throw new Error(message);

}

class Demo {
    name: string | null;

    logNullableNameWithAssert() {
        if (this.name === null)
            return fail("Name cannot be null");

        this.log(this.name);
    }

    log(message: string) {
        console.log(message);
    }
}

The key here is return - it lets TS know that control flow exits at that point.

@szahn
Copy link
Author

szahn commented Sep 27, 2016

Thanks!

@szahn szahn closed this as completed Sep 27, 2016
@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Sep 27, 2016
@szahn szahn reopened this Sep 27, 2016
@szahn
Copy link
Author

szahn commented Sep 27, 2016

If I cache the results of a null check in a local variable it will fail. Do I have to always check for null and/or undefined inline?

function isNullOrUndefined(obj : any) : obj is null | undefined {
    return typeof obj === "undefined" || obj === null;
}

//Throws exception: TS2532: Object is possiblt 'undefined'
function callWithCustomNullCheck(callback ?: Function) {
    const hasCallback = !isNullOrUndefined(callback);
    if (hasCallback) {
        callback();
    }
}

function callWithInlineNullCheck(callback ?: Function) {
    if (!isNullOrUndefined(callback)) {
        callback();
    }
}

@RyanCavanaugh
Copy link
Member

Once a type guard goes into a variable, it's no longer tracked (otherwise we'd have to track all function call arguments at all points throughout the program, which would be too expensive).

@mhegazy mhegazy closed this as completed Sep 29, 2016
@pmoleri
Copy link

pmoleri commented Apr 3, 2017

Hi, I found this issue looking how to make assertions on null.

After reading this I have 2 questions:

1. Are there any plans to introduce not null | undefined asserts?
e.g.

a: string | null

assertNotNull(this.a, "a is null");
this.a.slice(1); // is a string here

It think it would be similar to user-defined type guards, but for not null asserts. But they wouldn't need an if () because they assert the type or they throw.

2. According to a previous comment, when using a function whose return type is never a return is needed.

The key here is return - it lets TS know that control flow exits at that point.

Are there plans to remove this return restriction when a function returns never?

@RyanCavanaugh
Copy link
Member

@pmoleri #8655, #12825

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants