Skip to content

Commit 6d9a825

Browse files
sandersnsandersn
authored
Improve binding and jsdoc of chained special js assignments (#23038)
* Search for jsdoc on chained assignments * Fix binding of chained binary expression js-assignments * Test:chained jsdoc+chained prototype assignment * Improve naming
1 parent c02b4f2 commit 6d9a825

18 files changed

+823
-73
lines changed

‎src/compiler/binder.ts

+9-2
Original file line numberDiff line numberDiff line change
@@ -2445,8 +2445,8 @@ namespace ts {
24452445
function bindPropertyAssignment(name: EntityNameExpression, propertyAccess: PropertyAccessEntityNameExpression, isPrototypeProperty: boolean) {
24462446
let symbol = getJSInitializerSymbolFromName(name);
24472447
const isToplevelNamespaceableInitializer = isBinaryExpression(propertyAccess.parent)
2448-
? propertyAccess.parent.parent.parent.kind === SyntaxKind.SourceFile &&
2449-
!!getJavascriptInitializer(propertyAccess.parent.right, isPrototypeAccess(propertyAccess.parent.left))
2448+
? getParentOfBinaryExpression(propertyAccess.parent).parent.kind === SyntaxKind.SourceFile &&
2449+
!!getJavascriptInitializer(getInitializerOfBinaryExpression(propertyAccess.parent), isPrototypeAccess(propertyAccess.parent.left))
24502450
: propertyAccess.parent.parent.kind === SyntaxKind.SourceFile;
24512451
if (!isPrototypeProperty && (!symbol || !(symbol.flags & SymbolFlags.Namespace)) && isToplevelNamespaceableInitializer) {
24522452
// make symbols or add declarations for intermediate containers
@@ -2478,6 +2478,13 @@ namespace ts {
24782478
declareSymbol(symbolTable, symbol, propertyAccess, symbolFlags, symbolExcludes);
24792479
}
24802480

2481+
function getParentOfBinaryExpression(expr: BinaryExpression) {
2482+
while (isBinaryExpression(expr.parent)) {
2483+
expr = expr.parent;
2484+
}
2485+
return expr.parent;
2486+
}
2487+
24812488
function lookupSymbolForPropertyAccess(node: EntityNameExpression, lookupContainer: Node = container): Symbol | undefined {
24822489
if (isIdentifier(node)) {
24832490
return lookupSymbolForNameWorker(lookupContainer, node.escapedText);

‎src/compiler/checker.ts

+4-5
Original file line numberDiff line numberDiff line change
@@ -18218,11 +18218,10 @@ namespace ts {
1821818218
while (parent && parent.kind === SyntaxKind.PropertyAccessExpression) {
1821918219
parent = parent.parent;
1822018220
}
18221-
return parent && isBinaryExpression(parent) &&
18222-
isPrototypeAccess(parent.left) &&
18223-
parent.operatorToken.kind === SyntaxKind.EqualsToken &&
18224-
isObjectLiteralExpression(parent.right) &&
18225-
parent.right;
18221+
if (parent && isBinaryExpression(parent) && isPrototypeAccess(parent.left) && parent.operatorToken.kind === SyntaxKind.EqualsToken) {
18222+
const right = getInitializerOfBinaryExpression(parent);
18223+
return isObjectLiteralExpression(right) && right;
18224+
}
1822618225
}
1822718226

1822818227

‎src/compiler/utilities.ts

+13-2
Original file line numberDiff line numberDiff line change
@@ -1637,7 +1637,7 @@ namespace ts {
16371637
return SpecialPropertyAssignmentKind.ModuleExports;
16381638
}
16391639
else if (isEntityNameExpression(lhs.expression)) {
1640-
if (lhs.name.escapedText === "prototype" && isObjectLiteralExpression(expr.right)) {
1640+
if (lhs.name.escapedText === "prototype" && isObjectLiteralExpression(getInitializerOfBinaryExpression(expr))) {
16411641
// F.prototype = { ... }
16421642
return SpecialPropertyAssignmentKind.Prototype;
16431643
}
@@ -1664,6 +1664,13 @@ namespace ts {
16641664
return SpecialPropertyAssignmentKind.None;
16651665
}
16661666

1667+
export function getInitializerOfBinaryExpression(expr: BinaryExpression) {
1668+
while (isBinaryExpression(expr.right)) {
1669+
expr = expr.right;
1670+
}
1671+
return expr.right;
1672+
}
1673+
16671674
export function isPrototypePropertyAssignment(node: Node): boolean {
16681675
return isBinaryExpression(node) && getSpecialPropertyAssignmentKind(node) === SpecialPropertyAssignmentKind.PrototypeProperty;
16691676
}
@@ -1787,7 +1794,11 @@ namespace ts {
17871794

17881795
function getJSDocCommentsAndTagsWorker(node: Node): void {
17891796
const parent = node.parent;
1790-
if (parent && (parent.kind === SyntaxKind.PropertyAssignment || parent.kind === SyntaxKind.PropertyDeclaration || getNestedModuleDeclaration(parent))) {
1797+
if (parent &&
1798+
(parent.kind === SyntaxKind.PropertyAssignment ||
1799+
parent.kind === SyntaxKind.PropertyDeclaration ||
1800+
isBinaryExpression(parent) && getSpecialPropertyAssignmentKind(parent) !== SpecialPropertyAssignmentKind.None ||
1801+
getNestedModuleDeclaration(parent))) {
17911802
getJSDocCommentsAndTagsWorker(parent);
17921803
}
17931804
// Try to recognize this pattern when node is initializer of variable declaration and JSDoc comments are on containing variable statement.

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

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
tests/cases/conformance/salsa/use.js(5,5): error TS2345: Argument of type '"nope"' is not assignable to parameter of type 'number'.
2+
tests/cases/conformance/salsa/use.js(6,5): error TS2345: Argument of type '"not really"' is not assignable to parameter of type 'number'.
3+
4+
5+
==== tests/cases/conformance/salsa/use.js (2 errors) ====
6+
/// <reference path='./types.d.ts'/>
7+
var mod = require('./mod');
8+
var a = new mod.A()
9+
var b = new mod.B()
10+
a.m('nope')
11+
~~~~~~
12+
!!! error TS2345: Argument of type '"nope"' is not assignable to parameter of type 'number'.
13+
b.m('not really')
14+
~~~~~~~~~~~~
15+
!!! error TS2345: Argument of type '"not really"' is not assignable to parameter of type 'number'.
16+
17+
==== tests/cases/conformance/salsa/types.d.ts (0 errors) ====
18+
declare function require(name: string): any;
19+
declare var exports: any;
20+
==== tests/cases/conformance/salsa/mod.js (0 errors) ====
21+
/// <reference path='./types.d.ts'/>
22+
var A = function() {
23+
this.a = 1
24+
}
25+
var B = function() {
26+
this.b = 2
27+
}
28+
exports.A = A
29+
exports.B = B
30+
A.prototype = B.prototype = {
31+
/** @param {number} n */
32+
m(n) {
33+
return n + 1
34+
}
35+
}
36+

‎tests/baselines/reference/chainedPrototypeAssignment.symbols

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
=== tests/cases/conformance/salsa/use.js ===
2+
/// <reference path='./types.d.ts'/>
3+
var mod = require('./mod');
4+
>mod : Symbol(mod, Decl(use.js, 1, 3))
5+
>require : Symbol(require, Decl(types.d.ts, 0, 0))
6+
>'./mod' : Symbol("tests/cases/conformance/salsa/mod", Decl(mod.js, 0, 0))
7+
8+
var a = new mod.A()
9+
>a : Symbol(a, Decl(use.js, 2, 3))
10+
>mod.A : Symbol(A, Decl(mod.js, 6, 1))
11+
>mod : Symbol(mod, Decl(use.js, 1, 3))
12+
>A : Symbol(A, Decl(mod.js, 6, 1))
13+
14+
var b = new mod.B()
15+
>b : Symbol(b, Decl(use.js, 3, 3))
16+
>mod.B : Symbol(B, Decl(mod.js, 7, 13))
17+
>mod : Symbol(mod, Decl(use.js, 1, 3))
18+
>B : Symbol(B, Decl(mod.js, 7, 13))
19+
20+
a.m('nope')
21+
>a.m : Symbol(m, Decl(mod.js, 9, 29))
22+
>a : Symbol(a, Decl(use.js, 2, 3))
23+
>m : Symbol(m, Decl(mod.js, 9, 29))
24+
25+
b.m('not really')
26+
>b.m : Symbol(m, Decl(mod.js, 9, 29))
27+
>b : Symbol(b, Decl(use.js, 3, 3))
28+
>m : Symbol(m, Decl(mod.js, 9, 29))
29+
30+
=== tests/cases/conformance/salsa/types.d.ts ===
31+
declare function require(name: string): any;
32+
>require : Symbol(require, Decl(types.d.ts, 0, 0))
33+
>name : Symbol(name, Decl(types.d.ts, 0, 25))
34+
35+
declare var exports: any;
36+
>exports : Symbol(exports, Decl(types.d.ts, 1, 11))
37+
38+
=== tests/cases/conformance/salsa/mod.js ===
39+
/// <reference path='./types.d.ts'/>
40+
var A = function() {
41+
>A : Symbol(A, Decl(mod.js, 1, 3), Decl(mod.js, 8, 13))
42+
43+
this.a = 1
44+
>a : Symbol(A.a, Decl(mod.js, 1, 20))
45+
}
46+
var B = function() {
47+
>B : Symbol(B, Decl(mod.js, 4, 3), Decl(mod.js, 9, 13))
48+
49+
this.b = 2
50+
>b : Symbol(B.b, Decl(mod.js, 4, 20))
51+
}
52+
exports.A = A
53+
>exports.A : Symbol(A, Decl(mod.js, 6, 1))
54+
>exports : Symbol(A, Decl(mod.js, 6, 1))
55+
>A : Symbol(A, Decl(mod.js, 6, 1))
56+
>A : Symbol(A, Decl(mod.js, 1, 3), Decl(mod.js, 8, 13))
57+
58+
exports.B = B
59+
>exports.B : Symbol(B, Decl(mod.js, 7, 13))
60+
>exports : Symbol(B, Decl(mod.js, 7, 13))
61+
>B : Symbol(B, Decl(mod.js, 7, 13))
62+
>B : Symbol(B, Decl(mod.js, 4, 3), Decl(mod.js, 9, 13))
63+
64+
A.prototype = B.prototype = {
65+
>A.prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --))
66+
>A : Symbol(A, Decl(mod.js, 1, 3), Decl(mod.js, 8, 13))
67+
>prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --))
68+
>B.prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --))
69+
>B : Symbol(B, Decl(mod.js, 4, 3), Decl(mod.js, 9, 13))
70+
>prototype : Symbol(Function.prototype, Decl(lib.d.ts, --, --))
71+
72+
/** @param {number} n */
73+
m(n) {
74+
>m : Symbol(m, Decl(mod.js, 9, 29))
75+
>n : Symbol(n, Decl(mod.js, 11, 6))
76+
77+
return n + 1
78+
>n : Symbol(n, Decl(mod.js, 11, 6))
79+
}
80+
}
81+

‎tests/baselines/reference/chainedPrototypeAssignment.types

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
=== tests/cases/conformance/salsa/use.js ===
2+
/// <reference path='./types.d.ts'/>
3+
var mod = require('./mod');
4+
>mod : typeof "tests/cases/conformance/salsa/mod"
5+
>require('./mod') : typeof "tests/cases/conformance/salsa/mod"
6+
>require : (name: string) => any
7+
>'./mod' : "./mod"
8+
9+
var a = new mod.A()
10+
>a : { a: number; } & { [x: string]: any; m(n: number): number; }
11+
>new mod.A() : { a: number; } & { [x: string]: any; m(n: number): number; }
12+
>mod.A : () => void
13+
>mod : typeof "tests/cases/conformance/salsa/mod"
14+
>A : () => void
15+
16+
var b = new mod.B()
17+
>b : { b: number; } & { [x: string]: any; m(n: number): number; }
18+
>new mod.B() : { b: number; } & { [x: string]: any; m(n: number): number; }
19+
>mod.B : () => void
20+
>mod : typeof "tests/cases/conformance/salsa/mod"
21+
>B : () => void
22+
23+
a.m('nope')
24+
>a.m('nope') : number
25+
>a.m : (n: number) => number
26+
>a : { a: number; } & { [x: string]: any; m(n: number): number; }
27+
>m : (n: number) => number
28+
>'nope' : "nope"
29+
30+
b.m('not really')
31+
>b.m('not really') : number
32+
>b.m : (n: number) => number
33+
>b : { b: number; } & { [x: string]: any; m(n: number): number; }
34+
>m : (n: number) => number
35+
>'not really' : "not really"
36+
37+
=== tests/cases/conformance/salsa/types.d.ts ===
38+
declare function require(name: string): any;
39+
>require : (name: string) => any
40+
>name : string
41+
42+
declare var exports: any;
43+
>exports : any
44+
45+
=== tests/cases/conformance/salsa/mod.js ===
46+
/// <reference path='./types.d.ts'/>
47+
var A = function() {
48+
>A : () => void
49+
>function() { this.a = 1} : () => void
50+
51+
this.a = 1
52+
>this.a = 1 : 1
53+
>this.a : any
54+
>this : any
55+
>a : any
56+
>1 : 1
57+
}
58+
var B = function() {
59+
>B : () => void
60+
>function() { this.b = 2} : () => void
61+
62+
this.b = 2
63+
>this.b = 2 : 2
64+
>this.b : any
65+
>this : any
66+
>b : any
67+
>2 : 2
68+
}
69+
exports.A = A
70+
>exports.A = A : () => void
71+
>exports.A : () => void
72+
>exports : typeof "tests/cases/conformance/salsa/mod"
73+
>A : () => void
74+
>A : () => void
75+
76+
exports.B = B
77+
>exports.B = B : () => void
78+
>exports.B : () => void
79+
>exports : typeof "tests/cases/conformance/salsa/mod"
80+
>B : () => void
81+
>B : () => void
82+
83+
A.prototype = B.prototype = {
84+
>A.prototype = B.prototype = { /** @param {number} n */ m(n) { return n + 1 }} : { [x: string]: any; m(n: number): number; }
85+
>A.prototype : any
86+
>A : () => void
87+
>prototype : any
88+
>B.prototype = { /** @param {number} n */ m(n) { return n + 1 }} : { [x: string]: any; m(n: number): number; }
89+
>B.prototype : any
90+
>B : () => void
91+
>prototype : any
92+
>{ /** @param {number} n */ m(n) { return n + 1 }} : { [x: string]: any; m(n: number): number; }
93+
94+
/** @param {number} n */
95+
m(n) {
96+
>m : (n: number) => number
97+
>n : number
98+
99+
return n + 1
100+
>n + 1 : number
101+
>n : number
102+
>1 : 1
103+
}
104+
}
105+

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

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
tests/cases/conformance/jsdoc/a.js(12,21): error TS2339: Property 'x' does not exist on type 'typeof A'.
2+
tests/cases/conformance/jsdoc/a.js(15,5): error TS2345: Argument of type '"no"' is not assignable to parameter of type 'number'.
3+
tests/cases/conformance/jsdoc/a.js(16,5): error TS2345: Argument of type '"not really"' is not assignable to parameter of type 'number'.
4+
tests/cases/conformance/jsdoc/a.js(17,5): error TS2345: Argument of type '"still no"' is not assignable to parameter of type 'number'.
5+
tests/cases/conformance/jsdoc/a.js(18,5): error TS2345: Argument of type '"not here either"' is not assignable to parameter of type 'number'.
6+
tests/cases/conformance/jsdoc/a.js(19,1): error TS2322: Type '10' is not assignable to type '1'.
7+
8+
9+
==== tests/cases/conformance/jsdoc/a.js (6 errors) ====
10+
function A () {
11+
this.x = 1
12+
/** @type {1} */
13+
this.first = this.second = 1
14+
}
15+
/** @param {number} n */
16+
A.prototype.y = A.prototype.z = function f(n) {
17+
return n + this.x
18+
}
19+
/** @param {number} m */
20+
A.s = A.t = function g(m) {
21+
return m + this.x
22+
~
23+
!!! error TS2339: Property 'x' does not exist on type 'typeof A'.
24+
}
25+
var a = new A()
26+
a.y('no') // error
27+
~~~~
28+
!!! error TS2345: Argument of type '"no"' is not assignable to parameter of type 'number'.
29+
a.z('not really') // error
30+
~~~~~~~~~~~~
31+
!!! error TS2345: Argument of type '"not really"' is not assignable to parameter of type 'number'.
32+
A.s('still no') // error
33+
~~~~~~~~~~
34+
!!! error TS2345: Argument of type '"still no"' is not assignable to parameter of type 'number'.
35+
A.t('not here either') // error
36+
~~~~~~~~~~~~~~~~~
37+
!!! error TS2345: Argument of type '"not here either"' is not assignable to parameter of type 'number'.
38+
a.first = 10 // error: '10' isn't assignable to '1'
39+
~~~~~~~
40+
!!! error TS2322: Type '10' is not assignable to type '1'.
41+

0 commit comments

Comments
 (0)