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

Suggestion: Controlflow with assertions, add ability for "throws" type guards #31512

Closed
richardspence opened this issue May 21, 2019 · 9 comments
Labels
Duplicate An existing issue was already created

Comments

@richardspence
Copy link

richardspence commented May 21, 2019

Search Terms Assert null, control flow strictNullChecks

Suggestion

With strict null checks turned on, I'd like to avoid the use of the escape operator ! when an assert library is used. The assert library ensures downstream statements are acting on initialized or otherwise constrained values. It would be great to provide hints to the compiler of the expected type narrowing as a result of the assertions.

Add a new way to define a typeguard that instructs the compiler that it would throw errors in certain scenarios. Syntax can be something like:
throws when [Existing type guard]

Use Cases

Using runtime assertion libraries/functions that correctly narrow the runtime types for subsequent statements.

Examples

//proposal for new control flow syntax "throws when..."
function assertNotNull(item: any): throws when item is undefined {
   if(!item){
      throw new Error();
   }
}

function doSomething(arg: string | undefined){
  //after invocation, all instances of arg should be treated as arg!
  assertNotNull(arg);

  //this should succeed
  var vals = arg.splt('');
}

Checklist

My suggestion meets these guidelines:

  • [ X] This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • [ X] This wouldn't change the runtime behavior of existing JavaScript code
  • [ X] This could be implemented without emitting different JS based on the types of the expressions
  • [X ] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • [ X] This feature would agree with the rest of TypeScript's Design Goals.
@lll000111
Copy link

lll000111 commented May 21, 2019

Or in other words, this is the negation of the existing "positive" checks:

// Already works (strict checks on)
function doSomething(arg: string | undefined) {
  if (arg) {
    var vals = arg.split('');
  }
}

Makes sense, the negation (excluding the undesired value(s)) instead of including the desired ones) should work too. Everything could be reformulated in the existing "positive" checks but why not support both ways to express this.

Here are five ways to type-guard. OPs option (at 3rd position) is the only one that does not work (strict null checks need to be turned on to see the error). An alternative to OPs version is the 5th one — I guess the main selling point of OPs version is that there does not need to be a block. That 5th version does not work unless the return value of the type guard is used though, ignoring the throw statement is OPs issue, the function would work "stand-alone" but not for TS. I (too) think it would be nice if throw statements can be used to ensure a type, i.e. excluding some types (negative check).

@richardspence
Copy link
Author

Another option, using the "positive" flow:

function assertNotNull<T>(arg: T): arg is Exclude<T, undefined> {
   if(!arg){
      throw new Error();
  }
  return true;
}

This returns the narrowed type in the typeguard. Although that works, It still adds another block increasing the cyclomatic complexity and possibly requiring the addition of an else block (especially if strict return types, implicit returns, etc. is turned on).

The main goal is to remove the need of those additional code blocks and subsequent nesting.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label May 21, 2019
@RyanCavanaugh
Copy link
Member

Duplicate #8655 and others

@richardspence
Copy link
Author

@RyanCavanaugh It doesn't look #8655 comes to the same proposal (but it's the same problem). Has the throws when... piece been proposed elsewhere or should I add a comment to whichever issue you are tracking?

Also #8655 is closed due to design limitation - but I don't think that fit's my proposal here since the syntax is an extension of the current type guards

@RyanCavanaugh
Copy link
Member

The underlying root cause is the same - bare function calls can't be uniformly inserted into the control flow graph without destroying performance. Additional syntax proposals for the assertion functions' return type annotations don't help that.

@richardspence
Copy link
Author

Thanks for the explanation @RyanCavanaugh.

Can you share any pointers / area in code where that logic resides? I wouldn't mind taking a look when I have free cycles.

You are already visiting the statements inside the method, given that "inlining" the assert would succeed. Going one level deeper to see a function call's return signature doesn't seem like it should offer substantially different performance, especially since this signature (per proposal) wouldn't be inferred.

@RyanCavanaugh
Copy link
Member

I thought I had a write-up on this already, but I can't find it.

Anyway, the process for typechecking proceeds as follows:

  • Construct the control flow graph of each file based on the syntax (not types) of the code
  • Typecheck the program, which involves (among other things):
    • Resolving which declaration identifiers point to
    • Determining the types of those declarations
    • Checking the control flow graph to see which type guards are in effect

There is both a phase ordering problem and an inherent circularity here: In order to see which functions affect the control flow graph, you have to have a control flow graph in the first place in order to be able to determine which other narrowings might be affecting it.

You can construct examples that show that determining the "true" control flow graph (i.e. one that only has nodes at actual divergence points) would require an unbounded (but finite) number of inlining steps.

The simple fix is to put all potentially-diverging nodes (e.g. function calls) in the control flow graph, but again this is prohibitively expensive.

@fatcerberus
Copy link

All this talk of control flow analysis and function calls is starting to sound suspiciously like #9998.

@RyanCavanaugh
Copy link
Member

In the limit, all issues eventually link to #9998

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants