From 4aca0c81218a9623e7629dbc37ef764bf7494dd0 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 16 Jan 2018 16:58:56 -0800 Subject: [PATCH 1/2] Fix destructuring assignment when assignment target is RHS --- src/compiler/transformers/destructuring.ts | 40 ++++++++++++++++++- src/compiler/types.ts | 2 +- tests/baselines/reference/api/typescript.d.ts | 2 +- .../destructuringReassignsRightHandSide.js | 18 +++++++++ ...estructuringReassignsRightHandSide.symbols | 21 ++++++++++ .../destructuringReassignsRightHandSide.types | 27 +++++++++++++ tests/baselines/reference/objectRest.js | 6 +-- .../destructuringReassignsRightHandSide.ts | 9 +++++ 8 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/destructuringReassignsRightHandSide.js create mode 100644 tests/baselines/reference/destructuringReassignsRightHandSide.symbols create mode 100644 tests/baselines/reference/destructuringReassignsRightHandSide.types create mode 100644 tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts diff --git a/src/compiler/transformers/destructuring.ts b/src/compiler/transformers/destructuring.ts index a283275d066..bafe43b0b15 100644 --- a/src/compiler/transformers/destructuring.ts +++ b/src/compiler/transformers/destructuring.ts @@ -70,7 +70,13 @@ namespace ts { if (value) { value = visitNode(value, visitor, isExpression); - if (needsValue) { + + if (isIdentifier(value) && bindingOrAssignmentElementAssignsToName(node, value.escapedText)) { + // If the right-hand value of the assignment is also an assignment target then + // we need to cache the right-hand value. + value = ensureIdentifier(flattenContext, value, /*reuseIdentifierExpressions*/ false, location); + } + else if (needsValue) { // If the right-hand value of the destructuring assignment needs to be preserved (as // is the case when the destructuring assignment is part of a larger expression), // then we need to cache the right-hand value. @@ -123,6 +129,27 @@ namespace ts { } } + function bindingOrAssignmentElementAssignsToName(element: BindingOrAssignmentElement, escapedName: __String): boolean { + const target = getTargetOfBindingOrAssignmentElement(element); + if (isBindingOrAssignmentPattern(target)) { + return bindingOrAssignmentPatternAssignsToName(target, escapedName); + } + else if (isIdentifier(target)) { + return target.escapedText === escapedName; + } + return false; + } + + function bindingOrAssignmentPatternAssignsToName(pattern: BindingOrAssignmentPattern, escapedName: __String): boolean { + const elements = getElementsOfBindingOrAssignmentPattern(pattern); + for (const element of elements) { + if (bindingOrAssignmentElementAssignsToName(element, escapedName)) { + return true; + } + } + return false; + } + /** * Flattens a VariableDeclaration or ParameterDeclaration to one or more variable declarations. * @@ -157,6 +184,17 @@ namespace ts { createArrayBindingOrAssignmentElement: makeBindingElement, visitor }; + + if (isVariableDeclaration(node)) { + let initializer = getInitializerOfBindingOrAssignmentElement(node); + if (initializer && isIdentifier(initializer) && bindingOrAssignmentElementAssignsToName(node, initializer.escapedText)) { + // If the right-hand value of the assignment is also an assignment target then + // we need to cache the right-hand value. + initializer = ensureIdentifier(flattenContext, initializer, /*reuseIdentifierExpressions*/ false, initializer); + node = updateVariableDeclaration(node, node.name, node.type, initializer); + } + } + flattenBindingOrAssignmentElement(flattenContext, node, rval, node, skipInitializer); if (pendingExpressions) { const temp = createTempVariable(/*recordTempVariable*/ undefined); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 7bac69d4a77..9d970b88197 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1467,7 +1467,7 @@ namespace ts { | SpreadAssignment // AssignmentRestProperty ; - export type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Expression; + export type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Identifier | PropertyAccessExpression | ElementAccessExpression | OmittedExpression; export type ObjectBindingOrAssignmentPattern = ObjectBindingPattern diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 45ad8135e2b..ce90fef044e 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -877,7 +877,7 @@ declare namespace ts { type DestructuringAssignment = ObjectDestructuringAssignment | ArrayDestructuringAssignment; type BindingOrAssignmentElement = VariableDeclaration | ParameterDeclaration | BindingElement | PropertyAssignment | ShorthandPropertyAssignment | SpreadAssignment | OmittedExpression | SpreadElement | ArrayLiteralExpression | ObjectLiteralExpression | AssignmentExpression | Identifier | PropertyAccessExpression | ElementAccessExpression; type BindingOrAssignmentElementRestIndicator = DotDotDotToken | SpreadElement | SpreadAssignment; - type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Expression; + type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Identifier | PropertyAccessExpression | ElementAccessExpression | OmittedExpression; type ObjectBindingOrAssignmentPattern = ObjectBindingPattern | ObjectLiteralExpression; type ArrayBindingOrAssignmentPattern = ArrayBindingPattern | ArrayLiteralExpression; type AssignmentPattern = ObjectLiteralExpression | ArrayLiteralExpression; diff --git a/tests/baselines/reference/destructuringReassignsRightHandSide.js b/tests/baselines/reference/destructuringReassignsRightHandSide.js new file mode 100644 index 00000000000..b893202aa1e --- /dev/null +++ b/tests/baselines/reference/destructuringReassignsRightHandSide.js @@ -0,0 +1,18 @@ +//// [destructuringReassignsRightHandSide.ts] +var foo: any = { foo: 1, bar: 2 }; +var bar: any; + +// reassignment in destructuring pattern +({ foo, bar } = foo); + +// reassignment in subsequent var +var { foo, baz } = foo; + +//// [destructuringReassignsRightHandSide.js] +var foo = { foo: 1, bar: 2 }; +var bar; +// reassignment in destructuring pattern +(_a = foo, foo = _a.foo, bar = _a.bar); +// reassignment in subsequent var +var _b = foo, foo = _b.foo, baz = _b.baz; +var _a; diff --git a/tests/baselines/reference/destructuringReassignsRightHandSide.symbols b/tests/baselines/reference/destructuringReassignsRightHandSide.symbols new file mode 100644 index 00000000000..a0b68b7fe53 --- /dev/null +++ b/tests/baselines/reference/destructuringReassignsRightHandSide.symbols @@ -0,0 +1,21 @@ +=== tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts === +var foo: any = { foo: 1, bar: 2 }; +>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 3), Decl(destructuringReassignsRightHandSide.ts, 7, 5)) +>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 16)) +>bar : Symbol(bar, Decl(destructuringReassignsRightHandSide.ts, 0, 24)) + +var bar: any; +>bar : Symbol(bar, Decl(destructuringReassignsRightHandSide.ts, 1, 3)) + +// reassignment in destructuring pattern +({ foo, bar } = foo); +>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 4, 2)) +>bar : Symbol(bar, Decl(destructuringReassignsRightHandSide.ts, 4, 7)) +>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 3), Decl(destructuringReassignsRightHandSide.ts, 7, 5)) + +// reassignment in subsequent var +var { foo, baz } = foo; +>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 3), Decl(destructuringReassignsRightHandSide.ts, 7, 5)) +>baz : Symbol(baz, Decl(destructuringReassignsRightHandSide.ts, 7, 10)) +>foo : Symbol(foo, Decl(destructuringReassignsRightHandSide.ts, 0, 3), Decl(destructuringReassignsRightHandSide.ts, 7, 5)) + diff --git a/tests/baselines/reference/destructuringReassignsRightHandSide.types b/tests/baselines/reference/destructuringReassignsRightHandSide.types new file mode 100644 index 00000000000..f5ba19820e0 --- /dev/null +++ b/tests/baselines/reference/destructuringReassignsRightHandSide.types @@ -0,0 +1,27 @@ +=== tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts === +var foo: any = { foo: 1, bar: 2 }; +>foo : any +>{ foo: 1, bar: 2 } : { foo: number; bar: number; } +>foo : number +>1 : 1 +>bar : number +>2 : 2 + +var bar: any; +>bar : any + +// reassignment in destructuring pattern +({ foo, bar } = foo); +>({ foo, bar } = foo) : any +>{ foo, bar } = foo : any +>{ foo, bar } : { foo: any; bar: any; } +>foo : any +>bar : any +>foo : any + +// reassignment in subsequent var +var { foo, baz } = foo; +>foo : any +>baz : any +>foo : any + diff --git a/tests/baselines/reference/objectRest.js b/tests/baselines/reference/objectRest.js index 3576ffb286c..c314377f807 100644 --- a/tests/baselines/reference/objectRest.js +++ b/tests/baselines/reference/objectRest.js @@ -85,10 +85,10 @@ var i = removable; var { removed } = i, removableRest2 = __rest(i, ["removed"]); let computed = 'b'; let computed2 = 'a'; -var _g = computed, stillNotGreat = o[_g], _h = computed2, soSo = o[_h], o = __rest(o, [typeof _g === "symbol" ? _g : _g + "", typeof _h === "symbol" ? _h : _h + ""]); -(_j = computed, stillNotGreat = o[_j], _k = computed2, soSo = o[_k], o = __rest(o, [typeof _j === "symbol" ? _j : _j + "", typeof _k === "symbol" ? _k : _k + ""])); +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 + ""]); +(_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 + ""])); var noContextualType = (_a) => { var { aNumber = 12 } = _a, notEmptyObject = __rest(_a, ["aNumber"]); return aNumber + notEmptyObject.anythingGoes; }; -var _d, _f, _j, _k; +var _d, _f, _k, _l, _m; diff --git a/tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts b/tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts new file mode 100644 index 00000000000..042ed5d5248 --- /dev/null +++ b/tests/cases/conformance/es6/destructuring/destructuringReassignsRightHandSide.ts @@ -0,0 +1,9 @@ +// @target: es5 +var foo: any = { foo: 1, bar: 2 }; +var bar: any; + +// reassignment in destructuring pattern +({ foo, bar } = foo); + +// reassignment in subsequent var +var { foo, baz } = foo; \ No newline at end of file From 18a2fc82f7d35528ae31032ade15e2ea07ab3f54 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 17 Jan 2018 11:06:09 -0800 Subject: [PATCH 2/2] Accept tsserverlibrary.d.ts and fix gulpfile --- Gulpfile.ts | 2 +- tests/baselines/reference/api/tsserverlibrary.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gulpfile.ts b/Gulpfile.ts index 2909b10471a..0c410bdfb08 100644 --- a/Gulpfile.ts +++ b/Gulpfile.ts @@ -528,7 +528,7 @@ gulp.task(tsserverLibraryFile, /*help*/ false, [servicesFile, typesMapJson], (do const serverLibraryProject = tsc.createProject("src/server/tsconfig.library.json", getCompilerSettings({ removeComments: false }, /*useBuiltCompiler*/ true)); const {js, dts}: { js: NodeJS.ReadableStream, dts: NodeJS.ReadableStream } = serverLibraryProject.src() .pipe(sourcemaps.init()) - .pipe(newer(tsserverLibraryFile)) + .pipe(newer({ dest: tsserverLibraryFile, extra: ["src/compiler/**/*.ts", "src/services/**/*.ts"] })) .pipe(serverLibraryProject()); return merge2([ diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index e57b52942f4..a0dfa5f5d17 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -877,7 +877,7 @@ declare namespace ts { type DestructuringAssignment = ObjectDestructuringAssignment | ArrayDestructuringAssignment; type BindingOrAssignmentElement = VariableDeclaration | ParameterDeclaration | BindingElement | PropertyAssignment | ShorthandPropertyAssignment | SpreadAssignment | OmittedExpression | SpreadElement | ArrayLiteralExpression | ObjectLiteralExpression | AssignmentExpression | Identifier | PropertyAccessExpression | ElementAccessExpression; type BindingOrAssignmentElementRestIndicator = DotDotDotToken | SpreadElement | SpreadAssignment; - type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Expression; + type BindingOrAssignmentElementTarget = BindingOrAssignmentPattern | Identifier | PropertyAccessExpression | ElementAccessExpression | OmittedExpression; type ObjectBindingOrAssignmentPattern = ObjectBindingPattern | ObjectLiteralExpression; type ArrayBindingOrAssignmentPattern = ArrayBindingPattern | ArrayLiteralExpression; type AssignmentPattern = ObjectLiteralExpression | ArrayLiteralExpression;