Skip to content

strictNullChecks mode - variable initial value is ignored in the control flow #10640

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

Closed
aliai opened this issue Aug 31, 2016 · 8 comments
Closed
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Revisit An issue worth coming back to Suggestion An idea for TypeScript

Comments

@aliai
Copy link

aliai commented Aug 31, 2016

TypeScript Version: nightly (2.0.2)

In strictNullChecks mode only:

Code

    interface IFoo {
      foo?: string;
    };

    var a: IFoo = { foo: '' };
    a.foo.split('');  // <-- Error: a.foo is possibly undefined!

Expected behavior:

This is a valid piece of code. a.foo is defined, therefore, a.foo.split is a valid expression.

Actual behavior:

a.foo is concluded as type string | defined, even though it is defined in the initiation phase var a:IFoo = { foo: '' }. tslint/typescript complains that foo is possibly undefined and therefore, cannot access to split method.

the following code runs without problem simply because foo is assigned after a declaration:

    var a: IFoo = {};
    a.foo: '';
    a.foo.split('');
@alitaheri
Copy link

{ foo: '' } is assignable to IFoo. so the type of a becomes IFoo. in the second code you're assigning something to a.foo and the control flow analyzer can prove that a.foo has value. I understand what you're asking the type-checker is valid, but it's way too far fetched!

Embrace for By Design or Working as Intended 😅

@aliai
Copy link
Author

aliai commented Aug 31, 2016

I don't know if I've managed to explain the problem clearly. I'll elaborate :)

The first code snippet doesn't work and that's the bug I am writing about. a.foo is initialized as a string and not a string|undefined. The linter fails to recognize that there is a initial value for a. And therefore, I cannot call split method on a.foo because it thinks that it is string|undefined.

The code fails on the last line a.foo.split('')

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Aug 31, 2016
@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Aug 31, 2016

@alitaheri This snippet might help illustrate the problem.

var a: string | null = 'str'

a.split('123') // no error

a = null

a.split('123') // error

@alitaheri
Copy link

@aliai @HerringtonDarkholme I did get the problem. It's just that I thought maybe the cast to IFoo erases the information about foo.

Interestingly, this code doesn't work either:

interface IFoo {
  foo?: string;
};

let a: IFoo;
a = { foo: '' };
a.foo.split('');  // <-- Error: a.foo is possibly undefined!

This sure does look like a bug to me 😅

@danielearwicker
Copy link

Naturally this works fine (inferred type of a):

let a = { foo: '' };
a.foo.split('');  // <-- no error

The reason for adding the explicit type annotation is because you are planning to assign to a objects that may or may not have a foo property. Therefore code such as a.foo.split does need to be protected against that possibility. In future maintenance it might move away from its current position and get a line inserted before it that changes a. The compiler is being helpful based on what you've told it.

If you don't want the help, don't ask for it! :) (just remove the type annotation from a).

@aliai
Copy link
Author

aliai commented Sep 2, 2016

@danielearwicker yes for the example I provided it may look silly to annotate the type :) But, this is just a simplified example to show the bug, not the necessity. The use case of this can vary of course:

    interface IUser {
      name: string;
      permissions?: {
        adminRights?: {
            moreInDepthFileld: any;
            thatIWouldLikeToHaveIntellisenseFor: any;
        };
        canPublishPost: boolean;
        canLeaveComments: boolean;
      };
      otherThingsIMightWantForMyObject?: ISomething;
    };

    var user: IUser = { name: 'foo', permissions: {} };
    user.permissions.canPublishPost = ...; // <-- Error

This is not the code I encountered this problem with since it is out of topic here. But it is a close example to the real case. The point being, the compiler doesn't take the initialization phase of the variable into account to not infer object|undefined type in strictNullCheck mode.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Bug A bug in TypeScript labels Sep 21, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Sep 21, 2016

We only narrow unions. the type of a is not, it is an IFoo. so a is not narrowed. so perhaps we need to consider property declarations as if they were assignments, i.e. treat it as:

var a: IFoo = {};
a.foo = "";
``

@RyanCavanaugh RyanCavanaugh added Revisit An issue worth coming back to Design Limitation Constraints of the existing architecture prevent this from being fixed and removed In Discussion Not yet reached consensus labels Sep 28, 2016
@RyanCavanaugh
Copy link
Member

There are some technical challenges here that would make this rather expensive in our current design. We'll think about it more and see if there's something that can be done, but there isn't an obvious solution at this time.

@mhegazy mhegazy closed this as completed Apr 24, 2017
@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
Design Limitation Constraints of the existing architecture prevent this from being fixed Revisit An issue worth coming back to Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants