Navigation Menu

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

Leading and middle rest elements in tuple types #41544

Merged
merged 7 commits into from Jan 5, 2021
Merged

Leading and middle rest elements in tuple types #41544

merged 7 commits into from Jan 5, 2021

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Nov 15, 2020

With this PR we support leading and middle rest elements in tuple types. For example, we now support the following:

type T1 = [...string[], number];  // Zero or more strings followed by a number
type T2 = [number, ...boolean[], string, string];  // Number followed by zero or more booleans followed by two strings

Previously it was not possible to have anything follow a rest element and thus it was not possible to express tuple types that end in a fixed set of elements. Such tuple types are useful for strongly typing functions with variable parameter lists that end in a fixed set of parameters. For example, the following is now permitted:

function f1(...args: [...string[], number]) {
    const strs = args.slice(0, -1) as string[];
    const num = args[args.length - 1] as number;
    // ...
}

f1(5);
f1('abc', 5);
f1('abc', 'def', 5);
f1('abc', 'def', 5, 6);  // Error

Note the use of a rest parameter with a tuple type that starts with a rest element. Also note that indexing a tuple type beyond its starting fixed elements (if any) yields a union type of all possible element types (in this case string | number). Thus, type assertions are required in the code above.

Tuple type layouts are now governed by these rules:

  • An optional element cannot precede a required element or follow a rest element.
  • Multiple rest elements are not permitted.

Thus, the supported layouts of tuple types are:

  • Zero or more required elements, followed by zero or more optional elements, followed by zero or one rest element, or
  • Zero or more required elements, followed by a rest element, followed by zero or more required elements.

In either layout, zero or more generic variadic elements may be present at any position in the tuple type. Errors are reported on tuple types that don't match the layout rules, but through generic type instantiation it is still possible to create invalid layouts. For this reason, tuple types are normalized following generic type instantiation:

  • Optional elements preceding the last required element are turned into required elements.
  • Elements between the first rest element and the last rest or optional element are turned into a single rest element with a union of the element types.

Some examples of normalization:

type Tup3<T extends unknown[], U extends unknown[], V extends unknown[]> = [...T, ...U, ...V];

type TN1 = Tup3<[number], string[], [number]>;  // [number, ...string[], number]
type TN2 = Tup3<[number], [string?], [boolean]>;  // [number, string | undefined, boolean]
type TN3 = Tup3<[number], string[], [boolean?]>;  // [number, (string | boolean | undefined)[]]
type TN4 = Tup3<[number], string[], boolean[]>;  // [number, (string | boolean)[]]
type TN5 = Tup3<string[], number[], boolean[]>;  // (string | number | boolean)[]

Fixes #39595.

@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 15, 2020

Heya @ahejlsberg, I've started to run the perf test suite on this PR at 77722bb. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 15, 2020

Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 77722bb. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 15, 2020

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 77722bb. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 15, 2020

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 77722bb. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

@ahejlsberg
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..41544

