Skip to content

Commit 8c01efb

Browse files
sandersnsandersn
committed
Allow JS multiple declarations of ctor properties
When a property is declared in the constructor and on the prototype of an ES6 class, the property's symbol is discarded in favour of the method's symbol. That because the usual use for this pattern is to bind an instance function: `this.m = this.m.bind(this)`. In this case the type you want really is the method's type.
1 parent 0eeb9cb commit 8c01efb

File tree

6 files changed

+183
-29
lines changed

6 files changed

+183
-29
lines changed

‎src/compiler/binder.ts

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -298,8 +298,10 @@ namespace ts {
298298
const name = isDefaultExport && parent ? "default" : getDeclarationName(node);
299299

300300
let symbol: Symbol;
301-
if (name !== undefined) {
302-
301+
if (name === undefined) {
302+
symbol = createSymbol(SymbolFlags.None, "__missing");
303+
}
304+
else {
303305
// Check and see if the symbol table already has a symbol with this name. If not,
304306
// create a new symbol with this name and add it to the table. Note that we don't
305307
// give the new symbol any flags *yet*. This ensures that it will not conflict
@@ -326,34 +328,38 @@ namespace ts {
326328
classifiableNames[name] = name;
327329
}
328330

329-
if (symbol.flags & excludes) {
330-
if (node.name) {
331-
node.name.parent = node;
331+
else if (symbol.flags & excludes) {
332+
if (symbol.isDiscardable) {
333+
// Javascript constructor-declared symbols can be discarded in favor of
334+
// prototype symbols like methods.
335+
symbol = symbolTable[name] = createSymbol(SymbolFlags.None, name);
332336
}
337+
else {
338+
if (node.name) {
339+
node.name.parent = node;
340+
}
333341

334-
// Report errors every position with duplicate declaration
335-
// Report errors on previous encountered declarations
336-
let message = symbol.flags & SymbolFlags.BlockScopedVariable
337-
? Diagnostics.Cannot_redeclare_block_scoped_variable_0
338-
: Diagnostics.Duplicate_identifier_0;
342+
// Report errors every position with duplicate declaration
343+
// Report errors on previous encountered declarations
344+
let message = symbol.flags & SymbolFlags.BlockScopedVariable
345+
? Diagnostics.Cannot_redeclare_block_scoped_variable_0
346+
: Diagnostics.Duplicate_identifier_0;
339347

340-
forEach(symbol.declarations, declaration => {
341-
if (declaration.flags & NodeFlags.Default) {
342-
message = Diagnostics.A_module_cannot_have_multiple_default_exports;
343-
}
344-
});
348+
forEach(symbol.declarations, declaration => {
349+
if (declaration.flags & NodeFlags.Default) {
350+
message = Diagnostics.A_module_cannot_have_multiple_default_exports;
351+
}
352+
});
345353

346-
forEach(symbol.declarations, declaration => {
347-
file.bindDiagnostics.push(createDiagnosticForNode(declaration.name || declaration, message, getDisplayName(declaration)));
348-
});
349-
file.bindDiagnostics.push(createDiagnosticForNode(node.name || node, message, getDisplayName(node)));
354+
forEach(symbol.declarations, declaration => {
355+
file.bindDiagnostics.push(createDiagnosticForNode(declaration.name || declaration, message, getDisplayName(declaration)));
356+
});
357+
file.bindDiagnostics.push(createDiagnosticForNode(node.name || node, message, getDisplayName(node)));
350358

351-
symbol = createSymbol(SymbolFlags.None, name);
359+
symbol = createSymbol(SymbolFlags.None, name);
360+
}
352361
}
353362
}
354-
else {
355-
symbol = createSymbol(SymbolFlags.None, "__missing");
356-
}
357363

358364
addDeclarationToSymbol(symbol, node, includes);
359365
symbol.parent = parent;
@@ -1967,7 +1973,7 @@ namespace ts {
19671973
}
19681974

19691975
function bindThisPropertyAssignment(node: BinaryExpression) {
1970-
// Declare a 'member' in case it turns out the container was an ES5 class or ES6 constructor
1976+
// Declare a 'member' if the container is an ES5 class or ES6 constructor
19711977
let assignee: Node;
19721978
if (container.kind === SyntaxKind.FunctionDeclaration || container.kind === SyntaxKind.FunctionExpression) {
19731979
assignee = container;
@@ -1980,7 +1986,9 @@ namespace ts {
19801986
}
19811987
assignee.symbol.members = assignee.symbol.members || {};
19821988
// It's acceptable for multiple 'this' assignments of the same identifier to occur
1983-
declareSymbol(assignee.symbol.members, assignee.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property);
1989+
// AND it can be overwritten by subsequent method declarations
1990+
let symbol = declareSymbol(assignee.symbol.members, assignee.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property);
1991+
symbol.isDiscardable = true;
19841992
}
19851993

19861994
function bindPrototypePropertyAssignment(node: BinaryExpression) {

‎src/compiler/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2137,6 +2137,7 @@ namespace ts {
21372137
/* @internal */ exportSymbol?: Symbol; // Exported symbol associated with this symbol
21382138
/* @internal */ constEnumOnlyModule?: boolean; // True if module contains only const enums or other modules with only const enums
21392139
/* @internal */ isReferenced?: boolean; // True if the symbol is referenced elsewhere
2140+
/* @internal */ isDiscardable?: boolean;// True if a Javascript class property can be overwritten by a method
21402141
}
21412142

21422143
/* @internal */

‎tests/baselines/reference/multipleDeclarations.js

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,22 @@ function C() {
55
}
66
C.prototype.m = function() {
77
this.nothing();
8-
};
8+
}
9+
10+
class X {
11+
constructor() {
12+
this.m = this.m.bind(this);
13+
this.mistake = 'frankly, complete nonsense';
14+
}
15+
m() {
16+
}
17+
mistake() {
18+
}
19+
}
20+
let x = new X();
21+
X.prototype.mistake = false;
22+
x.m();
23+
x.mistake;
924

1025

1126
//// [output.js]
@@ -15,3 +30,18 @@ function C() {
1530
C.prototype.m = function () {
1631
this.nothing();
1732
};
33+
var X = (function () {
34+
function X() {
35+
this.m = this.m.bind(this);
36+
this.mistake = 'frankly, complete nonsense';
37+
}
38+
X.prototype.m = function () {
39+
};
40+
X.prototype.mistake = function () {
41+
};
42+
return X;
43+
}());
44+
var x = new X();
45+
X.prototype.mistake = false;
46+
x.m();
47+
x.mistake;

‎tests/baselines/reference/multipleDeclarations.symbols

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,51 @@ C.prototype.m = function() {
1414

1515
this.nothing();
1616
>this : Symbol(C, Decl(input.js, 0, 0))
17+
}
18+
19+
class X {
20+
>X : Symbol(X, Decl(input.js, 6, 1))
21+
22+
constructor() {
23+
this.m = this.m.bind(this);
24+
>this.m : Symbol(X.m, Decl(input.js, 12, 5))
25+
>this : Symbol(X, Decl(input.js, 6, 1))
26+
>m : Symbol(X.m, Decl(input.js, 9, 19))
27+
>this.m.bind : Symbol(Function.bind, Decl(lib.d.ts, --, --))
28+
>this.m : Symbol(X.m, Decl(input.js, 12, 5))
29+
>this : Symbol(X, Decl(input.js, 6, 1))
30+
>m : Symbol(X.m, Decl(input.js, 12, 5))
31+
>bind : Symbol(Function.bind, Decl(lib.d.ts, --, --))
32+
>this : Symbol(X, Decl(input.js, 6, 1))
33+
34+
this.mistake = 'frankly, complete nonsense';
35+
>this.mistake : Symbol(X.mistake, Decl(input.js, 14, 5))
36+
>this : Symbol(X, Decl(input.js, 6, 1))
37+
>mistake : Symbol(X.mistake, Decl(input.js, 10, 35))
38+
}
39+
m() {
40+
>m : Symbol(X.m, Decl(input.js, 12, 5))
41+
}
42+
mistake() {
43+
>mistake : Symbol(X.mistake, Decl(input.js, 14, 5))
44+
}
45+
}
46+
let x = new X();
47+
>x : Symbol(x, Decl(input.js, 18, 3))
48+
>X : Symbol(X, Decl(input.js, 6, 1))
49+
50+
X.prototype.mistake = false;
51+
>X.prototype.mistake : Symbol(X.mistake, Decl(input.js, 14, 5))
52+
>X : Symbol(X, Decl(input.js, 6, 1))
53+
>prototype : Symbol(X.prototype)
54+
55+
x.m();
56+
>x.m : Symbol(X.m, Decl(input.js, 12, 5))
57+
>x : Symbol(x, Decl(input.js, 18, 3))
58+
>m : Symbol(X.m, Decl(input.js, 12, 5))
1759

18-
};
60+
x.mistake;
61+
>x.mistake : Symbol(X.mistake, Decl(input.js, 14, 5))
62+
>x : Symbol(x, Decl(input.js, 18, 3))
63+
>mistake : Symbol(X.mistake, Decl(input.js, 14, 5))
1964

‎tests/baselines/reference/multipleDeclarations.types

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,61 @@ C.prototype.m = function() {
2424
>this.nothing : any
2525
>this : { m: () => void; }
2626
>nothing : any
27+
}
28+
29+
class X {
30+
>X : X
31+
32+
constructor() {
33+
this.m = this.m.bind(this);
34+
>this.m = this.m.bind(this) : any
35+
>this.m : () => void
36+
>this : this
37+
>m : () => void
38+
>this.m.bind(this) : any
39+
>this.m.bind : (this: Function, thisArg: any, ...argArray: any[]) => any
40+
>this.m : () => void
41+
>this : this
42+
>m : () => void
43+
>bind : (this: Function, thisArg: any, ...argArray: any[]) => any
44+
>this : this
45+
46+
this.mistake = 'frankly, complete nonsense';
47+
>this.mistake = 'frankly, complete nonsense' : string
48+
>this.mistake : () => void
49+
>this : this
50+
>mistake : () => void
51+
>'frankly, complete nonsense' : string
52+
}
53+
m() {
54+
>m : () => void
55+
}
56+
mistake() {
57+
>mistake : () => void
58+
}
59+
}
60+
let x = new X();
61+
>x : X
62+
>new X() : X
63+
>X : typeof X
64+
65+
X.prototype.mistake = false;
66+
>X.prototype.mistake = false : boolean
67+
>X.prototype.mistake : () => void
68+
>X.prototype : X
69+
>X : typeof X
70+
>prototype : X
71+
>mistake : () => void
72+
>false : boolean
73+
74+
x.m();
75+
>x.m() : void
76+
>x.m : () => void
77+
>x : X
78+
>m : () => void
2779

28-
};
80+
x.mistake;
81+
>x.mistake : () => void
82+
>x : X
83+
>mistake : () => void
2984

‎tests/cases/conformance/salsa/multipleDeclarations.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,19 @@ function C() {
77
}
88
C.prototype.m = function() {
99
this.nothing();
10-
};
10+
}
11+
12+
class X {
13+
constructor() {
14+
this.m = this.m.bind(this);
15+
this.mistake = 'frankly, complete nonsense';
16+
}
17+
m() {
18+
}
19+
mistake() {
20+
}
21+
}
22+
let x = new X();
23+
X.prototype.mistake = false;
24+
x.m();
25+
x.mistake;

0 commit comments

Comments
 (0)