Skip to content

Missing variance inversion in rest-parameters type inference (extends clause) #47191

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

Closed
witemple-msft opened this issue Dec 18, 2021 · 2 comments
Labels
Duplicate An existing issue was already created

Comments

@witemple-msft
Copy link
Member

Bug Report

This is a quirk that I first heard of in the TypeScript discord, and I've just been doing a little writing about variance in TypeScript and revisited and found that it's a real soundness issue in the compiler with rest-paramters, but moreover it's inconsistent with the way individual parameters are handled.

🔎 Search Terms

"inference", "contravariance", "variance inversion", "extends clause", "rest parameters"

I found some issues that seemed like they might be related, but didn't see anything that exactly matches this situation.

🕗 Version & Regression Information

  • This is the behavior in every version I tried, including: 12/18 Nightly, 4.5.3, 4.0.5, and 3.3.3 (all TypeScript playground).

⏯ Playground Link

Playground link with relevant code. This playground is heavily annotated and compares the buggy code with other code that doesn't exhibit the bug (but is logically the same).

💻 Code

The elemental code sample is this:

// All functions under the sun are assignable to this constraint.
type AnyFunction = (...params: never[]) => unknown;

function test<F extends AnyFunction>(...args: F extends (...params: infer P) => unknown ? P : never): number[] {
  // `args` should not be assignable to `number[]`, but it is!
  return args;
}

🙁 Actual behavior

I explain it more in detail in the playground link, but the basic problem is that the constraint of P in the above snippet is computed as never[]. It should be unknown[]. This leads to a soundness issue where the function could be instantiated with any function in the universe, but the variance is backwards and so they are treated as if they are nothing and therefore assignable to anything without an assertion.

In general, when rest-parameters of some function type T are inferred in an extends clause, the inferred parameters are erroneously constrained to the parameters of constraint of T itself. That misses a variance inversion and results in the soundness issue. To show that the constraint of P comes directly from the constraint of F, another example:

function f<F extends (...params: string[]) => unknown>(
  ...args: F extends (...params: infer P) => unknown ? P : never
): (string | number)[] {
  // This is now an obvious error: type `string[]` is not assignable to `number[]`
  const n: number[] = args;
  // But this is not, even though it should be:
  return args;
}

This does not produce an error (though there should be one) because the constraint of P is computed to be string[]. However, that's not right. The constraint over F isn't that its parameters are assignable to string[], but that string[] is assignable to its parameters. If I instantiate the type with rest parameters that are (string | number)[], then I will end up assigning (string | number)[] to string[], hence the soundness issue.

🙂 Expected behavior

When the following form of extends clause appears where T is a generic and P are rest-parameters that cannot be immediately distributed to named parameters, the constraint of P should be unknown[] regardless of the constraint of T.

T extends (...params: infer P) => any

There are no constraints about what T's parameters are assignable to, only that some other type must be assignable to them). The only constraint of the rest-parameters is that they must be an array type, so logically they must have the widest possible array constraint: unknown[].

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jan 3, 2022
@ahejlsberg
Copy link
Member

This is effectively a duplicate of #26945. It would be nice to do better here, but it would require complex changes. In an ideal world we'd reason about both upper and lower bounds of type variables and use the appropriate bound based on the variance of a location when creating constraint types. However, we only track upper bounds and we're not currently equipped to understand the variance of a location during type instantiation.

With respect to the examples in this PR, the reason some of them behave correctly is that when computing the constraint of a distributive conditional type, if the resulting type is never, we simply reject that constraint. However, we don't do that for other never-ish types such as never[]. You can observe this if you rewrite one of the examples to:

function test<F extends AnyFunction>(...args: (F extends (...params: (infer P)[]) => unknown ? P : unknown)[]): number[] {
  return args;  // Error
}

But this is obviously more of a band aid than a complete solution.

@ahejlsberg ahejlsberg added Duplicate An existing issue was already created and removed Needs Investigation This issue needs a team member to investigate its status. labels Jan 11, 2022
@ahejlsberg ahejlsberg removed their assignment Jan 13, 2022
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

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