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

Proper treatment of splicing tuples in array literals #36861

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

elibarzilay
Copy link
Contributor

Fixes #32465.

src/compiler/checker.ts Outdated Show resolved Hide resolved
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Looks basically right and the baselines improve. I agree with @weswigham that isTupleLikeType is preferable.

src/compiler/checker.ts Show resolved Hide resolved
src/compiler/checker.ts Show resolved Hide resolved
src/compiler/checker.ts Show resolved Hide resolved
src/compiler/checker.ts Outdated Show resolved Hide resolved
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

I think it's a good idea to have the isTupleLike implementation in the PR history but we always Squash & Merge so it should end up as one commit on master anyway.

Fixes microsoft#32465.

After this was done, I continued to extend the implementation to handle
TupleLike types but it'ss broken since JS doesn't allow splicing
TupleLike values into array literals so pulled that out, and instead
keeping it for reference below.  (It Includes tests, which are broken
too.)

modified   src/compiler/checker.ts
@@ -22268,6 +22268,21 @@ namespace ts {
                         else hasNonEndingSpreadElement = true;
                     }
                 }
+                else if (spreadType && isTupleLikeType(spreadType)) {
+                    let i = 0, tupleEltType: Type | undefined;
+                    while (tupleEltType = getTypeOfPropertyOfType(spreadType, "" + i as __String)) {
+                        elementTypes.push(tupleEltType);
+                        i++;
+                    }
+                    const stringIndexInfo = getIndexInfoOfType(spreadType, IndexKind.String);
+                    const numberIndexInfo = getIndexInfoOfType(spreadType, IndexKind.Number);
+                    if (stringIndexInfo || numberIndexInfo) {
+                        if (stringIndexInfo) elementTypes.push(stringIndexInfo.type);
+                        if (numberIndexInfo) elementTypes.push(numberIndexInfo.type);
+                        if (i === elementCount - 1) hasEndingSpreadElement = true;
+                        else hasNonEndingSpreadElement = true;
+                    }
+                }
                 else {
                     if (inDestructuringPattern && spreadType) {
                         // Given the following situation:
new file   tests/cases/compiler/spliceTupleLikesWIntegers.ts
@@ -0,0 +1,23 @@
+declare const sb: { [0]: string, [1]: boolean };
+
+let k1: [number, string, boolean];
+k1 = [1, ...sb];
+
+let k2: [number, string, boolean, number];
+k2 = [1, ...sb, 1];
+
+// declare const sb_: [string, ...boolean[]];
+
+// let k3: [number, string, ...boolean[]];
+// k3 = [1, ...sb_];
+
+// declare const sbb_: [string, boolean, ...boolean[]];
+
+// let k4: [number, string, ...boolean[]];
+// k4 = [1, ...sbb_];
+
+// let k5: [number, string, boolean, ...boolean[]];
+// k5 = [1, ...sbb_];
+
+// let k6: [number, string, boolean, boolean, ...boolean[]];
+// k6 = [1, ...sbb_];
new file   tests/cases/compiler/spliceTupleLikesWStrings.ts
@@ -0,0 +1,23 @@
+declare const sb: { 0: string, 1: boolean };
+
+let k1: [number, string, boolean];
+k1 = [1, ...sb];
+
+let k2: [number, string, boolean, number];
+k2 = [1, ...sb, 1];
+
+declare const sb_: { 0: string, [s: string]: (boolean|string) };
+
+let k3: [number, string, ...(boolean|string)[]];
+k3 = [1, ...sb_];
+
+declare const sbb_: { 0: string, 1: boolean, [s: string]: (boolean|string) };
+
+let k4: [number, string, boolean, ...(boolean|string)[]];
+k4 = [1, ...sbb_];
+
+// let k5: [number, string, boolean, ...(boolean|string)[]];
+// k5 = [1, ...sbb_];
+
+// let k6: [number, string, boolean, boolean, ...(boolean|string)[]];
+// k6 = [1, ...sbb_];
@elibarzilay elibarzilay merged commit e71614a into microsoft:master Feb 27, 2020
@elibarzilay elibarzilay deleted the 32465 branch February 27, 2020 21:43
@dandv
Copy link
Contributor

dandv commented Mar 10, 2020

Thank you for the fix. Would it solve this issue as well?

const date: Date = new Date();
const num = 42;

// OK
const [a, b]: [Date, number] = [date, num];

const args = [date, num];
// Type '(number | Date)[]' is missing the following properties from type '[Date, number]': 0, 1(2739)
const [c, d]: [Date, number] = args;

@sandersn
Copy link
Member

No, that's caused by the lack of contextual typing in const args = [date, num] -- the compiler has no way to know that the contents of [date, num] are immutable, so assumes that they're an array instead of a tuple. You can make it explicit with [date, num] as const; we've prototyped making that the default, but it Breaks Everything when we do.

@WeiShengv99
Copy link

No, that's caused by the lack of contextual typing in const args = [date, num] -- the compiler has no way to know that the contents of [date, num] are immutable, so assumes that they're an array instead of a tuple. You can make it explicit with [date, num] as const; we've prototyped making that the default, but it Breaks Everything when we do.

i dont know what happend in this case , is this the same problem?
https://www.typescriptlang.org/play?ts=3.9.2#code/MYewdgzgLgBApgGzgWzmKAuGBtMBXZAIzgCcAaGfI0gXRgF4cBmCgFjoG4AoL0SWAGYgQDGAAoAhlirFyMQtIKyKwRdTkATNbICUDAHwwA3jwC+3APQWYQkGIB0jsdgCMFR-cQo0sCkzoSEDB80Do6lta2Ym4sHl6o6OFcVjbC0WRxSAlQZEzhQA

@elibarzilay
Copy link
Contributor Author

@GRJser, no -- this was only about splicing into array literals. There is this PR that fixes some spreads in calls and might be relevant for your example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

typescript can't infer destructuring of array properly
6 participants