Metric master 41544 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 344,475k (± 0.02%) 344,506k (± 0.02%) +31k (+ 0.01%) 344,327k 344,763k
Parse Time 1.99s (± 0.63%) 1.99s (± 0.71%) +0.00s (+ 0.05%) 1.96s 2.03s
Bind Time 0.83s (± 0.63%) 0.83s (± 1.14%) +0.01s (+ 0.72%) 0.81s 0.85s
Check Time 4.97s (± 0.58%) 4.97s (± 0.82%) +0.01s (+ 0.12%) 4.89s 5.06s
Emit Time 5.34s (± 0.54%) 5.35s (± 0.72%) +0.01s (+ 0.13%) 5.30s 5.48s
Total Time 13.12s (± 0.37%) 13.14s (± 0.63%) +0.02s (+ 0.13%) 13.04s 13.40s
Monaco - node (v10.16.3, x64)
Memory used 354,710k (± 0.02%) 354,684k (± 0.01%) -26k (- 0.01%) 354,571k 354,767k
Parse Time 1.60s (± 0.44%) 1.60s (± 0.59%) +0.00s (+ 0.12%) 1.58s 1.62s
Bind Time 0.73s (± 0.65%) 0.73s (± 0.55%) -0.00s (- 0.55%) 0.72s 0.74s
Check Time 5.15s (± 0.69%) 5.15s (± 0.37%) -0.00s (- 0.02%) 5.10s 5.18s
Emit Time 2.81s (± 0.75%) 2.81s (± 0.60%) +0.00s (+ 0.07%) 2.78s 2.85s
Total Time 10.29s (± 0.52%) 10.29s (± 0.21%) -0.00s (- 0.01%) 10.26s 10.37s
TFS - node (v10.16.3, x64)
Memory used 307,863k (± 0.03%) 307,868k (± 0.02%) +5k (+ 0.00%) 307,764k 308,026k
Parse Time 1.24s (± 0.60%) 1.24s (± 0.81%) -0.00s (- 0.24%) 1.21s 1.26s
Bind Time 0.68s (± 0.85%) 0.68s (± 0.53%) +0.00s (+ 0.59%) 0.68s 0.69s
Check Time 4.59s (± 0.68%) 4.60s (± 0.79%) +0.01s (+ 0.17%) 4.52s 4.67s
Emit Time 2.94s (± 1.15%) 2.95s (± 0.79%) +0.00s (+ 0.10%) 2.89s 2.99s
Total Time 9.46s (± 0.52%) 9.47s (± 0.60%) +0.01s (+ 0.14%) 9.33s 9.60s
material-ui - node (v10.16.3, x64)
Memory used 489,238k (± 0.01%) 489,183k (± 0.01%) -55k (- 0.01%) 489,034k 489,299k
Parse Time 2.06s (± 0.43%) 2.07s (± 0.64%) +0.00s (+ 0.19%) 2.04s 2.10s
Bind Time 0.66s (± 0.91%) 0.65s (± 0.95%) -0.01s (- 1.07%) 0.64s 0.66s
Check Time 13.58s (± 0.91%) 13.60s (± 0.58%) +0.02s (+ 0.15%) 13.46s 13.85s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.30s (± 0.77%) 16.32s (± 0.52%) +0.02s (+ 0.11%) 16.16s 16.59s
Angular - node (v12.1.0, x64)
Memory used 322,356k (± 0.03%) 322,349k (± 0.02%) -7k (- 0.00%) 322,170k 322,505k
Parse Time 1.98s (± 0.95%) 1.97s (± 0.53%) -0.01s (- 0.40%) 1.95s 1.99s
Bind Time 0.81s (± 0.37%) 0.82s (± 0.63%) +0.01s (+ 0.86%) 0.81s 0.83s
Check Time 4.88s (± 0.39%) 4.90s (± 0.39%) +0.02s (+ 0.49%) 4.85s 4.94s
Emit Time 5.50s (± 0.53%) 5.54s (± 0.89%) +0.04s (+ 0.76%) 5.43s 5.64s
Total Time 13.17s (± 0.35%) 13.23s (± 0.54%) +0.06s (+ 0.48%) 13.06s 13.37s
Monaco - node (v12.1.0, x64)
Memory used 336,801k (± 0.01%) 336,814k (± 0.02%) +13k (+ 0.00%) 336,638k 336,950k
Parse Time 1.59s (± 0.33%) 1.60s (± 0.47%) +0.01s (+ 0.31%) 1.58s 1.61s
Bind Time 0.71s (± 0.73%) 0.71s (± 1.15%) +0.00s (+ 0.56%) 0.70s 0.74s
Check Time 4.91s (± 0.46%) 4.92s (± 0.44%) +0.01s (+ 0.29%) 4.86s 4.96s
Emit Time 2.88s (± 0.78%) 2.86s (± 0.64%) -0.02s (- 0.52%) 2.81s 2.90s
Total Time 10.09s (± 0.30%) 10.10s (± 0.32%) +0.00s (+ 0.04%) 10.03s 10.17s
TFS - node (v12.1.0, x64)
Memory used 292,077k (± 0.03%) 292,095k (± 0.03%) +18k (+ 0.01%) 291,932k 292,263k
Parse Time 1.26s (± 0.85%) 1.26s (± 0.74%) +0.00s (+ 0.24%) 1.24s 1.29s
Bind Time 0.65s (± 0.46%) 0.66s (± 1.22%) +0.01s (+ 0.92%) 0.64s 0.68s
Check Time 4.53s (± 0.62%) 4.52s (± 0.65%) -0.01s (- 0.29%) 4.45s 4.56s
Emit Time 2.96s (± 0.84%) 2.99s (± 1.03%) +0.03s (+ 1.01%) 2.93s 3.04s
Total Time 9.39s (± 0.58%) 9.41s (± 0.62%) +0.02s (+ 0.21%) 9.29s 9.51s
material-ui - node (v12.1.0, x64)
Memory used 467,184k (± 0.05%) 467,181k (± 0.05%) -3k (- 0.00%) 466,205k 467,368k
Parse Time 2.08s (± 0.65%) 2.08s (± 0.36%) +0.00s (+ 0.10%) 2.06s 2.09s
Bind Time 0.64s (± 0.90%) 0.64s (± 0.74%) 0.00s ( 0.00%) 0.63s 0.65s
Check Time 12.06s (± 1.07%) 12.14s (± 0.87%) +0.08s (+ 0.69%) 11.97s 12.41s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.78s (± 0.96%) 14.86s (± 0.74%) +0.09s (+ 0.59%) 14.68s 15.15s
Angular - node (v8.9.0, x64)
Memory used 347,023k (± 0.02%) 347,043k (± 0.02%) +20k (+ 0.01%) 346,898k 347,140k
Parse Time 2.52s (± 0.48%) 2.52s (± 0.23%) +0.00s (+ 0.16%) 2.51s 2.53s
Bind Time 0.87s (± 0.46%) 0.87s (± 0.89%) 0.00s ( 0.00%) 0.86s 0.89s
Check Time 5.63s (± 0.43%) 5.60s (± 0.48%) -0.03s (- 0.46%) 5.56s 5.68s
Emit Time 6.35s (± 0.76%) 6.35s (± 1.14%) +0.01s (+ 0.09%) 6.16s 6.47s
Total Time 15.36s (± 0.33%) 15.34s (± 0.47%) -0.02s (- 0.11%) 15.21s 15.53s
Monaco - node (v8.9.0, x64)
Memory used 358,531k (± 0.01%) 358,537k (± 0.01%) +6k (+ 0.00%) 358,423k 358,598k
Parse Time 1.93s (± 0.30%) 1.93s (± 0.40%) +0.00s (+ 0.21%) 1.91s 1.95s
Bind Time 0.91s (± 0.54%) 0.91s (± 0.49%) +0.01s (+ 0.66%) 0.90s 0.92s
Check Time 5.65s (± 0.65%) 5.71s (± 0.53%) +0.06s (+ 1.11%) 5.65s 5.78s
Emit Time 3.40s (± 1.24%) 3.43s (± 0.40%) +0.03s (+ 1.03%) 3.41s 3.46s
Total Time 11.88s (± 0.46%) 11.99s (± 0.31%) +0.12s (+ 0.97%) 11.93s 12.09s
TFS - node (v8.9.0, x64)
Memory used 310,349k (± 0.01%) 310,376k (± 0.01%) +27k (+ 0.01%) 310,305k 310,496k
Parse Time 1.57s (± 0.43%) 1.57s (± 0.37%) -0.01s (- 0.44%) 1.56s 1.58s
Bind Time 0.69s (± 0.65%) 0.69s (± 0.81%) 0.00s ( 0.00%) 0.68s 0.70s
Check Time 5.33s (± 0.56%) 5.37s (± 0.44%) +0.04s (+ 0.83%) 5.31s 5.41s
Emit Time 2.97s (± 0.59%) 2.98s (± 0.97%) +0.01s (+ 0.27%) 2.91s 3.05s
Total Time 10.56s (± 0.27%) 10.61s (± 0.41%) +0.04s (+ 0.40%) 10.52s 10.70s
material-ui - node (v8.9.0, x64)
Memory used 496,232k (± 0.01%) 496,285k (± 0.01%) +53k (+ 0.01%) 496,125k 496,386k
Parse Time 2.50s (± 0.48%) 2.51s (± 0.88%) +0.01s (+ 0.44%) 2.47s 2.58s
Bind Time 0.82s (± 1.69%) 0.82s (± 1.41%) -0.00s (- 0.24%) 0.79s 0.84s
Check Time 18.14s (± 0.86%) 18.21s (± 0.89%) +0.07s (+ 0.39%) 17.97s 18.73s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 21.45s (± 0.73%) 21.53s (± 0.80%) +0.08s (+ 0.37%) 21.23s 22.05s
Angular - node (v8.9.0, x86)
Memory used 198,978k (± 0.02%) 198,975k (± 0.03%) -3k (- 0.00%) 198,803k 199,118k
Parse Time 2.44s (± 0.55%) 2.44s (± 0.64%) -0.00s (- 0.04%) 2.41s 2.47s
Bind Time 1.02s (± 0.87%) 1.02s (± 0.75%) -0.01s (- 0.78%) 1.00s 1.03s
Check Time 5.09s (± 0.58%) 5.06s (± 0.53%) -0.02s (- 0.47%) 5.01s 5.13s
Emit Time 6.12s (± 0.82%) 6.12s (± 0.65%) +0.00s (+ 0.02%) 6.05s 6.20s
Total Time 14.67s (± 0.52%) 14.64s (± 0.49%) -0.03s (- 0.17%) 14.50s 14.81s
Monaco - node (v8.9.0, x86)
Memory used 203,062k (± 0.02%) 203,089k (± 0.02%) +27k (+ 0.01%) 203,022k 203,150k
Parse Time 1.99s (± 0.76%) 1.98s (± 0.94%) -0.01s (- 0.45%) 1.96s 2.04s
Bind Time 0.72s (± 0.81%) 0.72s (± 1.58%) +0.01s (+ 0.70%) 0.71s 0.76s
Check Time 5.77s (± 0.60%) 5.81s (± 0.59%) +0.04s (+ 0.73%) 5.73s 5.90s
Emit Time 2.76s (± 1.39%) 2.75s (± 0.85%) -0.01s (- 0.33%) 2.71s 2.81s
Total Time 11.24s (± 0.32%) 11.27s (± 0.49%) +0.03s (+ 0.23%) 11.19s 11.41s
TFS - node (v8.9.0, x86)
Memory used 177,488k (± 0.02%) 177,511k (± 0.02%) +24k (+ 0.01%) 177,449k 177,568k
Parse Time 1.61s (± 0.72%) 1.61s (± 1.03%) +0.00s (+ 0.19%) 1.58s 1.65s
Bind Time 0.66s (± 0.76%) 0.66s (± 1.04%) +0.00s (+ 0.15%) 0.65s 0.68s
Check Time 4.90s (± 0.63%) 4.87s (± 0.38%) -0.04s (- 0.73%) 4.82s 4.91s
Emit Time 2.85s (± 0.73%) 2.84s (± 0.80%) -0.01s (- 0.39%) 2.77s 2.88s
Total Time 10.01s (± 0.42%) 9.97s (± 0.50%) -0.04s (- 0.44%) 9.85s 10.06s
material-ui - node (v8.9.0, x86)
Memory used 279,352k (± 0.02%) 279,381k (± 0.02%) +29k (+ 0.01%) 279,263k 279,484k
Parse Time 2.55s (± 0.65%) 2.56s (± 0.37%) +0.00s (+ 0.08%) 2.54s 2.58s
Bind Time 0.81s (± 6.62%) 0.81s (± 6.31%) -0.00s (- 0.37%) 0.70s 0.89s
Check Time 16.55s (± 0.40%) 16.57s (± 1.00%) +0.02s (+ 0.11%) 16.25s 16.92s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.91s (± 0.45%) 19.93s (± 0.78%) +0.02s (+ 0.10%) 19.64s 20.34s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
Benchmark Name Iterations
Current 41544 10
Baseline master 10

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@ahejlsberg
Copy link
Member Author

