Support naming tuple members #38234

weswigham · 2020-04-28T21:05:30Z

This PR allows tuple members to have names like so:

type Segment = [length: number, count: number]

The optional marker and variadic markers (? and ...) are expected in the same places as the parameter lists. I already have code in place to gracefully parse a ? or ... in the type and issue an error. Parameter lists automatically make named tuples (in fact, there is no new checker machinery for trafficking the names for named tuples, since it was already in place for parameter lists - mostly just new parser/emitter code). Names do not affect assignability in any way, and just exist for documentation and quickinfo.

weswigham · 2020-04-28T21:11:01Z

tests/baselines/reference/namedTupleMembersErrors.errors.txt

+==== tests/cases/conformance/types/tuple/named/namedTupleMembersErrors.ts (5 errors) ====
+    export type Segment1 = [length: number, number]; // partially named, disallowed
+                                            ~~~~~~
+!!! error TS5084: Tuple members must all have names or not have names.


@sandersn @DanielRosenwasser 🚲 🏚️ - I rewrote this message like 3 times and they all sound awkward. I want to say something like Tuple member namedness must be homogeneous but that's maybe also bad.

Tuple members must all be named or anonymous.

or

All members of a tuple must either be named or anonymous.

or

Named tuple members cannot be mixed with anonymous tuple members.

But you might have to play where you issue the error depending on which one you pick. Related errors can help too as a learning device for what an "anonymous" member is.

"Either all or no tuple member can be named"

or

"Naming of tuples is only supported if all members are named."

or

"Either all or no tuple members need to be named"

weswigham · 2020-04-28T21:21:13Z

src/compiler/types.ts

@@ -1273,7 +1275,15 @@ namespace ts {

    export interface TupleTypeNode extends TypeNode {
        kind: SyntaxKind.TupleType;
-        elementTypes: NodeArray<TypeNode>;
+        elements: NodeArray<TypeNode | NamedTupleMember>;


In the end, I made NamedTupleMember a TypeNode, which is inline with OptionalType and RestType, so I don't have to rename this field (or even change the type). If anyone feels strongly about it, I can change it back. However renaming it was very useful for figuring out where I needed to adjust/handle the new node; so "breaking" the API because of the new child kind might be worthwhile for other consumers, too. Depends on how strongly we want to maintain AST compatibility I suppose.

I like the rename.

This rename is missing in the API breaking changes for 4.0 - https://github.com/microsoft/TypeScript/wiki/API-Breaking-Changes

weswigham · 2020-04-28T21:30:18Z

@typescript-bot pack this

@orta @DanielRosenwasser can either of you think of any extra language service features that these names might need hooking up to? I haven't implemented it yet, but I figure trafficking doc comments along with the names might also be useful for quick info. So, eg, when you have

if you tupleize it, you can retain the same quickinfo experience, rather than it being missing, since today it is:

Though admittedly, that's not directly related to this and could be a separate feature request!

typescript-bot · 2020-04-28T21:30:23Z

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

DanielRosenwasser · 2020-04-28T21:35:43Z

It'd be interesting to see how JSDoc plays into a lot of this. Not sure what tests you want, but quick info and signature help on these examples might be useful.

/**
 * @param args {[a: string, b: number]} the params
 */
function foo(...args) {
  let [a, b] = args;
}

foo()

/**
 * @param args {[a: string, b: number]} the params
 */
function bar(...[a, b]) {
}

bar()

typescript-bot · 2020-04-28T21:46:15Z

Hey @weswigham, 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/72444/artifacts?artifactName=tgz&fileId=2D62822DA042EC2DA13BEE472478F4D5575F9C36062F7718D8375C60BB11C2B002&fileName=/typescript-4.0.0-insiders.20200428.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

TimvdLippe · 2020-04-29T19:45:24Z

Incidentally, this also fixes a small bug do do with jsdoc variadic types and declaration emit (which is as far as I can tell, was unreported, but was reflected in our type baselines, which I noticed while I was adjusting tuple whitespace emit to support preserving comments/multiline-ness).

I tried 3.9.1-rc on DevTools today and ran into that issue: #38242 Glad to see a fix in progress!

weswigham · 2020-05-01T18:56:11Z

From design meeting: Better tuple language service experiences across the board would be nice:

Element completions (that mention tuple name) eg: 0 (property) first (also relevant to non-labeled tuple members)
Teasing apart unions of rest tuples into separate signature help overloads
Quickfix for grammar errors on ... and ? in name
Refactoring for extracting a parameter list as a tuple type/set of overload parameter lists as a tuple type
Try to preserve doc comments with the tuple on the best effort basis

weswigham · 2020-05-12T01:06:56Z

@typescript-bot pack this

The syntax has been swapped to the parameter-like one, and I've got most of what we want in-place now (so should be usable to test). Still todo:

Teasing apart unions of rest tuples into separate signature help overloads
More tests for the refactoring
- Better parameter comment preservation for the refactoring
More tests in general, just for increased coverage (codifying the behavior of recursive aliases, generics, and such, while I know it works, should probably be in a test, and it's probably worth sanity checking the assignability relationships involving tuples with labels, even though I didn't really touch the existing implementation)

typescript-bot · 2020-05-12T01:07:03Z

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

typescript-bot · 2020-05-12T01:22:14Z

Hey @weswigham, 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/73632/artifacts?artifactName=tgz&fileId=6E6817DF35333947E853CF063D39FB4BE556988D7B92F809B8CD718CE706059602&fileName=/typescript-4.0.0-insiders.20200512.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

…played as seperate entries now

weswigham · 2020-05-14T21:44:02Z

@sandersn @ahejlsberg @rbuckton @andrewbranch reviews and feedback whenever you're ready would be appreciated now ❤️

weswigham · 2020-05-14T21:45:00Z

It shouldn't affect much, mostly just being a new syntax feature, but

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

typescript-bot · 2020-05-14T21:45:03Z

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

typescript-bot · 2020-05-14T21:45:03Z

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

typescript-bot · 2020-05-14T21:45:03Z

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

typescript-bot · 2020-05-14T21:45:03Z

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

typescript-bot · 2020-05-14T21:58:28Z

Hey @weswigham, 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/73966/artifacts?artifactName=tgz&fileId=31DDF907A9FA5808E09BD99C8A1AC55236B525EFE19354549F6E4ECF150B754F02&fileName=/typescript-4.0.0-insiders.20200514.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

typescript-bot · 2020-05-14T22:13:36Z

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

andrewbranch · 2020-05-14T22:17:30Z

src/compiler/types.ts

+        kind: SyntaxKind.NamedTupleMember;
+        dotDotDotToken?: Token<SyntaxKind.DotDotDotToken>;
+        name: Identifier;
+        questionToken?: Token<SyntaxKind.QuestionToken>;


General design question: how do you decide between saving a reference to a token like this vs. saving a boolean property that indicates optionality, like ImportTypeNode['isTypeOf']? Is this token actually used anywhere?

In this case, mostly so the members of the same name are the same type as ParameterDeclaration. In the case of ParameterDeclaration, so comments can more easily be collected on the intervening tokens (iirc). (Types don't normally care about comment preservation on intervening tokens)

andrewbranch · 2020-05-14T22:22:09Z

src/services/codefixes/fixIncorrectNamedTupleSyntax.ts

+            else if (unwrappedType.kind === SyntaxKind.RestType) {
+                sawRest = true;
+            }
+            unwrappedType = (unwrappedType as OptionalTypeNode | RestTypeNode | ParenthesizedTypeNode).type;


Nit / nagging question: any reason not to use isOptionalType and friends in the while so as to avoid this type assertion?

Uhh, mostly just because iirc one of them doesn't exist. Yeah, I think there isn't a isRestType and mixing and matching looked odd.

andrewbranch · 2020-05-14T22:42:27Z

src/compiler/checker.ts

                                }
-                                const tupleTypeNode = createTupleTypeNode(tupleConstituentNodes);
+                                const tupleTypeNode = setEmitFlags(createTupleTypeNode(tupleConstituentNodes), EmitFlags.SingleLine);


How do you know single-line is a reasonable choice from here? It almost seems like single-line ought to remain the default for tuples, but I guess that would have meant introducing a new EmitFlag (which we’re running out of space for).

Exactly that. Unlike object types, tuples have hisorically only been pretty printed single line, however we only have a flag for single line. So in many cases, I now detect if the input is single line and add the single line flag, so I can use it's absence as the multiline case. In this location specifically, I have no reason to change it to multiline (which is now the "default", lacking any emit flags), so it stays single line.

andrewbranch · 2020-05-14T23:04:37Z

src/compiler/checker.ts

@@ -9520,27 +9541,36 @@ namespace ts {
            return result;
        }

-        function getExpandedParameters(sig: Signature): readonly Symbol[] {
+        function getExpandedParameters(sig: Signature, skipUnionExpanding?: boolean): readonly (readonly Symbol[])[] {


I understand this change on its surface, but I don’t immediately understand why it was necessary. Can you point me to the test case(s) where this matters?

It returns of array of arrays so it can return, to the language service, a list of "psuedooverloads" - one for each rest union tuple member. The flag exists to disable that behavior for node serialization, where we'd like to keep it as the single overload.

Got it, so this only comes into play in signature help (and possibly quick info or whatever)?

weswigham · 2020-05-14T23:16:34Z

Fixed a crash the user suite found (a parameter without name is apparently a thing which can happen, even though our types disallow it?), the rest of the user suite looks pretty good - just named tuples in error messages. RWC diff is just a tuple in declaration emit being multiline (since now we can preserve that). DT diff is currently filled with failures due to dropping 2.9 support, but the few failures not related to that are places where tuples acquired named and thus no longer match ExpectType calls, so are also good (though should probably be preemptively updated to accept the new labeled printback, once we can get a full list).

Perf test died early with a crash in npm itself... looks like it's running an old version of npm. @rbuckton is it safe to update the npm version in use on the perf testing machine (and if so, would you be able to)? It's on 6.7.0, while hosted workers are working fine on 6.14.4.

weswigham · 2020-05-15T00:29:17Z

@typescript-bot run dt again since the 2.9 thing should be fixed now

typescript-bot · 2020-05-15T00:29:25Z

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

ahejlsberg

Looks good. The one thing I wonder about is whether it is better to omit tuple element names in certain error cases (see my review comment). I worry we'll confuse people between the actual names ('0', '1', etc.) and the element names.

ahejlsberg · 2020-05-19T17:32:06Z

src/compiler/checker.ts

-        function getTupleTypeOfArity(arity: number, minLength: number, hasRestElement: boolean, readonly: boolean, associatedNames?: __String[]): GenericType {
-            const key = arity + (hasRestElement ? "+" : ",") + minLength + (readonly ? "R" : "") + (associatedNames && associatedNames.length ? "," + associatedNames.join(",") : "");
+        function getTupleTypeOfArity(arity: number, minLength: number, hasRestElement: boolean, readonly: boolean, namedMemberDeclarations?: readonly (NamedTupleMember | ParameterDeclaration)[]): GenericType {
+            const key = arity + (hasRestElement ? "+" : ",") + minLength + (readonly ? "R" : "") + (namedMemberDeclarations && namedMemberDeclarations.length ? "," + map(namedMemberDeclarations, getNodeId).join(",") : "");


I like using node IDs here, though it will increase the number of unique tuple target types we generate.

Yeah. Using node id here brings tuples in line with normal anonymous object types. It gets us good name and documentation preservation, though!

ahejlsberg · 2020-05-19T17:32:49Z

src/compiler/checker.ts

@@ -13531,7 +13573,7 @@ namespace ts {

        function getAliasSymbolForTypeNode(node: Node) {
            let host = node.parent;
-            while (isParenthesizedTypeNode(host) || isTypeOperatorNode(host) && host.operator === SyntaxKind.ReadonlyKeyword) {
+            while (isParenthesizedTypeNode(host) || host.kind === SyntaxKind.NamedTupleMember || isTypeOperatorNode(host) && host.operator === SyntaxKind.ReadonlyKeyword) {


Can't think of when a named tuple member would occur in the parent chain?

This is mostly a leftover from an initial stage where I considered labels a kind of parenthesized declaration. I must have missed removing this in the cleanup.

ahejlsberg · 2020-05-19T17:49:05Z

src/compiler/checker.ts

@@ -25117,19 +25176,23 @@ namespace ts {
                }
            }
            const types = [];
+            const names: (ParameterDeclaration | NamedTupleMember)[] = [];


What's the scenario for capturing names here?

When you concatenate one tuple to another via spread, this allows us to concatenate the name list, too.

ahejlsberg · 2020-05-19T17:55:48Z

src/compiler/checker.ts

            for (let i = pos; i < nonRestCount; i++) {
                types.push(getTypeAtPosition(source, i));
-                names.push(getParameterNameAtPosition(source, i));
+                const name = getNameableDeclarationAtPosition(source, i);


So previously we'd always collect names (even made up arg_xxx names), but now we only collect names with an actual declaration associated?

Correct! That does mean that generated names can still be shoved into parameter lists (like arg_0) but will disappear when copied into a tuple. I actually think this is a good thing, since the generated names were just positional anyway.

ahejlsberg · 2020-05-19T17:59:33Z

src/compiler/checker.ts

+                    seenNamedElement = true;
+                }
+                else if (seenNamedElement) {
+                    grammarErrorOnNode(e, Diagnostics.Tuple_members_must_all_have_names_or_not_have_names);


How about "Tuple members must all have names or all not have names". As it is now, you can read it as all members must have a name or not, which obviously is always true.

ahejlsberg · 2020-05-19T18:01:14Z

src/compiler/types.ts

@@ -1273,7 +1275,15 @@ namespace ts {

    export interface TupleTypeNode extends TypeNode {
        kind: SyntaxKind.TupleType;
-        elementTypes: NodeArray<TypeNode>;
+        elements: NodeArray<TypeNode | NamedTupleMember>;


I like the rename.

ahejlsberg · 2020-05-19T18:10:11Z

tests/baselines/reference/genericRestParameters3.errors.txt

-    Type '[string] | [number, boolean]' is not assignable to type '[number, boolean]'.
-      Property '1' is missing in type '[string]' but required in type '[number, boolean]'.
+    Type '[string] | [number, boolean]' is not assignable to type '[y: number, z: boolean]'.
+      Property '1' is missing in type '[string]' but required in type '[y: number, z: boolean]'.


Hmm, the actual property name is '1', but it sure looks like it's name is z. Wonder if it is better to omit the names in some cases?

Eh... I think it'd be kinda odd to sometimes display the same type with labels and sometimes without. Plus, this error is somewhat rare; I believe we usually try to report tuple length errors as an arity mismatch (ie, on length), but fail to do so here because of the union.

How about

Type '[string] | [number, boolean]' is not assignable to type '[y: number, z: boolean]'. Property '1' (labeled as 'z') is missing in type '[string]' but required in type '[y: number, z: boolean]'.

Sorry for the late question, I was super excited for this feature as I want to use it now (and get the benefit in future as Angular updates it's TS dependency)

weswigham · 2020-05-19T21:20:11Z

@typescript-bot run dt

typescript-bot · 2020-05-19T21:20:12Z

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

* upstream/master: Support naming tuple members (microsoft#38234) LEGO: check in for master to temporary branch. fix: extract const in jsx (microsoft#37912) No contextual types from circular mapped type properties (microsoft#38653) Ensure formatter can always get a newline character (microsoft#38579) Fix debug command for Node debugging Remove mentions of runtests-browser in CONTRIBUTING.md fix(33233): add outlining for comments before property access expression regression(38485): allow using rawText property in processing a tagged template

ftzi · 2020-09-24T00:59:03Z

This new feature looks like is just what I needed right now. But, I do not know how to connect the dots:

In summary, I have a readonly array of const strings, like readonly ['aaa', 'bbb', 'ccc']. I can't find a way (if there is one) to transform this into a generic variadic function parameter type, like [aaa: string | number, bbb: string | number, ccc: string | number].

Gerrit0 · 2020-09-24T03:27:26Z

There is no such functionality in this PR (or, as far as I know, planned). Tuple member names are purely documentation... they don't interact with the rest of the type system in any way.

ftzi · 2020-09-24T04:06:25Z

Hmm, that's bad. Would be a really cool feature to allow "dynamic functions"

typescript-bot assigned weswigham Apr 28, 2020

typescript-bot added the Author: Team label Apr 28, 2020

weswigham added the Experiment A fork with an experimental idea which might not make it into master label Apr 28, 2020

weswigham commented Apr 28, 2020

View reviewed changes

weswigham requested review from sandersn, rbuckton, DanielRosenwasser and ahejlsberg April 28, 2020 21:12

weswigham commented Apr 28, 2020

View reviewed changes

weswigham mentioned this pull request Apr 30, 2020

Fix jsdoc variadic type nodes not being remapped to equivalent TS in output #38276

Merged

DanielRosenwasser mentioned this pull request May 1, 2020

Design Meeting Notes, 5/1/2020 #38292

Closed

weswigham force-pushed the named-tuple-members branch from c80c84a to e1c5989 Compare May 7, 2020 00:41

weswigham added 5 commits May 11, 2020 14:31

Initial draft of named tuple members

086e01f

Show tuple labels and documentation in completions

4c50574

Swap allowed syntax to parameter-like

c7e115a

Add quickfix for labeled tuple syntax mistakes

6b754a9

Add refactoring to convert list of signatures to single overload

ac57f0a

weswigham force-pushed the named-tuple-members branch from e1c5989 to ac57f0a Compare May 12, 2020 00:54

Fix small bug in visitor verification

f0cf8d4

weswigham added 2 commits May 12, 2020 14:39

Signature help for rest parameters which are unions of tuples are dis…

71b3f04

…played as seperate entries now

Expand sanity check test cases in conformance suite

fcb324d

weswigham requested a review from andrewbranch May 14, 2020 21:43

typescript-bot mentioned this pull request May 14, 2020

🤖 User test baselines have changed for named-tuple-members typescript-bot/TypeScript#36

Open

Handle missing parameter named in isValidDeclarationForTupleLabel

e3a1cb1

andrewbranch approved these changes May 14, 2020

View reviewed changes

ahejlsberg approved these changes May 19, 2020

View reviewed changes

Minor text fixes

9adfb14

weswigham mentioned this pull request May 19, 2020

Add alternative labeled tuple printback for parameter capture lists expected in tests DefinitelyTyped/DefinitelyTyped#44897

Merged

Merge branch 'master' into named-tuple-members

05d398e

weswigham merged commit 5f597e6 into microsoft:master May 19, 2020

sheetalkamat added a commit to microsoft/TypeScript-TmLanguage that referenced this pull request May 21, 2020

Support for named tuple added as part of microsoft/TypeScript#38234

fa4e0d3

Tyler-Murphy mentioned this pull request Aug 2, 2020

Extended TypeScript support choojs/nanobus#41

Merged

jcalz mentioned this pull request Feb 26, 2021

Make calling "pseudo-overloads" behave more like calling true overloads #42987

Open

5 tasks

github-actions bot mentioned this pull request Oct 4, 2022

[Korean] Fix weird expressions like '_제네릭_' microsoft/TypeScript-Website-Localizations#171

Merged

Support naming tuple members #38234

Support naming tuple members #38234

Conversation

weswigham commented Apr 28, 2020 • edited Loading

Choose a reason for hiding this comment

DanielRosenwasser Apr 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weswigham commented Apr 28, 2020

typescript-bot commented Apr 28, 2020 • edited Loading

DanielRosenwasser commented Apr 28, 2020

typescript-bot commented Apr 28, 2020 • edited Loading

TimvdLippe commented Apr 29, 2020

weswigham commented May 1, 2020 • edited Loading

weswigham commented May 12, 2020 • edited Loading

typescript-bot commented May 12, 2020 • edited Loading

typescript-bot commented May 12, 2020 • edited Loading

weswigham commented May 14, 2020

weswigham commented May 14, 2020

typescript-bot commented May 14, 2020 • edited Loading

typescript-bot commented May 14, 2020 • edited Loading

typescript-bot commented May 14, 2020 • edited Loading

typescript-bot commented May 14, 2020 • edited Loading

typescript-bot commented May 14, 2020 • edited Loading

typescript-bot commented May 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weswigham commented May 14, 2020 • edited Loading

weswigham commented May 15, 2020

typescript-bot commented May 15, 2020 • edited Loading

ahejlsberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weswigham commented May 19, 2020

typescript-bot commented May 19, 2020 • edited Loading

ftzi commented Sep 24, 2020 • edited Loading

Gerrit0 commented Sep 24, 2020

ftzi commented Sep 24, 2020

weswigham commented Apr 28, 2020 •

edited

Loading

DanielRosenwasser Apr 28, 2020 •

edited

Loading

typescript-bot commented Apr 28, 2020 •

edited

Loading

typescript-bot commented Apr 28, 2020 •

edited

Loading

weswigham commented May 1, 2020 •

edited

Loading

weswigham commented May 12, 2020 •

edited

Loading

typescript-bot commented May 12, 2020 •

edited

Loading

typescript-bot commented May 12, 2020 •

edited

Loading

typescript-bot commented May 14, 2020 •

edited

Loading

typescript-bot commented May 14, 2020 •

edited

Loading

typescript-bot commented May 14, 2020 •

edited

Loading

typescript-bot commented May 14, 2020 •

edited

Loading

typescript-bot commented May 14, 2020 •

edited

Loading

weswigham commented May 14, 2020 •

edited

Loading

typescript-bot commented May 15, 2020 •

edited

Loading

typescript-bot commented May 19, 2020 •

edited

Loading

ftzi commented Sep 24, 2020 •

edited

Loading