Custom type guard function #3262

tinganho · 2015-05-25T14:03:58Z

Implements only the function part of #1007. I thought it was best to have some early feedback first before I implement the class method part. So that I know I'm on the same page.

I consider the type guard type as a new type. Pls. check my tests for all specifications right now. I'm not sure if it's according to your specs. So would be good to have feedback on that.

Another question I have is if I should ban declaring type guard types on non-function-like declarations? I haven't implemented this ban yet. Example:

var a: t is item //Error

mhegazy · 2015-05-26T20:26:11Z

src/compiler/binder.ts

@@ -429,6 +431,9 @@ module ts {
                case SyntaxKind.TypeParameter:
                    bindDeclaration(<Declaration>node, SymbolFlags.TypeParameter, SymbolFlags.TypeParameterExcludes, /*isBlockScopeContainer*/ false);
                    break;
+                case SyntaxKind.TypeGuardType:
+                    bindDeclaration(<Declaration>node, (SymbolFlags.TypeGuardType | SymbolFlags.Value), SymbolFlags.TypeGuardTypeExcludes, /*isBlockScopeContainer*/ false);


why SymbolFlags.Value?

why is that even bound? it does not create a symbol.. it is not creating a new type, it is just boolean with additional properties.. correct?

I removed the binding on a later commit.

mhegazy · 2015-05-26T22:41:24Z

pinging @JsonFreeman and @ahejlsberg for this one as well.

mhegazy · 2015-05-27T00:14:25Z

@tinganho thanks for submitting this change. I took a first stab at reviewing it. Overall i do not have major concerns, just my comments above. i would want @JsonFreeman and/or @ahejlsberg to take a look as well though.

JsonFreeman · 2015-05-27T06:13:47Z

I haven't started reviewing this yet, but I don't think it's possible to only allow a certain type kind in return position because of type argument inference. You could have a function like this

function moveReturnToParameter<T>(func: () => T): (p: T) => void;

and then passing in a type guarding function would output a function that takes a type guard as a parameter.

tinganho · 2015-05-27T07:10:17Z

function moveReturnToParameter<T>(func: () => T): (p: T) => void;

Also a function that takes a type guard type(not a type guard function) as a parameter is pretty much useless.

Let say that that the returned function is:

function(isFoo: item is Foo) {
  if(isFoo) {// This won't be able to take an argument so it is not useful?
  }
}

Maybe have the type checker catch all type guard types so that they are only assigned to function like declarations?

JsonFreeman · 2015-05-27T15:08:07Z

Actually, I take that back. It is possible if you special case inference to infer boolean every time it encounters one of these type guards.

tinganho · 2015-05-28T04:20:17Z

@JsonFreeman just for clarification do you mean that:
Passing in a type guard function to the following function

function moveReturnToParameter<T>(func: () => T): (p: T) => void;

will convert the returned function to accept a boolean instead of a type guard type?

And so every non-function declaration with a declared type guard type will be converted to boolean?

tinganho · 2015-05-28T04:38:13Z

@mhegazy I fixed some of the issue you pointed out. Though I didn't understood some of your comments, it would be good if you could clarify some of them.

JsonFreeman · 2015-05-28T08:29:09Z

src/compiler/types.ts

+
+    export interface TypeGuardType extends Type {
+        parameterName: string;
+        parameterIndex?: number; // Call expression argument


If the parameterName is a name found in a binding pattern, will parameterIndex be the index of the top level binding pattern? How will you then know which binding element needs to be narrowed? Also, please ensure you have tests that try to type guard a parameter inside a binding pattern.

tinganho · 2015-06-06T06:02:07Z

I just added a restriction on type predicate signature declarations. Constructors, get/set accessors can't have type predicates. I also fixed some of the feedback.

tinganho · 2015-06-07T10:34:05Z

Just added an error for referencing a top-level binding element. Also addressed all the other feedback.

JsonFreeman · 2015-06-08T17:59:01Z

Thanks @tinganho! After these last few minor tweaks, can you merge the upstream master branch into here? Then we can work on merging this in.

yuit · 2015-06-08T19:00:03Z

Let me know if I miss anything here, but wonder what happens in the following example?

function isB(b: any): x is B {...}

I would assume we will give an error in this case.Also could you add the case in the tests.

JsonFreeman · 2015-06-08T20:14:47Z

src/compiler/checker.ts

@@ -8486,6 +8587,34 @@ module ts {
                node.kind === SyntaxKind.FunctionExpression;
        }

+        function getTypePredicateParameterIndex(parameterList: NodeArray<ParameterDeclaration>, parameter: Identifier): number {
+            let index = -1;


Is index used?

JsonFreeman · 2015-06-08T20:16:42Z

Yes @yuit, that code would be an error. The function getTypePredicateParameterIndex would return -1 / undefined, and then we would issue the error at https://github.com/Microsoft/TypeScript/pull/3262/files#diff-c3ed224e4daa84352f7f1abcd23e8ccaR8680

yuit · 2015-06-08T20:54:58Z

@JsonFreeman. I see, Thanks :)

JsonFreeman · 2015-06-09T03:26:24Z

Awesome job @tinganho! Thank you for implementing this much requested feature!

Custom type guard function

RyanCavanaugh · 2015-06-09T03:41:43Z

Really cool to see this feature go in. Great work! 🏆

tinganho · 2015-06-09T04:05:53Z

thanks @JsonFreeman @RyanCavanaugh . 😃

DickvdBrink · 2015-06-09T13:10:08Z

I like this! thanks @tinganho

ahejlsberg · 2015-06-09T14:10:41Z

@tinganho Nice work!

RichiCoder1 · 2015-06-09T14:47:41Z

prepares to re-write all the definition files

Nice work @tinganho!

msftclas added the cla-already-signed label May 25, 2015

mhegazy reviewed May 26, 2015
View reviewed changes

tinganho force-pushed the customTypeGuard2 branch from a65d0c3 to 96a17fa Compare May 27, 2015 09:07

JsonFreeman reviewed May 28, 2015
View reviewed changes

Fixes CR feedback

487dff5

tinganho force-pushed the customTypeGuard2 branch from 8da7db9 to 487dff5 Compare June 6, 2015 05:59

tinganho force-pushed the customTypeGuard2 branch from 4a14e61 to 67f9ab0 Compare June 7, 2015 10:40

Addresses CR feedback

b1a8a5f

tinganho force-pushed the customTypeGuard2 branch 2 times, most recently from b34449f to 53b934c Compare June 8, 2015 03:45

Addresses CR feedback

51a43dd

tinganho force-pushed the customTypeGuard2 branch from 53b934c to 51a43dd Compare June 8, 2015 05:34

JsonFreeman reviewed Jun 8, 2015
View reviewed changes

tinganho added 2 commits June 9, 2015 07:32

Addresses CR feedback

ebe755b

Merge commits from master

efb7013

JsonFreeman added a commit that referenced this pull request Jun 9, 2015

Merge pull request #3262 from tinganho/customTypeGuard2

6e69a9e

Custom type guard function

JsonFreeman merged commit 6e69a9e into microsoft:master Jun 9, 2015

mihailik mentioned this pull request Jun 9, 2015

Use instanceof instead of is in type guard function syntax #3437

Closed

leebyron mentioned this pull request Jul 31, 2015

Support predicate types facebook/flow#34

Closed

tinganho mentioned this pull request Aug 6, 2015

Overloads returning type predicate types are always incompatible with implementation #4180

Closed

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom type guard function #3262

Custom type guard function #3262

tinganho commented May 25, 2015

mhegazy May 26, 2015

mhegazy May 26, 2015

tinganho May 27, 2015

mhegazy commented May 26, 2015

mhegazy commented May 27, 2015

JsonFreeman commented May 27, 2015

tinganho commented May 27, 2015

JsonFreeman commented May 27, 2015

tinganho commented May 28, 2015

tinganho commented May 28, 2015

JsonFreeman May 28, 2015

tinganho commented Jun 6, 2015

tinganho commented Jun 7, 2015

JsonFreeman commented Jun 8, 2015

yuit commented Jun 8, 2015

JsonFreeman Jun 8, 2015

JsonFreeman commented Jun 8, 2015

yuit commented Jun 8, 2015

JsonFreeman commented Jun 9, 2015

RyanCavanaugh commented Jun 9, 2015

tinganho commented Jun 9, 2015

DickvdBrink commented Jun 9, 2015

ahejlsberg commented Jun 9, 2015

RichiCoder1 commented Jun 9, 2015

Custom type guard function #3262

Custom type guard function #3262

Conversation

tinganho commented May 25, 2015

mhegazy May 26, 2015

Choose a reason for hiding this comment

mhegazy May 26, 2015

Choose a reason for hiding this comment

tinganho May 27, 2015

Choose a reason for hiding this comment

mhegazy commented May 26, 2015

mhegazy commented May 27, 2015

JsonFreeman commented May 27, 2015

tinganho commented May 27, 2015

JsonFreeman commented May 27, 2015

tinganho commented May 28, 2015

tinganho commented May 28, 2015

JsonFreeman May 28, 2015

Choose a reason for hiding this comment

tinganho commented Jun 6, 2015

tinganho commented Jun 7, 2015

JsonFreeman commented Jun 8, 2015

yuit commented Jun 8, 2015

JsonFreeman Jun 8, 2015

Choose a reason for hiding this comment

JsonFreeman commented Jun 8, 2015

yuit commented Jun 8, 2015

JsonFreeman commented Jun 9, 2015

RyanCavanaugh commented Jun 9, 2015

tinganho commented Jun 9, 2015

DickvdBrink commented Jun 9, 2015

ahejlsberg commented Jun 9, 2015

RichiCoder1 commented Jun 9, 2015