From 876dbe86ee17b8521f6535739a01fe489d768a26 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Mon, 9 Jan 2017 08:53:10 -0800 Subject: [PATCH 1/2] Omit class methods from spreads. Others stay. Previously, all methods were omitted except those from the object literal that contained the spread. This gets rid of the ugly third argument to `getSpreadType`. It also fixes a bug that arose from removing the spread type late in the development of object spread; methods from the left-hand-side of a multi-spread object literal were not removed. The spread type code normalised spreads so the left-hand is never an object, but that code was removed. --- src/compiler/checker.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 879be1c6fbf58..d33ce37473604 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6241,7 +6241,7 @@ namespace ts { * this function should be called in a left folding style, with left = previous result of getSpreadType * and right = the new element to be spread. */ - function getSpreadType(left: Type, right: Type, isFromObjectLiteral: boolean): Type { + function getSpreadType(left: Type, right: Type): Type { if (left.flags & TypeFlags.Any || right.flags & TypeFlags.Any) { return anyType; } @@ -6254,10 +6254,10 @@ namespace ts { return left; } if (left.flags & TypeFlags.Union) { - return mapType(left, t => getSpreadType(t, right, isFromObjectLiteral)); + return mapType(left, t => getSpreadType(t, right)); } if (right.flags & TypeFlags.Union) { - return mapType(right, t => getSpreadType(left, t, isFromObjectLiteral)); + return mapType(right, t => getSpreadType(left, t)); } const members = createMap(); @@ -6276,18 +6276,18 @@ namespace ts { for (const rightProp of getPropertiesOfType(right)) { // we approximate own properties as non-methods plus methods that are inside the object literal - const isOwnProperty = !(rightProp.flags & SymbolFlags.Method) || isFromObjectLiteral; const isSetterWithoutGetter = rightProp.flags & SymbolFlags.SetAccessor && !(rightProp.flags & SymbolFlags.GetAccessor); if (getDeclarationModifierFlagsFromSymbol(rightProp) & (ModifierFlags.Private | ModifierFlags.Protected)) { skippedPrivateMembers[rightProp.name] = true; } - else if (isOwnProperty && !isSetterWithoutGetter) { + else if (!isClassMethod(rightProp) && !isSetterWithoutGetter) { members[rightProp.name] = rightProp; } } for (const leftProp of getPropertiesOfType(left)) { if (leftProp.flags & SymbolFlags.SetAccessor && !(leftProp.flags & SymbolFlags.GetAccessor) - || leftProp.name in skippedPrivateMembers) { + || leftProp.name in skippedPrivateMembers + || isClassMethod(leftProp)) { continue; } if (leftProp.name in members) { @@ -6312,6 +6312,10 @@ namespace ts { return createAnonymousType(undefined, members, emptyArray, emptyArray, stringIndexInfo, numberIndexInfo); } + function isClassMethod(prop: Symbol) { + return prop.flags & SymbolFlags.Method && find(prop.declarations, decl => isClassLike(decl.parent)); + } + function createLiteralType(flags: TypeFlags, text: string) { const type = createType(flags); type.text = text; @@ -11658,7 +11662,7 @@ namespace ts { checkExternalEmitHelpers(memberDecl, ExternalEmitHelpers.Assign); } if (propertiesArray.length > 0) { - spread = getSpreadType(spread, createObjectLiteralType(), /*isFromObjectLiteral*/ true); + spread = getSpreadType(spread, createObjectLiteralType()); propertiesArray = []; propertiesTable = createMap(); hasComputedStringProperty = false; @@ -11670,7 +11674,7 @@ namespace ts { error(memberDecl, Diagnostics.Spread_types_may_only_be_created_from_object_types); return unknownType; } - spread = getSpreadType(spread, type, /*isFromObjectLiteral*/ false); + spread = getSpreadType(spread, type); offset = i + 1; continue; } @@ -11715,7 +11719,7 @@ namespace ts { if (spread !== emptyObjectType) { if (propertiesArray.length > 0) { - spread = getSpreadType(spread, createObjectLiteralType(), /*isFromObjectLiteral*/ true); + spread = getSpreadType(spread, createObjectLiteralType()); } if (spread.flags & TypeFlags.Object) { // only set the symbol and flags if this is a (fresh) object type From 309a361b19a1a8435f2c4f5f232d1716995ff171 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Mon, 9 Jan 2017 08:59:36 -0800 Subject: [PATCH 2/2] Test method removal of object spread Test that 1. Only class methods get removed 2. Methods from both left and right get removed. --- .../reference/spreadMethods.errors.txt | 33 ++++++++++ tests/baselines/reference/spreadMethods.js | 63 +++++++++++++++++++ .../conformance/types/spread/spreadMethods.ts | 23 +++++++ 3 files changed, 119 insertions(+) create mode 100644 tests/baselines/reference/spreadMethods.errors.txt create mode 100644 tests/baselines/reference/spreadMethods.js create mode 100644 tests/cases/conformance/types/spread/spreadMethods.ts diff --git a/tests/baselines/reference/spreadMethods.errors.txt b/tests/baselines/reference/spreadMethods.errors.txt new file mode 100644 index 0000000000000..b2b3491ba7936 --- /dev/null +++ b/tests/baselines/reference/spreadMethods.errors.txt @@ -0,0 +1,33 @@ +tests/cases/conformance/types/spread/spreadMethods.ts(7,4): error TS2339: Property 'm' does not exist on type '{ p: number; }'. +tests/cases/conformance/types/spread/spreadMethods.ts(9,5): error TS2339: Property 'm' does not exist on type '{ p: number; }'. + + +==== tests/cases/conformance/types/spread/spreadMethods.ts (2 errors) ==== + class K { p = 12; m() { } } + interface I { p: number, m(): void } + let k = new K() + let sk = { ...k }; + let ssk = { ...k, ...k }; + sk.p; + sk.m(); // error + ~ +!!! error TS2339: Property 'm' does not exist on type '{ p: number; }'. + ssk.p; + ssk.m(); // error + ~ +!!! error TS2339: Property 'm' does not exist on type '{ p: number; }'. + let i: I = { p: 12, m() { } }; + let si = { ...i }; + let ssi = { ...i, ...i }; + si.p; + si.m(); // ok + ssi.p; + ssi.m(); // ok + let o = { p: 12, m() { } }; + let so = { ...o }; + let sso = { ...o, ...o }; + so.p; + so.m(); // ok + sso.p; + sso.m(); // ok + \ No newline at end of file diff --git a/tests/baselines/reference/spreadMethods.js b/tests/baselines/reference/spreadMethods.js new file mode 100644 index 0000000000000..691d6fa282944 --- /dev/null +++ b/tests/baselines/reference/spreadMethods.js @@ -0,0 +1,63 @@ +//// [spreadMethods.ts] +class K { p = 12; m() { } } +interface I { p: number, m(): void } +let k = new K() +let sk = { ...k }; +let ssk = { ...k, ...k }; +sk.p; +sk.m(); // error +ssk.p; +ssk.m(); // error +let i: I = { p: 12, m() { } }; +let si = { ...i }; +let ssi = { ...i, ...i }; +si.p; +si.m(); // ok +ssi.p; +ssi.m(); // ok +let o = { p: 12, m() { } }; +let so = { ...o }; +let sso = { ...o, ...o }; +so.p; +so.m(); // ok +sso.p; +sso.m(); // ok + + +//// [spreadMethods.js] +var __assign = (this && this.__assign) || Object.assign || function(t) { + for (var s, i = 1, n = arguments.length; i < n; i++) { + s = arguments[i]; + for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p)) + t[p] = s[p]; + } + return t; +}; +var K = (function () { + function K() { + this.p = 12; + } + K.prototype.m = function () { }; + return K; +}()); +var k = new K(); +var sk = __assign({}, k); +var ssk = __assign({}, k, k); +sk.p; +sk.m(); // error +ssk.p; +ssk.m(); // error +var i = { p: 12, m: function () { } }; +var si = __assign({}, i); +var ssi = __assign({}, i, i); +si.p; +si.m(); // ok +ssi.p; +ssi.m(); // ok +var o = { p: 12, m: function () { } }; +var so = __assign({}, o); +var sso = __assign({}, o, o); +so.p; +so.m(); // ok +sso.p; +sso.m(); // ok diff --git a/tests/cases/conformance/types/spread/spreadMethods.ts b/tests/cases/conformance/types/spread/spreadMethods.ts new file mode 100644 index 0000000000000..5d56265342484 --- /dev/null +++ b/tests/cases/conformance/types/spread/spreadMethods.ts @@ -0,0 +1,23 @@ +class K { p = 12; m() { } } +interface I { p: number, m(): void } +let k = new K() +let sk = { ...k }; +let ssk = { ...k, ...k }; +sk.p; +sk.m(); // error +ssk.p; +ssk.m(); // error +let i: I = { p: 12, m() { } }; +let si = { ...i }; +let ssi = { ...i, ...i }; +si.p; +si.m(); // ok +ssi.p; +ssi.m(); // ok +let o = { p: 12, m() { } }; +let so = { ...o }; +let sso = { ...o, ...o }; +so.p; +so.m(); // ok +sso.p; +sso.m(); // ok