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

Custom type guard fails to account for assignment within the argument expression #11271

Closed
axefrog opened this issue Sep 30, 2016 · 4 comments
Closed
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@axefrog
Copy link

axefrog commented Sep 30, 2016

TypeScript Version: 2.0.3

Code

function isDefined<T>(arg: T|undefined): arg is T {
  return arg !== void 0;
}

interface A { foo: string };

function test(a: A|undefined) {
  var b: A|undefined;
  if(isDefined(b = a)) {
    a.foo; // compile error
    b.foo; // compile error
  }
}

Expected behavior:

b is a copy of a, and is in fact what the type guard function is checking, so I'd expect the type guard to apply to both a and b.

Actual behavior:

Compiler narrows neither a nor b.

Context:

What I actually want is this:

interface A {
  bar: string
};

class Foo {
  field: A|undefined;
}

function test(foo: Foo) {
  var x: A|undefined;
  if(isDefined(x = foo.field)) {
    // x is now a copy of foo.field, and can be used without further indirection
    return x.bar;
  }
  // x is undefined here
}

The above is still possible with casting, but it seems like the type system should be able to handle this implicitly.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Sep 30, 2016
@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature and removed In Discussion Not yet reached consensus labels Oct 31, 2016
@RyanCavanaugh
Copy link
Member

Awaiting more feedback since this doesn't seem to be common. The problem is that we'd potentially have to dive arbitrarily deep into the syntax trees to look for narrowing targets, whereas it's easy to rewrite this code to the recognized form

function test(foo: Foo) {
  var x: A|undefined;
  if(x = foo.field, isDefined(x)) {
    // x is now a copy of foo.field, and can be used without further indirection
    return x.bar;
  }
  // x is undefined here
}

@axefrog
Copy link
Author

axefrog commented Nov 1, 2016

Hi Ryan, what you say is true. Though I've used the comma operator like this in the past, it hadn't occurred to me that I could apply it in TypeScript to solve this particular issue. For what it's worth, the case I mentioned becomes useful when comparing multiple values in a hot code path, and only wanting to execute the assignment if the preceding expression is true, like so:

function test(foo: Foo) {
  var x: A|undefined, y: B|undefined;
  if(isDefined(x = foo.field) && isDefined(y = foo.other)) {
    // ...
    return x.bar;
  }
}

as you say, I could write it like so:

function test(foo: Foo) {
  var x: A|undefined, y: B|undefined;
  if((x = foo.field, isDefined(x)) && (y = foo.other, isDefined(y))) {
    // ...
    return x.bar;
  }
}

@aluanhaddad
Copy link
Contributor

I think using the comma operator really detracts from the readability. Maybe it's just years of conditioning that assumes it is a bug...

@RyanCavanaugh
Copy link
Member

Declining on the basis of low feedback volume. Intentional assignment expressions in function arguments seems quite rare in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants