Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Don't include class getter in spread type
Browse files Browse the repository at this point in the history
Andy Hanson committed Aug 8, 2018
1 parent 1a05f13 commit 7dded40
Showing 7 changed files with 282 additions and 140 deletions.
24 changes: 10 additions & 14 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
@@ -4461,10 +4461,7 @@ namespace ts {
names.set(getTextOfPropertyName(name), true);
}
for (const prop of getPropertiesOfType(source)) {
const inNamesToRemove = names.has(prop.escapedName);
const isPrivate = getDeclarationModifierFlagsFromSymbol(prop) & (ModifierFlags.Private | ModifierFlags.Protected);
const isSetOnlyAccessor = prop.flags & SymbolFlags.SetAccessor && !(prop.flags & SymbolFlags.GetAccessor);
if (!inNamesToRemove && !isPrivate && !isClassMethod(prop) && !isSetOnlyAccessor) {
if (!names.has(prop.escapedName) && !(getDeclarationModifierFlagsFromSymbol(prop) & (ModifierFlags.Private | ModifierFlags.Protected)) && isSpreadableProperty(prop)) {
members.set(prop.escapedName, getNonReadonlySymbol(prop));
}
}
@@ -9646,20 +9643,16 @@ namespace ts {
}

for (const rightProp of getPropertiesOfType(right)) {
// we approximate own properties as non-methods plus methods that are inside the object literal
const isSetterWithoutGetter = rightProp.flags & SymbolFlags.SetAccessor && !(rightProp.flags & SymbolFlags.GetAccessor);
if (getDeclarationModifierFlagsFromSymbol(rightProp) & (ModifierFlags.Private | ModifierFlags.Protected)) {
skippedPrivateMembers.set(rightProp.escapedName, true);
}
else if (!isClassMethod(rightProp) && !isSetterWithoutGetter) {
else if (isSpreadableProperty(rightProp)) {
members.set(rightProp.escapedName, getNonReadonlySymbol(rightProp));
}
}

for (const leftProp of getPropertiesOfType(left)) {
if (leftProp.flags & SymbolFlags.SetAccessor && !(leftProp.flags & SymbolFlags.GetAccessor)
|| skippedPrivateMembers.has(leftProp.escapedName)
|| isClassMethod(leftProp)) {
if (skippedPrivateMembers.has(leftProp.escapedName) || !isSpreadableProperty(leftProp)) {
continue;
}
if (members.has(leftProp.escapedName)) {
@@ -9694,6 +9687,13 @@ namespace ts {
return spread;
}

function isSpreadableProperty(prop: Symbol): boolean {
// We approximate own properties as non-methods plus methods that are inside the object literal
return prop.flags & (SymbolFlags.Method | SymbolFlags.GetAccessor)
? !prop.declarations.some(decl => isClassLike(decl.parent))
: !(prop.flags & SymbolFlags.SetAccessor); // Setter without getter is not spreadable
}

function getNonReadonlySymbol(prop: Symbol) {
if (!isReadonlySymbol(prop)) {
return prop;
@@ -9714,10 +9714,6 @@ namespace ts {
return index;
}

function isClassMethod(prop: Symbol) {
return prop.flags & SymbolFlags.Method && find(prop.declarations, decl => isClassLike(decl.parent));
}

function createLiteralType(flags: TypeFlags, value: string | number, symbol: Symbol | undefined) {
const type = <LiteralType>createType(flags);
type.symbol = symbol!;
2 changes: 1 addition & 1 deletion tests/baselines/reference/objectRest.types
Original file line number Diff line number Diff line change
@@ -175,7 +175,7 @@ var removable = new Removable();

var { removed, ...removableRest } = removable;
>removed : string
>removableRest : { both: number; remainder: string; }
>removableRest : { remainder: string; }
>removable : Removable

var i: I = removable;
37 changes: 30 additions & 7 deletions tests/baselines/reference/spreadMethods.errors.txt
Original file line number Diff line number Diff line change
@@ -1,33 +1,56 @@
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(16,4): error TS2339: Property 'm' does not exist on type '{ p: number; }'.
tests/cases/conformance/types/spread/spreadMethods.ts(17,4): error TS2339: Property 'g' does not exist on type '{ p: number; }'.
tests/cases/conformance/types/spread/spreadMethods.ts(19,5): error TS2339: Property 'm' does not exist on type '{ p: number; }'.
tests/cases/conformance/types/spread/spreadMethods.ts(20,5): error TS2339: Property 'g' 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 }
==== tests/cases/conformance/types/spread/spreadMethods.ts (4 errors) ====
class K {
p = 12;
m() { }
get g() { return 0; }
}
interface I {
p: number;
m(): void;
readonly g: number;
}

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; }'.
sk.g; // error
~
!!! error TS2339: Property 'g' 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() { } };
ssk.g; // error
~
!!! error TS2339: Property 'g' does not exist on type '{ p: number; }'.

let i: I = { p: 12, m() { }, get g() { return 0; } };
let si = { ...i };
let ssi = { ...i, ...i };
si.p;
si.m(); // ok
si.g; // ok
ssi.p;
ssi.m(); // ok
let o = { p: 12, m() { } };
ssi.g; // ok

let o = { p: 12, m() { }, get g() { return 0; } };
let so = { ...o };
let sso = { ...o, ...o };
so.p;
so.m(); // ok
so.g; // ok
sso.p;
sso.m(); // ok
sso.g; // ok

70 changes: 41 additions & 29 deletions tests/baselines/reference/spreadMethods.js
Original file line number Diff line number Diff line change
@@ -1,66 +1,78 @@
//// [spreadMethods.ts]
class K { p = 12; m() { } }
interface I { p: number, m(): void }
class K {
p = 12;
m() { }
get g() { return 0; }
}
interface I {
p: number;
m(): void;
readonly g: number;
}

let k = new K()
let sk = { ...k };
let ssk = { ...k, ...k };
sk.p;
sk.m(); // error
sk.g; // error
ssk.p;
ssk.m(); // error
let i: I = { p: 12, m() { } };
ssk.g; // error

let i: I = { p: 12, m() { }, get g() { return 0; } };
let si = { ...i };
let ssi = { ...i, ...i };
si.p;
si.m(); // ok
si.g; // ok
ssi.p;
ssi.m(); // ok
let o = { p: 12, m() { } };
ssi.g; // ok

let o = { p: 12, m() { }, get g() { return 0; } };
let so = { ...o };
let sso = { ...o, ...o };
so.p;
so.m(); // ok
so.g; // ok
sso.p;
sso.m(); // ok
sso.g; // ok


//// [spreadMethods.js]
var __assign = (this && this.__assign) || function () {
__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;
};
return __assign.apply(this, arguments);
};
var K = /** @class */ (function () {
function K() {
class K {
constructor() {
this.p = 12;
}
K.prototype.m = function () { };
return K;
}());
var k = new K();
var sk = __assign({}, k);
var ssk = __assign({}, k, k);
m() { }
get g() { return 0; }
}
let k = new K();
let sk = { ...k };
let ssk = { ...k, ...k };
sk.p;
sk.m(); // error
sk.g; // error
ssk.p;
ssk.m(); // error
var i = { p: 12, m: function () { } };
var si = __assign({}, i);
var ssi = __assign({}, i, i);
ssk.g; // error
let i = { p: 12, m() { }, get g() { return 0; } };
let si = { ...i };
let ssi = { ...i, ...i };
si.p;
si.m(); // ok
si.g; // ok
ssi.p;
ssi.m(); // ok
var o = { p: 12, m: function () { } };
var so = __assign({}, o);
var sso = __assign({}, o, o);
ssi.g; // ok
let o = { p: 12, m() { }, get g() { return 0; } };
let so = { ...o };
let sso = { ...o, ...o };
so.p;
so.m(); // ok
so.g; // ok
sso.p;
sso.m(); // ok
sso.g; // ok
Loading

0 comments on commit 7dded40

Please sign in to comment.