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

Smarter control flow analysis for null check #10134

Closed
donaldpipowitch opened this issue Aug 4, 2016 · 5 comments
Closed

Smarter control flow analysis for null check #10134

donaldpipowitch opened this issue Aug 4, 2016 · 5 comments
Labels
Duplicate An existing issue was already created

Comments

@donaldpipowitch
Copy link
Contributor

donaldpipowitch commented Aug 4, 2016

TypeScript Version: 2.0.0

Code

class Test {
  foo: string | null;

  get hasFoo() {
    return this.foo !== null;
  }

  hasFoo2() {
    return this.foo !== null;
  }
}

function isNotNull(value) {
  return value !== null;
}

const test = new Test();

// works
if (test.foo !== null) {
  test.foo.toLowerCase();
}

// doesn't work
if (test.hasFoo) {
  test.foo.toLowerCase();
       // ^~~~ Object is possibly null.
}

// doesn't work
if (test.hasFoo2()) {
  test.foo.toLowerCase();
       // ^~~~ Object is possibly null.
}

// doesn't work
if (isNotNull(test.foo)) {
  test.foo.toLowerCase();
       // ^~~~ Object is possibly null.
}

It would be really nice, if the control flow analysis would check functions, methods and getters. This is really helpful for strictNullChecks: true.

@AdamWillden
Copy link

AdamWillden commented Sep 9, 2016

I think I'm hitting the same issue.

Is this not the same as #8437 (which was 'fixed') for getters? EDIT No it's not the same issue

@kitsonk
Copy link
Contributor

kitsonk commented Sep 9, 2016

What you are really asking for is some sort of guard that you can describe, otherwise how would TypeScript understand what is going without deep introspection of methods. To even have this in the realms of possibility, the return value needs to denote the guard.

Also, you are also implying you are asking for a negation guard as well.

Potentially, something like this:

class Test {
  foo: string | null;

  get hasFoo(): this.foo is string {
    return this.foo !== null;
  }

  hasFoo2(): this.foo is string {
    return this.foo !== null;
  }
}

function isNotNull(value): value is not null {
  return value !== null;
}

Related, #5101 suggests inferred type guards, which could potentially solve some if these use cases.

#4183 discusses negation/subtraction types.

@DanielRosenwasser
Copy link
Member

Note that with this-type guards you could write something like

hasFoo(): this is this & { foo: string } {
    return this.foo !== null;
}

But this has to be on a method, not a get-accessor.

@donaldpipowitch
Copy link
Contributor Author

@DanielRosenwasser Is it by design that get-accessor can't use this-type guards or is there an open issue I can follow?

I really want do to more complex stuff like that:

class FormValue<T> {
  @observable value: T;
  @observable error: FormError | null;

  @observable isPristine: boolean = true;
  @computed get isDirty(): boolean {
    return !this.isPristine;
  }

  @computed get isValid(): boolean {
    return this.error === null;
  }
  @computed get isInvalid(): boolean {
    return !this.isValid;
  }

  @computed get shouldShowErrors(): boolean {
    return this.isInvalid && this.isDirty;
  }
}

// and later somewhere
const Input = observer((props) => 
  <div>
    <input type="text" value={props.formValue.value}/>
    {props.formValue.shouldShowErrors ?
      <p>{props.formValue.error}</p> {/* error is possibly null here */}
    :
      null
    }
  </div>
)

It would be great if I could tell TypeScript:

  • When shouldShowErrors is true this means...
  • isInvalid is true which means...
  • isValid is false which means...
  • error must be a FormError and not null

@mhegazy
Copy link
Contributor

mhegazy commented Dec 14, 2016

Duplicate of #11796 and #11117?

@mhegazy mhegazy added the Duplicate An existing issue was already created label Dec 14, 2016
@mhegazy mhegazy closed this as completed Dec 29, 2016
@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
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants