Skip to content

Commit 004f18f

Browse files
rbucktonrbuckton
authored
Merge pull request microsoft#21219 from Microsoft/fix19020
Fix destructuring assignment when assignment target is RHS
2 parents 61fb845 + 18a2fc8 commit 004f18f

10 files changed

Lines changed: 121 additions & 8 deletions

File tree

‎Gulpfile.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ gulp.task(tsserverLibraryFile, /*help*/ false, [servicesFile, typesMapJson], (do
528528
const serverLibraryProject = tsc.createProject("src/server/tsconfig.library.json", getCompilerSettings({ removeComments: false }, /*useBuiltCompiler*/ true));
529529
const {js, dts}: { js: NodeJS.ReadableStream, dts: NodeJS.ReadableStream } = serverLibraryProject.src()
530530
.pipe(sourcemaps.init())
531-
.pipe(newer(tsserverLibraryFile))
531+
.pipe(newer(<any>{ dest: tsserverLibraryFile, extra: ["src/compiler/**/*.ts", "src/services/**/*.ts"] }))
532532
.pipe(serverLibraryProject());
533533

534534
return merge2([

‎src/compiler/transformers/destructuring.ts‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,13 @@ namespace ts {
7070

7171
if (value) {
7272
value = visitNode(value, visitor, isExpression);
73-
if (needsValue) {
73+
74+
if (isIdentifier(value) && bindingOrAssignmentElementAssignsToName(node, value.escapedText)) {
75+
// If the right-hand value of the assignment is also an assignment target then
76+
// we need to cache the right-hand value.
77+
value = ensureIdentifier(flattenContext, value, /*reuseIdentifierExpressions*/ false, location);
78+
}
79+
else if (needsValue) {
7480
// If the right-hand value of the destructuring assignment needs to be preserved (as
7581
// is the case when the destructuring assignment is part of a larger expression),
7682
// then we need to cache the right-hand value.
@@ -123,6 +129,27 @@ namespace ts {
123129
}
124130
}
125131

132+
function bindingOrAssignmentElementAssignsToName(element: BindingOrAssignmentElement, escapedName: __String): boolean {
133+
const target = getTargetOfBindingOrAssignmentElement(element);
134+
if (isBindingOrAssignmentPattern(target)) {
135+
return bindingOrAssignmentPatternAssignsToName(target, escapedName);
136+
}
137+
else if (isIdentifier(target)) {
138+
return target.escapedText === escapedName;
139+
}
140+
return false;
141+
}
142+
143+
function bindingOrAssignmentPatternAssignsToName(pattern: BindingOrAssignmentPattern, escapedName: __String): boolean {
144+
const elements = getElementsOfBindingOrAssignmentPattern(pattern);
145+
for (const element of elements) {
146+
if (bindingOrAssignmentElementAssignsToName(element, escapedName)) {
147+
return true;
148+
}
149+
}
150+
return false;
151+
}
152+
126153
/**
127154
* Flattens a VariableDeclaration or ParameterDeclaration to one or more variable declarations.
128155
*
@@ -157,6 +184,17 @@ namespace ts {
157184
createArrayBindingOrAssignmentElement: makeBindingElement,
158185
visitor
159186
};
187+
188+
if (isVariableDeclaration(node)) {
189+
let initializer = getInitializerOfBindingOrAssignmentElement(node);
190+
if (initializer && isIdentifier(initializer) && bindingOrAssignmentElementAssignsToName(node, initializer.escapedText)) {
191+
// If the right-hand value of the assignment is also an assignment target then
192+
// we need to cache the right-hand value.
193+
initializer = ensureIdentifier(flattenContext, initializer, /*reuseIdentifierExpressions*/ false, initializer);
194+
node = updateVariableDeclaration(node, node.name, node.type, initializer);
195+
}
196+
}
197+
160198
flattenBindingOrAssignmentElement(flattenContext, node, rval, node, skipInitializer);
161199
if (pendingExpressions) {
162200
const temp = createTempVariable(/*recordTempVariable*/ undefined);

‎src/compiler/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1467,7 +1467,7 @@ namespace ts {
14671467
| SpreadAssignment // AssignmentRestProperty
14681468
;
14691469

1470-
export type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Expression;
1470+
export type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Identifier | PropertyAccessExpression | ElementAccessExpression | OmittedExpression;
14711471

14721472
export type ObjectBindingOrAssignmentPattern
14731473
= ObjectBindingPattern

‎tests/baselines/reference/api/tsserverlibrary.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ declare namespace ts {
877877
type DestructuringAssignment = ObjectDestructuringAssignment | ArrayDestructuringAssignment;
878878
type BindingOrAssignmentElement = VariableDeclaration | ParameterDeclaration | BindingElement | PropertyAssignment | ShorthandPropertyAssignment | SpreadAssignment | OmittedExpression | SpreadElement | ArrayLiteralExpression | ObjectLiteralExpression | AssignmentExpression<EqualsToken> | Identifier | PropertyAccessExpression | ElementAccessExpression;
879879
type BindingOrAssignmentElementRestIndicator = DotDotDotToken | SpreadElement | SpreadAssignment;
880-
type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Expression;
880+
type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Identifier | PropertyAccessExpression | ElementAccessExpression | OmittedExpression;
881881
type ObjectBindingOrAssignmentPattern = ObjectBindingPattern | ObjectLiteralExpression;
882882
type ArrayBindingOrAssignmentPattern = ArrayBindingPattern | ArrayLiteralExpression;
883883
type AssignmentPattern = ObjectLiteralExpression | ArrayLiteralExpression;

‎tests/baselines/reference/api/typescript.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ declare namespace ts {
877877
type DestructuringAssignment = ObjectDestructuringAssignment | ArrayDestructuringAssignment;
878878
type BindingOrAssignmentElement = VariableDeclaration | ParameterDeclaration | BindingElement | PropertyAssignment | ShorthandPropertyAssignment | SpreadAssignment | OmittedExpression | SpreadElement | ArrayLiteralExpression | ObjectLiteralExpression | AssignmentExpression<EqualsToken> | Identifier | PropertyAccessExpression | ElementAccessExpression;
879879
type BindingOrAssignmentElementRestIndicator = DotDotDotToken | SpreadElement | SpreadAssignment;
880-
type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Expression;
880+
type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Identifier | PropertyAccessExpression | ElementAccessExpression | OmittedExpression;
881881
type ObjectBindingOrAssignmentPattern = ObjectBindingPattern | ObjectLiteralExpression;
882882
type ArrayBindingOrAssignmentPattern = ArrayBindingPattern | ArrayLiteralExpression;
883883
type AssignmentPattern = ObjectLiteralExpression | ArrayLiteralExpression;

‎tests/baselines/reference/destructuringReassignsRightHandSide.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//// [destructuringReassignsRightHandSide.ts]
2+
var foo: any = { foo: 1, bar: 2 };
3+
var bar: any;
4+
5+
// reassignment in destructuring pattern
6+
({ foo, bar } = foo);
7+
8+
// reassignment in subsequent var
9+
var { foo, baz } = foo;
10+
11+
//// [destructuringReassignsRightHandSide.js]
12+
var foo = { foo: 1, bar: 2 };
13+
var bar;
14+
// reassignment in destructuring pattern
15+
(_a = foo, foo = _a.foo, bar = _a.bar);
16+
// reassignment in subsequent var
17+
var _b = foo, foo = _b.foo, baz = _b.baz;
18+
var _a;

‎tests/baselines/reference/destructuringReassignsRightHandSide.symbols‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
=== tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts ===
2+
var foo: any = { foo: 1, bar: 2 };
3+
>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 3), Decl(destructuringReassignsRightHandSide.ts, 7, 5))
4+
>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 16))
5+
>bar : Symbol(bar, Decl(destructuringReassignsRightHandSide.ts, 0, 24))
6+
7+
var bar: any;
8+
>bar : Symbol(bar, Decl(destructuringReassignsRightHandSide.ts, 1, 3))
9+
10+
// reassignment in destructuring pattern
11+
({ foo, bar } = foo);
12+
>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 4, 2))
13+
>bar : Symbol(bar, Decl(destructuringReassignsRightHandSide.ts, 4, 7))
14+
>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 3), Decl(destructuringReassignsRightHandSide.ts, 7, 5))
15+
16+
// reassignment in subsequent var
17+
var { foo, baz } = foo;
18+
>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 3), Decl(destructuringReassignsRightHandSide.ts, 7, 5))
19+
>baz : Symbol(baz, Decl(destructuringReassignsRightHandSide.ts, 7, 10))
20+
>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 3), Decl(destructuringReassignsRightHandSide.ts, 7, 5))
21+

