GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Improve checking of complex rest parameter types by ahejlsberg · Pull Request #26676 · microsoft/TypeScript

Conversation

@ahejlsberg

@ahejlsberg Anders Hejlsberg (ahejlsberg) commented Aug 25, 2018

Copy link
Copy Markdown
Member

With this PR we properly check rest parameters of any subtype of any[], including user-defined array types and unions of tuple types. In cases where a rest parameter type is not exactly a T[] or a tuple type, we now collect the corresponding remainder of the argument list as a tuple type and then check that this tuple type is assignable to the rest parameter type.

declare const foo: (x: string, ...args: [string] | [number, boolean]) => void;
declare const t: [string] | [number, boolean];

foo("abc", "def");  // Ok
foo("abc", 10, true);  // Ok
foo("abc", ...t);  // Ok
foo("abc", 10);  // Error, [number] not assignable to [string] | [number, boolean]
foo("abc");  // Error, [] not assignable to [string] | [number, boolean]

Effectively, rest parameters typed as unions of tuple types provide a form of overloading expressed in a single function type signature. Unlike regular overloading, generic rest parameters can be used to form composed overloads:

type Func<T extends any[]> = (x: string, ...args: T) => void;

declare const f: Func<[] | [string] | [number, ...boolean[]]>;

f("abc");
f("abc", "def");
f("abc", 10);
f("abc", 10, true);
f("abc", 10, true, false);
f();  // Error
f("abc", true);  // Error

Function type relations have been improved to handle rest parameters of union types:

declare let f1: (x: string, ...args: [string] | [number, boolean]) => void;
declare let f2: (x: string, y: string) => void;
declare let f3: (x: string, y: number, z: boolean) => void;
declare let f4: (...args: [string, string] | [string, number, boolean]) => void;

f2 = f1;  // Ok
f3 = f1;  // Ok
f4 = f1;  // Error, misaligned complex rest types
f1 = f2;  // Error
f1 = f3;  // Error
f1 = f4;  // Error, misaligned complex rest types

Note that we currently don't "normalize" misaligned rest parameters. It would certainly be possible to do so, but it is not clear that the additional complexity is merited.

Fixes #26110.
Fixes #26491.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ulrichb, j-oliveras, Hotell, Igorbek, Kingwl, sebastianseilund, jcalz, piotrwitek, SlurpTheo, brod-dotdev, and 4 more reacted with thumbs up emoji j-oliveras, haltcase, Kingwl, KyleDavidE, and fvilante reacted with hooray emoji j-oliveras, Kingwl, sebastianseilund, KyleDavidE, janriemer, and fvilante reacted with heart emoji fvilante reacted with rocket emoji
All reactions
  • 👍 14 reactions
  • 🎉 5 reactions
  • ❤️ 6 reactions
  • 🚀 1 reaction
Anders Hejlsberg (ahejlsberg) added 6 commits August 24, 2018 13:29
@ahejlsberg
Use getSpreadArgumentType when relating to complex rest parameter types
9ef65ef
@ahejlsberg
Accept new baselines
4e44b08
@ahejlsberg
Revise complex rest parameter handling in relations and inference
f5f6133
@ahejlsberg
Accept new baselines
0e1b998
@ahejlsberg
Add tests
676892e
@ahejlsberg
Accept new baselines
349bee9
@ahejlsberg
Anders Hejlsberg (ahejlsberg) requested review from Daniel Rosenwasser (DanielRosenwasser), Ryan Cavanaugh (RyanCavanaugh) and Nathan Shively-Sanders (sandersn) August 25, 2018 15:40
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Aug 25, 2018
Tuples in rest parameters and spread expressions #24897
Merged
@mattmccutchen

Matt McCutchen (mattmccutchen) commented Aug 26, 2018

Copy link
Copy Markdown
Contributor

I just tested the PR (commit 349bee9) on the bad example from #26110 (comment) and I get no compile errors. Is this expected?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Aug 26, 2018
Spreads instantiated with array subtypes types exhibit bad behavior #26110
Closed
@mattmccutchen

Matt McCutchen (mattmccutchen) commented Aug 27, 2018

Copy link
Copy Markdown
Contributor

For anyone watching, the discussion of the unsoundness continues at #26110 (comment).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Aug 28, 2018
Improved mapped type support for arrays and tuples #26063
Merged
Mapped tuple types don't infer as arrays in generics #26163
Closed
sandersn
Nathan Shively-Sanders (sandersn) approved these changes Aug 28, 2018
View reviewed changes

@sandersn Nathan Shively-Sanders (sandersn) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the change. Just a few nits in the code.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/compiler/checker.ts Outdated
const sourceRestType = getNonArrayRestType(source);
const targetRestType = getNonArrayRestType(target);
if (sourceRestType && targetRestType && sourceCount !== targetCount) {
// We're not able to relate misaliged complex rest parameters

@sandersn Nathan Shively-Sanders (sandersn) Aug 28, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo:misaligned

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@ahejlsberg Anders Hejlsberg (ahejlsberg) Aug 29, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/compiler/checker.ts
return type.target.hasRestElement ? type.typeArguments![type.target.typeParameters!.length - 1] : undefined;
}

function getRestArrayTypeOfTupleType(type: TupleTypeReference) {

@sandersn Nathan Shively-Sanders (sandersn) Aug 28, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is only used in getEffectiveRestType, can you move this near to it?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@ahejlsberg Anders Hejlsberg (ahejlsberg) Aug 29, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it where it is, it is closely related to the function just above.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/compiler/checker.ts
}
if (restType) {
const spreadType = getSpreadArgumentType(args, argCount, args.length, restType, /*context*/ undefined);
const errorNode = reportErrors ? argCount < args.length ? args[argCount] : node : undefined;

@sandersn Nathan Shively-Sanders (sandersn) Aug 28, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of x ? y : e1 : e2 : undefined patterns. Would reportErrors && (argCount < args.length ? ...) work?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@ahejlsberg Anders Hejlsberg (ahejlsberg) Aug 29, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort of agree, but the rewrite you suggest doesn't work because it injects a false into the union type. Ima keepit.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Anders Hejlsberg (ahejlsberg) added 2 commits August 29, 2018 06:57
@ahejlsberg
Fix typo
9f4ae5f
@ahejlsberg
Merge branch 'master' into complexRestParameterTypes
a2d2f5a
@ahejlsberg
Anders Hejlsberg (ahejlsberg) merged commit d066e1e into master Aug 29, 2018
@ahejlsberg
Anders Hejlsberg (ahejlsberg) deleted the complexRestParameterTypes branch August 29, 2018 18:39
@brendankenny Brendan Kenny (brendankenny) mentioned this pull request Aug 30, 2018
Performance issue checking IndexedAccess rest parameter that resolves to large union of tuples #26756
Closed
@jacobwgillespie Jacob Gillespie (jacobwgillespie) mentioned this pull request Dec 31, 2018
React components cannot render undefined DefinitelyTyped/DefinitelyTyped#31782
Closed
9 tasks done
@ipotuzhnov Ilia Potuzhnov (ipotuzhnov) mentioned this pull request May 16, 2019
Extending empty tuple in generic functions causes compiler to infer arrays as tuples #31434
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
@typescript-bot TypeScript Bot (typescript-bot) assigned Anders Hejlsberg (ahejlsberg) Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Awaiting requested review from RyanCavanaugh
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Awaiting requested review from DanielRosenwasser

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@ahejlsberg Anders Hejlsberg (ahejlsberg)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@ahejlsberg @mattmccutchen @sandersn
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.