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

Generic type uplifting during assignment (ts 2775) #41047

Closed
unional opened this issue Oct 11, 2020 · 13 comments
Closed

Generic type uplifting during assignment (ts 2775) #41047

unional opened this issue Oct 11, 2020 · 13 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@unional
Copy link
Contributor

unional commented Oct 11, 2020

Search Terms:

generic uplifting, needs an explicit type annotation., 2775

This is probably an existing issue but I can't find one.

Code

function assertUnknown<T>(value: unknown, validate?: (s: T) => boolean): asserts value is T {
  if (validate && !validate(value as any)) throw new Error('failed')
  return
}
const foo = assertUnknown

const s: unknown = false
assertUnknown(s, (s: boolean) => typeof s === 'boolean')
foo(s, (s: boolean) => typeof s === 'boolean') // failed

Expected behavior:

foo works just like assertUnknown

Actual behavior:

const foo: (value: unknown, validate?: ((s: boolean) => boolean) | undefined) => asserts value is boolean
Assertions require every name in the call target to be declared with an explicit type annotation.ts(2775)
'foo' needs an explicit type annotation.

Playground Link:

playground

Related Issues:
#37818

This is probably caused by different handling of assignment vs declaration:

assertUnknown<boolean>(value: unknown, handler?: ((s: boolean) => boolean) | undefined): asserts value is boolean
foo: <boolean>(value: unknown, handler?: ((s: boolean) => boolean) | undefined) => asserts value is boolean

One interesting note:

// source.ts
export function assertType() { ... }

// works fine
assertType.unknown = function assertUnknown<T>() { ... }

// does not work
function assertUnknown<T>() { ... }
assertType.unknown = assertUnknown

// source.spec.ts
import { assertType } from './source'

assertType.unknown(..)
@MartinJohns
Copy link
Contributor

MartinJohns commented Oct 11, 2020

This is by design: #32695

All assertion functions must have an explicit type annotation, and your foo function does not have one - the type of foo is inferred from the assignment.

Once you add the explicit type annotation it works:

const foo: <T>(value: unknown, validate?: (s: T) => boolean) => asserts value is T = assertUnknown

Or a shorter version:

const foo: typeof assertUnknown = assertUnknown

I don't fully understand their reasoning behind this requirement, but it's there, and that's what the compiler told you.

@unional
Copy link
Contributor Author

unional commented Oct 11, 2020

I think it is a bug.

The fact that const foo: typeof assertUnknown = assertUnknown would fix the problem means some type information did not copy to the resulting variable while it should.

Consider this:

function assertCustom<T>(
  validator: (s: T) => boolean
): (subject: unknown) => asserts subject is T {
  return function (subject) {
    if (!validator(subject as any)) throw new TypeError(`subject fails assertion`)
    return
  }
}

const isBool = assertCustom<boolean>(s => typeof s === 'boolean')
const s: unknown = false

// fail
isBool(s)

// ok
expect(isBool(s)).toBe(undefined)

isBool has no generic type because the type is declared when calling assertCustom().

If you hover over isBool, you will see it already has the correct type:
const isBool: (subject: unknown) => asserts subject is boolean

https://stackblitz.com/edit/typescript-zcuu9x?file=index.ts

@MartinJohns
Copy link
Contributor

Your StackBlitz link doesn't work, there's no relevant content.

isBool has no generic type because the type is declared when calling assertCustom().

I'm not sure why you think that is relevant. It being generic or not has no relevance.

In your example the second call to isBool() is not used for any narrowing by the CFA, so the rule for assertion functions does not apply.

In your first call the explicit type annotation is missing. Once you add it it will work.

@unional
Copy link
Contributor Author

unional commented Oct 11, 2020

Your StackBlitz link doesn't work, there's no relevant content.

Ar, I didn't save the file. Please try again.

@MartinJohns
Copy link
Contributor

Please try again.

Works now. 👍 But the answer remains unchanged.

You also wrote:

If you hover over isBool, you will see it already has the correct type:
const isBool: (subject: unknown) => asserts subject is boolean

Having the correct type is not relevant either when the requirement is to have an explicit type annotation. Your isBool variable does not have any type annotation at all, so the type is inferred.

@unional
Copy link
Contributor Author

unional commented Oct 11, 2020

In your example the second call to isBool() is not used for any narrowing by the CFA, so the rule for assertion functions does not apply.

I see, that make sense.

However I don't see there is an explicit decision made in #32695 that the assertion did not copy to the variable.

@unional
Copy link
Contributor Author

unional commented Oct 11, 2020

I guess you mean this:

An entity is considered to have an explicit type when it is declared as a function, method, class or namespace, or as a variable, parameter or property with an explicit type annotation. (This particular rule exists so that control flow analysis of potential assertion calls doesn't circularly trigger further analysis.)

@MartinJohns
Copy link
Contributor

However I don't see there is an explicit decision made in #32695 that the assertion did not copy to the variable.

I do. :-)

A function call is analyzed as an assertion call or never-returning call when

  • each identifier in the function name references an entity with an explicit type, and

An entity is considered to have an explicit type when it is declared as a function, method, class or namespace, or as a variable, parameter or property with an explicit type annotation. (This particular rule exists so that control flow analysis of potential assertion calls doesn't circularly trigger further analysis.)

In your example isBool is not declared with an explicit type annotation.

@unional
Copy link
Contributor Author

unional commented Oct 11, 2020

Thanks.

I can't think of a case that this would trigger a circular reference during analysis.

On the other hand, I try doing the same with user-defined type guards.
In that case, it is working correctly without the need of explicit type annotation.

Therefore, it may worth some investigation for improvement.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Oct 12, 2020
@RyanCavanaugh
Copy link
Member

User-defined type guards are different from asserting methods because only the latter can cause a statement to become a new node in the control flow graph. This is not a bug or oversight.

Obviously if we could just make this work by looking at it and thinking a little, we would have, rather than write out multiple paragraphs in the PR description about the limitations 😉

@unional
Copy link
Contributor Author

unional commented Oct 12, 2020

Hi @RyanCavanaugh, thanks!

Just want to dealt into this a little bit more. 😛
I can understand that user-defined type guards and asserting methods are different at consumption.

But I don't understand why the declaration needs to be explicit.
The language service can get the needed information from the assignment:
image

So my guess is there are sufficient information available at the assignment to create the variable isBool declaring it as an asserting method.

btw, great TSConf! Thank you!

@RyanCavanaugh
Copy link
Member

The relevant process looks like this:

  1. Generate a control flow graph that determines which nodes need to be looked at in order to determine the type of an expression
  2. Get all the types of expressions using that control flow graph
  3. (There is no step 3; do not loop back to step 1)

The first part is syntactic, necessarily, because type information depends on the control flow graph. The second step then proceeds using that control flow graph.

What you need to "fix" this is to either make everything in expression space part of the control flow graph, which would be unpalatably slow (both in terms of memory consumed and CPU time consumed walking the graph), or to run some algorithm to keep augmenting the control flow graph and recalculating types until hopefully you reach a fixed point, which would also be very slow.

The "explicit type annotation" rule allows TS to augment the graph appropriately at step 1 so that it doesn't have to use information that is only available during step 2 to form a proper CFA graph.

@unional
Copy link
Contributor Author

unional commented Oct 12, 2020

Ar, got it. Thanks!

Great explanation as always. 🌷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants