Skip to content

Commit 639fdcc

Browse files
author
Andy
authored
Don't include class getter in spread type (#26287)
* Don't include class getter in spread type * Code review
1 parent 55a620c commit 639fdcc

7 files changed

Lines changed: 284 additions & 140 deletions

File tree

‎src/compiler/checker.ts‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4464,10 +4464,9 @@ namespace ts {
44644464
names.set(getTextOfPropertyName(name), true);
44654465
}
44664466
for (const prop of getPropertiesOfType(source)) {
4467-
const inNamesToRemove = names.has(prop.escapedName);
4468-
const isPrivate = getDeclarationModifierFlagsFromSymbol(prop) & (ModifierFlags.Private | ModifierFlags.Protected);
4469-
const isSetOnlyAccessor = prop.flags & SymbolFlags.SetAccessor && !(prop.flags & SymbolFlags.GetAccessor);
4470-
if (!inNamesToRemove && !isPrivate && !isClassMethod(prop) && !isSetOnlyAccessor) {
4467+
if (!names.has(prop.escapedName)
4468+
&& !(getDeclarationModifierFlagsFromSymbol(prop) & (ModifierFlags.Private | ModifierFlags.Protected))
4469+
&& isSpreadableProperty(prop)) {
44714470
members.set(prop.escapedName, getNonReadonlySymbol(prop));
44724471
}
44734472
}
@@ -9649,20 +9648,16 @@ namespace ts {
96499648
}
96509649

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

96629659
for (const leftProp of getPropertiesOfType(left)) {
9663-
if (leftProp.flags & SymbolFlags.SetAccessor && !(leftProp.flags & SymbolFlags.GetAccessor)
9664-
|| skippedPrivateMembers.has(leftProp.escapedName)
9665-
|| isClassMethod(leftProp)) {
9660+
if (skippedPrivateMembers.has(leftProp.escapedName) || !isSpreadableProperty(leftProp)) {
96669661
continue;
96679662
}
96689663
if (members.has(leftProp.escapedName)) {
@@ -9697,6 +9692,13 @@ namespace ts {
96979692
return spread;
96989693
}
96999694

9695+
/** We approximate own properties as non-methods plus methods that are inside the object literal */
9696+
function isSpreadableProperty(prop: Symbol): boolean {
9697+
return prop.flags & (SymbolFlags.Method | SymbolFlags.GetAccessor)
9698+
? !prop.declarations.some(decl => isClassLike(decl.parent))
9699+
: !(prop.flags & SymbolFlags.SetAccessor); // Setter without getter is not spreadable
9700+
}
9701+
97009702
function getNonReadonlySymbol(prop: Symbol) {
97019703
if (!isReadonlySymbol(prop)) {
97029704
return prop;
@@ -9717,10 +9719,6 @@ namespace ts {
97179719
return index;
97189720
}
97199721

9720-
function isClassMethod(prop: Symbol) {
9721-
return prop.flags & SymbolFlags.Method && find(prop.declarations, decl => isClassLike(decl.parent));
9722-
}
9723-
97249722
function createLiteralType(flags: TypeFlags, value: string | number, symbol: Symbol | undefined) {
97259723
const type = <LiteralType>createType(flags);
97269724
type.symbol = symbol!;

‎tests/baselines/reference/objectRest.types‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ var removable = new Removable();
175175

176176
var { removed, ...removableRest } = removable;
177177
>removed : string
178-
>removableRest : { both: number; remainder: string; }
178+
>removableRest : { remainder: string; }
179179
>removable : Removable
180180

181181
var i: I = removable;

‎tests/baselines/reference/spreadMethods.errors.txt‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,56 @@
1-
tests/cases/conformance/types/spread/spreadMethods.ts(7,4): error TS2339: Property 'm' does not exist on type '{ p: number; }'.
2-
tests/cases/conformance/types/spread/spreadMethods.ts(9,5): error TS2339: Property 'm' does not exist on type '{ p: number; }'.
1+
tests/cases/conformance/types/spread/spreadMethods.ts(16,4): error TS2339: Property 'm' does not exist on type '{ p: number; }'.
2+
tests/cases/conformance/types/spread/spreadMethods.ts(17,4): error TS2339: Property 'g' does not exist on type '{ p: number; }'.
3+
tests/cases/conformance/types/spread/spreadMethods.ts(19,5): error TS2339: Property 'm' does not exist on type '{ p: number; }'.
4+
tests/cases/conformance/types/spread/spreadMethods.ts(20,5): error TS2339: Property 'g' does not exist on type '{ p: number; }'.
35

46

5-
==== tests/cases/conformance/types/spread/spreadMethods.ts (2 errors) ====
6-
class K { p = 12; m() { } }
7-
interface I { p: number, m(): void }
7+
==== tests/cases/conformance/types/spread/spreadMethods.ts (4 errors) ====
8+
class K {
9+
p = 12;
10+
m() { }
11+
get g() { return 0; }
12+
}
13+
interface I {
14+
p: number;
15+
m(): void;
16+
readonly g: number;
17+
}
18+
819
let k = new K()
920
let sk = { ...k };
1021
let ssk = { ...k, ...k };
1122
sk.p;
1223
sk.m(); // error
1324
~
1425
!!! error TS2339: Property 'm' does not exist on type '{ p: number; }'.
26+
sk.g; // error
27+
~
28+
!!! error TS2339: Property 'g' does not exist on type '{ p: number; }'.
1529
ssk.p;
1630
ssk.m(); // error
1731
~
1832
!!! error TS2339: Property 'm' does not exist on type '{ p: number; }'.
19-
let i: I = { p: 12, m() { } };
33+
ssk.g; // error
34+
~
35+
!!! error TS2339: Property 'g' does not exist on type '{ p: number; }'.
36+
37+
let i: I = { p: 12, m() { }, get g() { return 0; } };
2038
let si = { ...i };
2139
let ssi = { ...i, ...i };
2240
si.p;
2341
si.m(); // ok
42+
si.g; // ok
2443
ssi.p;
2544
ssi.m(); // ok
26-
let o = { p: 12, m() { } };
45+
ssi.g; // ok
46+
47+
let o = { p: 12, m() { }, get g() { return 0; } };
2748
let so = { ...o };
2849
let sso = { ...o, ...o };
2950
so.p;
3051
so.m(); // ok
52+
so.g; // ok
3153
sso.p;
3254
sso.m(); // ok
55+
sso.g; // ok
3356

‎tests/baselines/reference/spreadMethods.js‎

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,78 @@
11
//// [spreadMethods.ts]
2-
class K { p = 12; m() { } }
3-
interface I { p: number, m(): void }
2+
class K {
3+
p = 12;
4+
m() { }
5+
get g() { return 0; }
6+
}
7+
interface I {
8+
p: number;
9+
m(): void;
10+
readonly g: number;
11+
}
12+
413
let k = new K()
514
let sk = { ...k };
615
let ssk = { ...k, ...k };
716
sk.p;
817
sk.m(); // error
18+
sk.g; // error
919
ssk.p;
1020
ssk.m(); // error
11-
let i: I = { p: 12, m() { } };
21+
ssk.g; // error
22+
23+
let i: I = { p: 12, m() { }, get g() { return 0; } };
1224
let si = { ...i };
1325
let ssi = { ...i, ...i };
1426
si.p;
1527
si.m(); // ok
28+
si.g; // ok
1629
ssi.p;
1730
ssi.m(); // ok
18-
let o = { p: 12, m() { } };
31+
ssi.g; // ok
32+
33+
let o = { p: 12, m() { }, get g() { return 0; } };
1934
let so = { ...o };
2035
let sso = { ...o, ...o };
2136
so.p;
2237
so.m(); // ok
38+
so.g; // ok
2339
sso.p;
2440
sso.m(); // ok
41+
sso.g; // ok
2542

2643

2744
//// [spreadMethods.js]
28-
var __assign = (this && this.__assign) || function () {
29-
__assign = Object.assign || function(t) {
30-
for (var s, i = 1, n = arguments.length; i < n; i++) {
31-
s = arguments[i];
32-
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
33-
t[p] = s[p];
34-
}
35-
return t;
36-
};
37-
return __assign.apply(this, arguments);
38-
};
39-
var K = /** @class */ (function () {
40-
function K() {
45+
class K {
46+
constructor() {
4147
this.p = 12;
4248
}
43-
K.prototype.m = function () { };
44-
return K;
45-
}());
46-
var k = new K();
47-
var sk = __assign({}, k);
48-
var ssk = __assign({}, k, k);
49+
m() { }
50+
get g() { return 0; }
51+
}
52+
let k = new K();
53+
let sk = { ...k };
54+
let ssk = { ...k, ...k };
4955
sk.p;
5056
sk.m(); // error
57+
sk.g; // error
5158
ssk.p;
5259
ssk.m(); // error
53-
var i = { p: 12, m: function () { } };
54-
var si = __assign({}, i);
55-
var ssi = __assign({}, i, i);
60+
ssk.g; // error
61+
let i = { p: 12, m() { }, get g() { return 0; } };
62+
let si = { ...i };
63+
let ssi = { ...i, ...i };
5664
si.p;
5765
si.m(); // ok
66+
si.g; // ok
5867
ssi.p;
5968
ssi.m(); // ok
60-
var o = { p: 12, m: function () { } };
61-
var so = __assign({}, o);
62-
var sso = __assign({}, o, o);
69+
ssi.g; // ok
70+
let o = { p: 12, m() { }, get g() { return 0; } };
71+
let so = { ...o };
72+
let sso = { ...o, ...o };
6373
so.p;
6474
so.m(); // ok
75+
so.g; // ok
6576
sso.p;
6677
sso.m(); // ok
78+
sso.g; // ok

0 commit comments

Comments
 (0)