‎tests/baselines/reference/destructuringReassignsRightHandSide.types‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts ===
2+
var foo: any = { foo: 1, bar: 2 };
3+
>foo : any
4+
>{ foo: 1, bar: 2 } : { foo: number; bar: number; }
5+
>foo : number
6+
>1 : 1
7+
>bar : number
8+
>2 : 2
9+
10+
var bar: any;
11+
>bar : any
12+
13+
// reassignment in destructuring pattern
14+
({ foo, bar } = foo);
15+
>({ foo, bar } = foo) : any
16+
>{ foo, bar } = foo : any
17+
>{ foo, bar } : { foo: any; bar: any; }
18+
>foo : any
19+
>bar : any
20+
>foo : any
21+
22+
// reassignment in subsequent var
23+
var { foo, baz } = foo;
24+
>foo : any
25+
>baz : any
26+
>foo : any
27+

‎tests/baselines/reference/objectRest.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,10 @@ var i = removable;
8585
var { removed } = i, removableRest2 = __rest(i, ["removed"]);
8686
let computed = 'b';
8787
let computed2 = 'a';
88-
var _g = computed, stillNotGreat = o[_g], _h = computed2, soSo = o[_h], o = __rest(o, [typeof _g === "symbol" ? _g : _g + "", typeof _h === "symbol" ? _h : _h + ""]);
89-
(_j = computed, stillNotGreat = o[_j], _k = computed2, soSo = o[_k], o = __rest(o, [typeof _j === "symbol" ? _j : _j + "", typeof _k === "symbol" ? _k : _k + ""]));
88+
var _g = o, _h = computed, stillNotGreat = _g[_h], _j = computed2, soSo = _g[_j], o = __rest(_g, [typeof _h === "symbol" ? _h : _h + "", typeof _j === "symbol" ? _j : _j + ""]);
89+
(_k = o, _l = computed, stillNotGreat = _k[_l], _m = computed2, soSo = _k[_m], o = __rest(_k, [typeof _l === "symbol" ? _l : _l + "", typeof _m === "symbol" ? _m : _m + ""]));
9090
var noContextualType = (_a) => {
9191
var { aNumber = 12 } = _a, notEmptyObject = __rest(_a, ["aNumber"]);
9292
return aNumber + notEmptyObject.anythingGoes;
9393
};
94-
var _d, _f, _j, _k;
94+
var _d, _f, _k, _l, _m;

‎tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// @target: es5
2+
var foo: any = { foo: 1, bar: 2 };
3+
var bar: any;
4+
5+
// reassignment in destructuring pattern
6+
({ foo, bar } = foo);
7+
8+
// reassignment in subsequent var
9+
var { foo, baz } = foo;

0 commit comments

Comments
 (0)