All tests look clean and perf appears unaffected.

@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 20, 2020

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 77722bb. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 20, 2020

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/89656/artifacts?artifactName=tgz&fileId=5EF2D96AB8BB9E4EB9303942A61DEA8C51BE11AC597FD4CE8F3C9ACCEA28D90E02&fileName=/typescript-4.2.0-insiders.20201120.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

@sandersn sandersn added this to Not started in PR Backlog Nov 23, 2020
@sandersn sandersn moved this from Not started to Needs review in PR Backlog Nov 23, 2020
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

I'm going to pause for a second here to post what I have thus far, since it's mostly design feedback - specifically around the premise of this PR. The first line of the description is:

With this PR we support leading and middle rest elements in tuple types

And that's a really good thing. Except as implemented right here, we only actually support them in generic cases. And then we always end up erasing middle/leading spreads once the generics are gone into a much broader type. If we were erasing them into a much more specific type (so the result allowed fewer assignments), I could maybe get it - but the significantly reduced accuracy of these constructs once we're talking about non-generic types really rubs me the wrong way. I think we need to preserve things like [...string[], ...number[]] because those types have significant meaning beyond the combined simplifications presented here - the same way the non-generic pattern literals we just added have meaning in the sequences they preserve.

@@ -28,5 +28,5 @@ tests/cases/compiler/spliceTuples.ts(23,1): error TS2322: Type '[number, string,
k6 = [1, ...sbb_];
~~
!!! error TS2322: Type '[number, string, boolean, ...boolean[]]' is not assignable to type '[number, string, boolean, boolean, ...boolean[]]'.
!!! error TS2322: Property '3' is optional in type '[number, string, boolean, ...boolean[]]' but required in type '[number, string, boolean, boolean, ...boolean[]]'.
!!! error TS2322: Source provides no match for required element at position 3 in target.
Copy link
Member

