Skip to content

Commit 4ec6848

Browse files
mhegazymhegazy
authored
Merge pull request microsoft#13903 from Microsoft/jsPropertyWidening
Widen special JS property declarations to match regular property declarations
2 parents 47203c5 + 24ddbe4 commit 4ec6848

9 files changed

Lines changed: 143 additions & 31 deletions

‎src/compiler/checker.ts‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3259,7 +3259,7 @@ namespace ts {
32593259
type;
32603260
}
32613261

3262-
function getTypeForVariableLikeDeclarationFromJSDocComment(declaration: VariableLikeDeclaration) {
3262+
function getTypeForDeclarationFromJSDocComment(declaration: Node ) {
32633263
const jsdocType = getJSDocType(declaration);
32643264
if (jsdocType) {
32653265
return getTypeFromTypeNode(jsdocType);
@@ -3287,7 +3287,7 @@ namespace ts {
32873287
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
32883288
// one as its type), otherwise fallback to the below standard TS codepaths to
32893289
// try to figure it out.
3290-
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
3290+
const type = getTypeForDeclarationFromJSDocComment(declaration);
32913291
if (type && type !== unknownType) {
32923292
return type;
32933293
}
@@ -3382,6 +3382,27 @@ namespace ts {
33823382
return undefined;
33833383
}
33843384

3385+
// Return the inferred type for a variable, parameter, or property declaration
3386+
function getTypeForJSSpecialPropertyDeclaration(declaration: Declaration): Type {
3387+
const expression = declaration.kind === SyntaxKind.BinaryExpression ? <BinaryExpression>declaration :
3388+
declaration.kind === SyntaxKind.PropertyAccessExpression ? <BinaryExpression>getAncestor(declaration, SyntaxKind.BinaryExpression) :
3389+
undefined;
3390+
3391+
if (!expression) {
3392+
return unknownType;
3393+
}
3394+
3395+
if (expression.flags & NodeFlags.JavaScriptFile) {
3396+
// If there is a JSDoc type, use it
3397+
const type = getTypeForDeclarationFromJSDocComment(expression.parent);
3398+
if (type && type !== unknownType) {
3399+
return getWidenedType(type);
3400+
}
3401+
}
3402+
3403+
return getWidenedLiteralType(checkExpressionCached(expression.right));
3404+
}
3405+
33853406
// Return the type implied by a binding pattern element. This is the type of the initializer of the element if
33863407
// one is present. Otherwise, if the element is itself a binding pattern, it is the type implied by the binding
33873408
// pattern. Otherwise, it is the type any.
@@ -3536,18 +3557,7 @@ namespace ts {
35363557
// * className.prototype.method = expr
35373558
if (declaration.kind === SyntaxKind.BinaryExpression ||
35383559
declaration.kind === SyntaxKind.PropertyAccessExpression && declaration.parent.kind === SyntaxKind.BinaryExpression) {
3539-
// Use JS Doc type if present on parent expression statement
3540-
if (declaration.flags & NodeFlags.JavaScriptFile) {
3541-
const jsdocType = getJSDocType(declaration.parent);
3542-
if (jsdocType) {
3543-
return links.type = getTypeFromTypeNode(jsdocType);
3544-
}
3545-
}
3546-
const declaredTypes = map(symbol.declarations,
3547-
decl => decl.kind === SyntaxKind.BinaryExpression ?
3548-
checkExpressionCached((<BinaryExpression>decl).right) :
3549-
checkExpressionCached((<BinaryExpression>decl.parent).right));
3550-
type = getUnionType(declaredTypes, /*subtypeReduction*/ true);
3560+
type = getWidenedType(getUnionType(map(symbol.declarations, getTypeForJSSpecialPropertyDeclaration), /*subtypeReduction*/ true));
35513561
}
35523562
else {
35533563
type = getWidenedTypeForVariableLikeDeclaration(<VariableLikeDeclaration>declaration, /*reportErrors*/ true);
@@ -3590,7 +3600,7 @@ namespace ts {
35903600
const setter = <AccessorDeclaration>getDeclarationOfKind(symbol, SyntaxKind.SetAccessor);
35913601

35923602
if (getter && getter.flags & NodeFlags.JavaScriptFile) {
3593-
const jsDocType = getTypeForVariableLikeDeclarationFromJSDocComment(getter);
3603+
const jsDocType = getTypeForDeclarationFromJSDocComment(getter);
35943604
if (jsDocType) {
35953605
return links.type = jsDocType;
35963606
}

‎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/jsFileClassPropertyType3.errors.txt‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
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+
if (cond) {
9+
this.p = null;
10+
}
11+
else {
12+
this.p = 0;
13+
}
14+
}
15+
}
16+
17+
==== tests/cases/compiler/bar.ts (1 errors) ====
18+
19+
(new C()).p = "string"; // Error
20+
~~~~~~~~~~~
21+
!!! error TS2322: Type '"string"' is not assignable to type 'number'.
22+

‎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/compiler/jsFileClassPropertyType3.ts‎

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

‎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)