Skip to content

Commit 914150f

Browse files
mhegazymhegazy
committed
Widen special JS property declarations to match regular property declarations
1 parent 12e8f91 commit 914150f

8 files changed

Lines changed: 104 additions & 32 deletions

‎src/compiler/checker.ts‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3254,7 +3254,7 @@ namespace ts {
32543254
type;
32553255
}
32563256

3257-
function getTypeForVariableLikeDeclarationFromJSDocComment(declaration: VariableLikeDeclaration) {
3257+
function getTypeForDeclarationFromJSDocComment(declaration: Node ) {
32583258
const jsdocType = getJSDocType(declaration);
32593259
if (jsdocType) {
32603260
return getTypeFromTypeNode(jsdocType);
@@ -3282,7 +3282,7 @@ namespace ts {
32823282
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
32833283
// one as its type), otherwise fallback to the below standard TS codepaths to
32843284
// try to figure it out.
3285-
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
3285+
const type = getTypeForDeclarationFromJSDocComment(declaration);
32863286
if (type && type !== unknownType) {
32873287
return type;
32883288
}
@@ -3377,6 +3377,27 @@ namespace ts {
33773377
return undefined;
33783378
}
33793379

3380+
// Return the inferred type for a variable, parameter, or property declaration
3381+
function getWidenedTypeForJSSpecialPropertyDeclaration(declaration: Declaration): Type {
3382+
const expression = declaration.kind === SyntaxKind.BinaryExpression ? <BinaryExpression>declaration :
3383+
declaration.kind === SyntaxKind.PropertyAccessExpression ? <BinaryExpression>getAncestor(declaration, SyntaxKind.BinaryExpression) :
3384+
undefined;
3385+
3386+
if (!expression) {
3387+
return unknownType;
3388+
}
3389+
3390+
if (expression.flags & NodeFlags.JavaScriptFile) {
3391+
// If there is a JSDoc type, use it
3392+
const type = getTypeForDeclarationFromJSDocComment(expression.parent);
3393+
if (type && type !== unknownType) {
3394+
return getWidenedType(type);
3395+
}
3396+
}
3397+
3398+
return getWidenedType(getWidenedLiteralType(checkExpressionCached(expression.right)));
3399+
}
3400+
33803401
// Return the type implied by a binding pattern element. This is the type of the initializer of the element if
33813402
// one is present. Otherwise, if the element is itself a binding pattern, it is the type implied by the binding
33823403
// pattern. Otherwise, it is the type any.
@@ -3531,18 +3552,7 @@ namespace ts {
35313552
// * className.prototype.method = expr
35323553
if (declaration.kind === SyntaxKind.BinaryExpression ||
35333554
declaration.kind === SyntaxKind.PropertyAccessExpression && declaration.parent.kind === SyntaxKind.BinaryExpression) {
3534-
// Use JS Doc type if present on parent expression statement
3535-
if (declaration.flags & NodeFlags.JavaScriptFile) {
3536-
const jsdocType = getJSDocType(declaration.parent);
3537-
if (jsdocType) {
3538-
return links.type = getTypeFromTypeNode(jsdocType);
3539-
}
3540-
}
3541-
const declaredTypes = map(symbol.declarations,
3542-
decl => decl.kind === SyntaxKind.BinaryExpression ?
3543-
checkExpressionCached((<BinaryExpression>decl).right) :
3544-
checkExpressionCached((<BinaryExpression>decl.parent).right));
3545-
type = getUnionType(declaredTypes, /*subtypeReduction*/ true);
3555+
type = getUnionType(map(symbol.declarations, getWidenedTypeForJSSpecialPropertyDeclaration), /*subtypeReduction*/ true);
35463556
}
35473557
else {
35483558
type = getWidenedTypeForVariableLikeDeclaration(<VariableLikeDeclaration>declaration, /*reportErrors*/ true);
@@ -3585,7 +3595,7 @@ namespace ts {
35853595
const setter = <AccessorDeclaration>getDeclarationOfKind(symbol, SyntaxKind.SetAccessor);
35863596

35873597
if (getter && getter.flags & NodeFlags.JavaScriptFile) {
3588-
const jsDocType = getTypeForVariableLikeDeclarationFromJSDocComment(getter);
3598+
const jsDocType = getTypeForDeclarationFromJSDocComment(getter);
35893599
if (jsDocType) {
35903600
return links.type = jsDocType;
35913601
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
tests/cases/compiler/bar.ts(2,1): error TS2322: Type '"string"' is not assignable to type 'number'.
2+
3+
4+
==== tests/cases/compiler/foo.js (0 errors) ====
5+
6+
class C {
7+
constructor () {
8+
this.p = 0;
9+
}
10+
}
11+
12+
==== tests/cases/compiler/bar.ts (1 errors) ====
13+
14+
(new C()).p = "string";
15+
~~~~~~~~~~~
16+
!!! error TS2322: Type '"string"' is not assignable to type 'number'.
17+

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
tests/cases/compiler/bar.ts(2,18): error TS2345: Argument of type '"string"' is not assignable to parameter of type 'number'.
2+
3+
4+
==== tests/cases/compiler/foo.js (0 errors) ====
5+
6+
class C {
7+
constructor() {
8+
/** @type {number[]}*/
9+
this.p = [];
10+
}
11+
}
12+
13+
==== tests/cases/compiler/bar.ts (1 errors) ====
14+
15+
(new C()).p.push("string");
16+
~~~~~~~~
17+
!!! error TS2345: Argument of type '"string"' is not assignable to parameter of type 'number'.
18+

‎tests/baselines/reference/multipleDeclarations.types‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ C.prototype.m = function() {
2121
this.nothing();
2222
>this.nothing() : any
2323
>this.nothing : any
24-
>this : { m: () => void; }
24+
>this : { m: any; }
2525
>nothing : any
2626
}
2727
class X {

‎tests/baselines/reference/signaturesUseJSDocForOptionalParameters.types‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,39 +15,39 @@ function MyClass() {
1515
* @returns {MyClass}
1616
*/
1717
MyClass.prototype.optionalParam = function(required, notRequired) {
18-
>MyClass.prototype.optionalParam = function(required, notRequired) { return this;} : (required: string, notRequired?: string) => { prop: null; optionalParam: any; }
18+
>MyClass.prototype.optionalParam = function(required, notRequired) { return this;} : (required: string, notRequired?: string) => { prop: any; optionalParam: any; }
1919
>MyClass.prototype.optionalParam : any
2020
>MyClass.prototype : any
2121
>MyClass : () => void
2222
>prototype : any
2323
>optionalParam : any
24-
>function(required, notRequired) { return this;} : (required: string, notRequired?: string) => { prop: null; optionalParam: any; }
24+
>function(required, notRequired) { return this;} : (required: string, notRequired?: string) => { prop: any; optionalParam: any; }
2525
>required : string
2626
>notRequired : string
2727

2828
return this;
29-
>this : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
29+
>this : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
3030

3131
};
3232
let pInst = new MyClass();
33-
>pInst : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
34-
>new MyClass() : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
33+
>pInst : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
34+
>new MyClass() : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
3535
>MyClass : () => void
3636

3737
let c1 = pInst.optionalParam('hello')
38-
>c1 : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
39-
>pInst.optionalParam('hello') : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
40-
>pInst.optionalParam : (required: string, notRequired?: string) => { prop: null; optionalParam: any; }
41-
>pInst : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
42-
>optionalParam : (required: string, notRequired?: string) => { prop: null; optionalParam: any; }
38+
>c1 : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
39+
>pInst.optionalParam('hello') : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
40+
>pInst.optionalParam : (required: string, notRequired?: string) => { prop: any; optionalParam: any; }
41+
>pInst : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
42+
>optionalParam : (required: string, notRequired?: string) => { prop: any; optionalParam: any; }
4343
>'hello' : "hello"
4444

4545
let c2 = pInst.optionalParam('hello', null)
46-
>c2 : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
47-
>pInst.optionalParam('hello', null) : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
48-
>pInst.optionalParam : (required: string, notRequired?: string) => { prop: null; optionalParam: any; }
49-
>pInst : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
50-
>optionalParam : (required: string, notRequired?: string) => { prop: null; optionalParam: any; }
46+
>c2 : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
47+
>pInst.optionalParam('hello', null) : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
48+
>pInst.optionalParam : (required: string, notRequired?: string) => { prop: any; optionalParam: any; }
49+
>pInst : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; }
50+
>optionalParam : (required: string, notRequired?: string) => { prop: any; optionalParam: any; }
5151
>'hello' : "hello"
5252
>null : null
5353

‎tests/cases/compiler/jsFileClassPropertyType.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// @allowJs: true
2+
// @noEmit: true
3+
4+
// @filename: foo.js
5+
class C {
6+
constructor () {
7+
this.p = 0;
8+
}
9+
}
10+
11+
// @filename: bar.ts
12+
13+
(new C()).p = "string";

‎tests/cases/compiler/jsFileClassPropertyType2.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// @allowJs: true
2+
// @noEmit: true
3+
4+
// @filename: foo.js
5+
class C {
6+
constructor() {
7+
/** @type {number[]}*/
8+
this.p = [];
9+
}
10+
}
11+
12+
// @filename: bar.ts
13+
14+
(new C()).p.push("string");

‎tests/cases/fourslash/getJavaScriptSyntacticDiagnostics24.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@
1212
//// let x = new Person(100);
1313
//// x.canVote/**/;
1414

15-
verify.quickInfoAt("", "(property) Person.canVote: true | 23");
15+
verify.quickInfoAt("", "(property) Person.canVote: number | boolean");

0 commit comments

Comments
 (0)