Choose a reason for hiding this comment

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

So I keep reading this error and coming to it feeling kinda lacking in information. Maybe something like

Suggested change
!!! error TS2322: Source provides no match for required element at position 3 in target.
!!! error TS2322: Type '[number, string, boolean, ...boolean[]]' provides no match for required element at position 3 in type '[number, string, boolean, boolean, ...boolean[]]'.

I feel like yeah, we work with source/target all the time in the compiler, sure, but I think having the types right there, inline, make it easier to take in. It'd also be neat if we could, like, underline or highlight the tuple element we're referring to somehow, like


!!! error TS2322:   Type '[number, string, boolean, ...boolean[]]' provides no match for required element at position 3 in type '[number, string, boolean, boolean, ...boolean[]]'.
                                                    ~~~~~~~~~~~~                                                                                           ~~~~~~~

because referring to a tuple element index across two tuples, while concrete, feels like it can be hard to pick out what pair of elements are being compared when the tuples are long. Though that last part (the underlining) we can discuss/consider implementing another time. I do think restoring a slightly more detailed error message is warranted, however.

Copy link
Member

Choose a reason for hiding this comment

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

#41860 now keeps track of some ideas we have in mind here.

type Tup3<T extends unknown[], U extends unknown[], V extends unknown[]> = [...T, ...U, ...V];

