Skip to content

Commit 9769718

Browse files
sandersnsandersn
authored
Merge pull request microsoft#10123 from Microsoft/allow-js-multiple-declaration-of-constructor-properties
Allow JS multiple declarations of ctor properties
2 parents 09bc2e6 + c73efe2 commit 9769718

File tree

6 files changed

+340
-53
lines changed

6 files changed

+340
-53
lines changed

‎src/compiler/binder.ts‎

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

301301
let symbol: Symbol;
302-
if (name !== undefined) {
303-
302+
if (name === undefined) {
303+
symbol = createSymbol(SymbolFlags.None, "__missing");
304+
}
305+
else {
304306
// Check and see if the symbol table already has a symbol with this name. If not,
305307
// create a new symbol with this name and add it to the table. Note that we don't
306308
// give the new symbol any flags *yet*. This ensures that it will not conflict
@@ -312,6 +314,11 @@ namespace ts {
312314
// declaration we have for this symbol, and then create a new symbol for this
313315
// declaration.
314316
//
317+
// Note that when properties declared in Javascript constructors
318+
// (marked by isReplaceableByMethod) conflict with another symbol, the property loses.
319+
// Always. This allows the common Javascript pattern of overwriting a prototype method
320+
// with an bound instance method of the same type: `this.method = this.method.bind(this)`
321+
//
315322
// If we created a new symbol, either because we didn't have a symbol with this name
316323
// in the symbol table, or we conflicted with an existing symbol, then just add this
317324
// node as the sole declaration of the new symbol.
@@ -326,33 +333,37 @@ namespace ts {
326333
}
327334

328335
if (symbol.flags & excludes) {
329-
if (node.name) {
330-
node.name.parent = node;
336+
if (symbol.isReplaceableByMethod) {
337+
// Javascript constructor-declared symbols can be discarded in favor of
338+
// prototype symbols like methods.
339+
symbol = symbolTable[name] = createSymbol(SymbolFlags.None, name);
331340
}
341+
else {
342+
if (node.name) {
343+
node.name.parent = node;
344+
}
332345

333-
// Report errors every position with duplicate declaration
334-
// Report errors on previous encountered declarations
335-
let message = symbol.flags & SymbolFlags.BlockScopedVariable
336-
? Diagnostics.Cannot_redeclare_block_scoped_variable_0
337-
: Diagnostics.Duplicate_identifier_0;
346+
// Report errors every position with duplicate declaration
347+
// Report errors on previous encountered declarations
348+
let message = symbol.flags & SymbolFlags.BlockScopedVariable
349+
? Diagnostics.Cannot_redeclare_block_scoped_variable_0
350+
: Diagnostics.Duplicate_identifier_0;
338351

339-
forEach(symbol.declarations, declaration => {
340-
if (declaration.flags & NodeFlags.Default) {
341-
message = Diagnostics.A_module_cannot_have_multiple_default_exports;
342-
}
343-
});
352+
forEach(symbol.declarations, declaration => {
353+
if (declaration.flags & NodeFlags.Default) {
354+
message = Diagnostics.A_module_cannot_have_multiple_default_exports;
355+
}
356+
});
344357

345-
forEach(symbol.declarations, declaration => {
346-
file.bindDiagnostics.push(createDiagnosticForNode(declaration.name || declaration, message, getDisplayName(declaration)));
347-
});
348-
file.bindDiagnostics.push(createDiagnosticForNode(node.name || node, message, getDisplayName(node)));
358+
forEach(symbol.declarations, declaration => {
359+
file.bindDiagnostics.push(createDiagnosticForNode(declaration.name || declaration, message, getDisplayName(declaration)));
360+
});
361+
file.bindDiagnostics.push(createDiagnosticForNode(node.name || node, message, getDisplayName(node)));
349362

350-
symbol = createSymbol(SymbolFlags.None, name);
363+
symbol = createSymbol(SymbolFlags.None, name);
364+
}
351365
}
352366
}
353-
else {
354-
symbol = createSymbol(SymbolFlags.None, "__missing");
355-
}
356367

357368
addDeclarationToSymbol(symbol, node, includes);
358369
symbol.parent = parent;
@@ -1966,31 +1977,25 @@ namespace ts {
19661977
}
19671978

19681979
function bindThisPropertyAssignment(node: BinaryExpression) {
1969-
// Declare a 'member' in case it turns out the container was an ES5 class or ES6 constructor
1970-
let assignee: Node;
1980+
Debug.assert(isInJavaScriptFile(node));
1981+
// Declare a 'member' if the container is an ES5 class or ES6 constructor
19711982
if (container.kind === SyntaxKind.FunctionDeclaration || container.kind === SyntaxKind.FunctionExpression) {
1972-
assignee = container;
1983+
container.symbol.members = container.symbol.members || createMap<Symbol>();
1984+
// It's acceptable for multiple 'this' assignments of the same identifier to occur
1985+
declareSymbol(container.symbol.members, container.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property);
19731986
}
19741987
else if (container.kind === SyntaxKind.Constructor) {
1975-
if (isInJavaScriptFile(node)) {
1976-
// this.foo assignment in a JavaScript class
1977-
// Bind this property to the containing class
1978-
const saveContainer = container;
1979-
container = container.parent;
1980-
bindPropertyOrMethodOrAccessor(node, SymbolFlags.Property, SymbolFlags.None);
1981-
container = saveContainer;
1982-
return;
1988+
// this.foo assignment in a JavaScript class
1989+
// Bind this property to the containing class
1990+
const saveContainer = container;
1991+
container = container.parent;
1992+
const symbol = bindPropertyOrMethodOrAccessor(node, SymbolFlags.Property, SymbolFlags.None);
1993+
if (symbol) {
1994+
// constructor-declared symbols can be overwritten by subsequent method declarations
1995+
(symbol as Symbol).isReplaceableByMethod = true;
19831996
}
1984-
else {
1985-
assignee = container.parent;
1986-
}
1987-
}
1988-
else {
1989-
return;
1997+
container = saveContainer;
19901998
}
1991-
assignee.symbol.members = assignee.symbol.members || createMap<Symbol>();
1992-
// It's acceptable for multiple 'this' assignments of the same identifier to occur
1993-
declareSymbol(assignee.symbol.members, assignee.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property);
19941999
}
19952000

19962001
function bindPrototypePropertyAssignment(node: BinaryExpression) {

‎src/compiler/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2162,6 +2162,7 @@ namespace ts {
21622162
/* @internal */ exportSymbol?: Symbol; // Exported symbol associated with this symbol
21632163
/* @internal */ constEnumOnlyModule?: boolean; // True if module contains only const enums or other modules with only const enums
21642164
/* @internal */ isReferenced?: boolean; // True if the symbol is referenced elsewhere
2165+
/* @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol?
21652166
/* @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments
21662167
}
21672168

‎tests/baselines/reference/multipleDeclarations.js‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,38 @@
11
//// [input.js]
2-
32
function C() {
43
this.m = null;
54
}
65
C.prototype.m = function() {
76
this.nothing();
8-
};
7+
}
8+
class X {
9+
constructor() {
10+
this.m = this.m.bind(this);
11+
this.mistake = 'frankly, complete nonsense';
12+
}
13+
m() {
14+
}
15+
mistake() {
16+
}
17+
}
18+
let x = new X();
19+
X.prototype.mistake = false;
20+
x.m();
21+
x.mistake;
22+
class Y {
23+
mistake() {
24+
}
25+
m() {
26+
}
27+
constructor() {
28+
this.m = this.m.bind(this);
29+
this.mistake = 'even more nonsense';
30+
}
31+
}
32+
Y.prototype.mistake = true;
33+
let y = new Y();
34+
y.m();
35+
y.mistake();
936

1037

1138
//// [output.js]
@@ -15,3 +42,33 @@ function C() {
1542
C.prototype.m = function () {
1643
this.nothing();
1744
};
45+
var X = (function () {
46+
function X() {
47+
this.m = this.m.bind(this);
48+
this.mistake = 'frankly, complete nonsense';
49+
}
50+
X.prototype.m = function () {
51+
};
52+
X.prototype.mistake = function () {
53+
};
54+
return X;
55+
}());
56+
var x = new X();
57+
X.prototype.mistake = false;
58+
x.m();
59+
x.mistake;
60+
var Y = (function () {
61+
function Y() {
62+
this.m = this.m.bind(this);
63+
this.mistake = 'even more nonsense';
64+
}
65+
Y.prototype.mistake = function () {
66+
};
67+
Y.prototype.m = function () {
68+
};
69+
return Y;
70+
}());
71+
Y.prototype.mistake = true;
72+
var y = new Y();
73+
y.m();
74+
y.mistake();

‎tests/baselines/reference/multipleDeclarations.symbols‎

Lines changed: 92 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,106 @@
11
=== tests/cases/conformance/salsa/input.js ===
2-
32
function C() {
43
>C : Symbol(C, Decl(input.js, 0, 0))
54

65
this.m = null;
7-
>m : Symbol(C.m, Decl(input.js, 1, 14), Decl(input.js, 3, 1))
6+
>m : Symbol(C.m, Decl(input.js, 0, 14), Decl(input.js, 2, 1))
87
}
98
C.prototype.m = function() {
10-
>C.prototype : Symbol(C.m, Decl(input.js, 1, 14), Decl(input.js, 3, 1))
9+
>C.prototype : Symbol(C.m, Decl(input.js, 0, 14), Decl(input.js, 2, 1))
1110
>C : Symbol(C, Decl(input.js, 0, 0))
1211
>prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --))
13-
>m : Symbol(C.m, Decl(input.js, 1, 14), Decl(input.js, 3, 1))
12+
>m : Symbol(C.m, Decl(input.js, 0, 14), Decl(input.js, 2, 1))
1413

1514
this.nothing();
1615
>this : Symbol(C, Decl(input.js, 0, 0))
16+
}
17+
class X {
18+
>X : Symbol(X, Decl(input.js, 5, 1))
19+
20+
constructor() {
21+
this.m = this.m.bind(this);
22+
>this.m : Symbol(X.m, Decl(input.js, 10, 5))
23+
>this : Symbol(X, Decl(input.js, 5, 1))
24+
>m : Symbol(X.m, Decl(input.js, 7, 19))
25+
>this.m.bind : Symbol(Function.bind, Decl(lib.d.ts, --, --))
26+
>this.m : Symbol(X.m, Decl(input.js, 10, 5))
27+
>this : Symbol(X, Decl(input.js, 5, 1))
28+
>m : Symbol(X.m, Decl(input.js, 10, 5))
29+
>bind : Symbol(Function.bind, Decl(lib.d.ts, --, --))
30+
>this : Symbol(X, Decl(input.js, 5, 1))
31+
32+
this.mistake = 'frankly, complete nonsense';
33+
>this.mistake : Symbol(X.mistake, Decl(input.js, 12, 5))
34+
>this : Symbol(X, Decl(input.js, 5, 1))
35+
>mistake : Symbol(X.mistake, Decl(input.js, 8, 35))
36+
}
37+
m() {
38+
>m : Symbol(X.m, Decl(input.js, 10, 5))
39+
}
40+
mistake() {
41+
>mistake : Symbol(X.mistake, Decl(input.js, 12, 5))
42+
}
43+
}
44+
let x = new X();
45+
>x : Symbol(x, Decl(input.js, 16, 3))
46+
>X : Symbol(X, Decl(input.js, 5, 1))
47+
48+
X.prototype.mistake = false;
49+
>X.prototype.mistake : Symbol(X.mistake, Decl(input.js, 12, 5))
50+
>X : Symbol(X, Decl(input.js, 5, 1))
51+
>prototype : Symbol(X.prototype)
52+
53+
x.m();
54+
>x.m : Symbol(X.m, Decl(input.js, 10, 5))
55+
>x : Symbol(x, Decl(input.js, 16, 3))
56+
>m : Symbol(X.m, Decl(input.js, 10, 5))
57+
58+
x.mistake;
59+
>x.mistake : Symbol(X.mistake, Decl(input.js, 12, 5))
60+
>x : Symbol(x, Decl(input.js, 16, 3))
61+
>mistake : Symbol(X.mistake, Decl(input.js, 12, 5))
62+
63+
class Y {
64+
>Y : Symbol(Y, Decl(input.js, 19, 10))
65+
66+
mistake() {
67+
>mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35))
68+
}
69+
m() {
70+
>m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19))
71+
}
72+
constructor() {
73+
this.m = this.m.bind(this);
74+
>this.m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19))
75+
>this : Symbol(Y, Decl(input.js, 19, 10))
76+
>m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19))
77+
>this.m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19))
78+
>this : Symbol(Y, Decl(input.js, 19, 10))
79+
>m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19))
80+
>this : Symbol(Y, Decl(input.js, 19, 10))
81+
82+
this.mistake = 'even more nonsense';
83+
>this.mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35))
84+
>this : Symbol(Y, Decl(input.js, 19, 10))
85+
>mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35))
86+
}
87+
}
88+
Y.prototype.mistake = true;
89+
>Y.prototype.mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35))
90+
>Y : Symbol(Y, Decl(input.js, 19, 10))
91+
>prototype : Symbol(Y.prototype)
92+
93+
let y = new Y();
94+
>y : Symbol(y, Decl(input.js, 31, 3))
95+
>Y : Symbol(Y, Decl(input.js, 19, 10))
96+
97+
y.m();
98+
>y.m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19))
99+
>y : Symbol(y, Decl(input.js, 31, 3))
100+
>m : Symbol(Y.m, Decl(input.js, 22, 5), Decl(input.js, 25, 19))
17101

18-
};
102+
y.mistake();
103+
>y.mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35))
104+
>y : Symbol(y, Decl(input.js, 31, 3))
105+
>mistake : Symbol(Y.mistake, Decl(input.js, 20, 9), Decl(input.js, 26, 35))
19106

0 commit comments

Comments
 (0)