type V20 = Tup3<[number], string[], [number]>; // [number, ...string[], number]
type V21 = Tup3<[number], [string?], [boolean]>; // [number, string | undefined, boolean]
Copy link
Member

Choose a reason for hiding this comment

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

This one seems wrong. A [string?] is equivalent to [] | [string | undefined], so I'd expect something equivalent to [number, boolean] | [number, string | undefined, boolean] to pop out (if not that, verbatim). There's no error here, so quietly swallowing the optionality without reflecting it in the result seems ripe for bad behavior.


type V20 = Tup3<[number], string[], [number]>; // [number, ...string[], number]
type V21 = Tup3<[number], [string?], [boolean]>; // [number, string | undefined, boolean]
type V22 = Tup3<[number], string[], boolean[]>; // [number, (string | boolean)[]]
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem equivalent? A sequence of strings followed by a sequence of booleans is certainly different than a mixed sequence of strings and booleans. Specifically, the later admits a type like [number, string, boolean, string, boolean] that the former does not. Why can we not preserve [number, ...string[], ...boolean[]] ? It certainly seems to me to be distinct from this.

type V20 = Tup3<[number], string[], [number]>; // [number, ...string[], number]
type V21 = Tup3<[number], [string?], [boolean]>; // [number, string | undefined, boolean]
type V22 = Tup3<[number], string[], boolean[]>; // [number, (string | boolean)[]]
type V23 = Tup3<[number], string[], [boolean?]>; // [number, (string | boolean | undefined)[]]
Copy link
Member

Choose a reason for hiding this comment

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

Since we "support" middle rests with this, I'd expect to support them in non-generic scenarios as well - building on my above comments, I'd expect [number, ...string[]] | [number, ...string[], boolean | undefined] or even [number, ...string[], boolean?] exactly. The inputs here would seem to insist that we shouldn't admit [number, boolean, string], but under the current calculation we do.

type V22 = Tup3<[number], string[], boolean[]>; // [number, (string | boolean)[]]
type V23 = Tup3<[number], string[], [boolean?]>; // [number, (string | boolean | undefined)[]]
type V24 = Tup3<[number], [boolean?], string[]>; // [number, boolean?, ...string[]]
type V25 = Tup3<string[], number[], boolean[]>; // (string | number | boolean)[]
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, again, the resulting type here is much, much looser than the sequence the inputs imply here. Since we support [...A, ...B, ...C] in generic cases, we should support preserving that sequencing in cases without generics. The resulting type here should allow assigning ["ok", 0, true] to it, but not [true, 0, "ok"]. We really should preserve that sequencing information that we're using on generics in the nongeneric cases. Which, in this case, means I think the resulting output type can only be [...string[], ...number[], ...boolean[]]. I liken this to our pattern literal types. This is very much like a ${string}${number}${boolean} - that pattern has meaning in its ordering, and shouldn't be collapsed.

type V23 = Tup3<[number], string[], [boolean?]>; // [number, (string | boolean | undefined)[]]
type V24 = Tup3<[number], [boolean?], string[]>; // [number, boolean?, ...string[]]
type V25 = Tup3<string[], number[], boolean[]>; // (string | number | boolean)[]
type V26 = Tup3<string[], number[], [boolean]>; // [...(string | number)[], boolean]
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm probably going to stop pointing out the "the result is much looser than the inputs imply because we're not preserving multiple spreads in nongeneric cases" - there's a lot of those in these tests, and I think I've said all I need to at this point.

type V24 = Tup3<[number], [boolean?], string[]>; // [number, boolean?, ...string[]]
type V25 = Tup3<string[], number[], boolean[]>; // (string | number | boolean)[]
type V26 = Tup3<string[], number[], [boolean]>; // [...(string | number)[], boolean]
type V27 = Tup3<[number?], [string], [boolean?]>; // [number | undefined, string, boolean?]
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd prefer [number | undefined, string, boolean?] | [string, boolean?] here, similarly to one of my above comments. I'm not a fan of dropping the arity-optionality without reflecting it in the resulting type.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Dec 7, 2020

@weswigham from your comments, it sounds like you want to loosen the restriction in place:

Tuple type layouts are now governed by these rules:

  • An optional element cannot precede a required element or follow a rest element.
  • Multiple rest elements are not permitted.

and add some new behavior for both the generic/non-generic cases.


For optional elements, I tend to somewhat agree. As a user, I would expect that we might desugar

type Foo = [string, string?, number];

into something like

type Foo = [string, number] | [string, string, number];

though not the suggested

type Foo = [string, number] | [string, string | undefined, number];

How often do we leverage the distinction between optional and potentially undefined when we have the property itself?


For multiple rest elements... well, I don't have a strong-enough intuition on that one, but it would be pretty different in behavior. It would line up better with what we have in template string types.

@weswigham
Copy link
Member

it sounds like you want to loosen the restriction in place:

I guess so. Since you can replicate those situations in cases where we do not issue an error (eg, via variadic concatenation), I think having accurate behavior for them is important. And thus, once you have accurate behavior for those situations, there's not much of a justification in forbidding them in normal circumstances, I think.

How often do we leverage the distinction between optional and potentially undefined when we have the property itself?

Infrequently. In fact, that we don't is pretty much one of the reasons for #41418 being a bug and not "intended behavior". So the inclusion of undefined in the type is potentially needed for consistency.

For multiple rest elements...

Yeah, I just reason that sequences of types in an array aren't that different from sequences of characters (or types) in a string (or template) - so the later being strictly more powerful with respect to what is preserved through concatenation operations rubs me the wrong way a bit - I feel like we've done some of the thinking on how to match up these sequences, and should probably try to do the same here, rather than quietly losing type fidelity in an unsafe way.

@ahejlsberg ahejlsberg merged commit 9b17186 into master Jan 5, 2021
PR Backlog automation moved this from Needs review to Done Jan 5, 2021
@ahejlsberg ahejlsberg deleted the fix39595 branch January 5, 2021 01:13
Zzzen pushed a commit to Zzzen/TypeScript that referenced this pull request Jan 16, 2021
* Support starting and middle rest elements in tuples

* Accept new baselines

* Include all rest arguments in error span

* Accept new baselines

* Fix tests

* Add new tests

* Fix lint errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

Support inferring rest args that are not at the end of the function's args
4 participants