From 914150f2f19583b40be46663628e9bbe3c66b2f4 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Sun, 5 Feb 2017 15:18:27 -0800 Subject: [PATCH 1/2] Widen special JS property declarations to match regular property declarations --- src/compiler/checker.ts | 40 ++++++++++++------- .../jsFileClassPropertyType.errors.txt | 17 ++++++++ .../jsFileClassPropertyType2.errors.txt | 18 +++++++++ .../reference/multipleDeclarations.types | 2 +- ...naturesUseJSDocForOptionalParameters.types | 30 +++++++------- .../cases/compiler/jsFileClassPropertyType.ts | 13 ++++++ .../compiler/jsFileClassPropertyType2.ts | 14 +++++++ .../getJavaScriptSyntacticDiagnostics24.ts | 2 +- 8 files changed, 104 insertions(+), 32 deletions(-) create mode 100644 tests/baselines/reference/jsFileClassPropertyType.errors.txt create mode 100644 tests/baselines/reference/jsFileClassPropertyType2.errors.txt create mode 100644 tests/cases/compiler/jsFileClassPropertyType.ts create mode 100644 tests/cases/compiler/jsFileClassPropertyType2.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b5306771dd7..10c5f5422e3 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -3254,7 +3254,7 @@ namespace ts { type; } - function getTypeForVariableLikeDeclarationFromJSDocComment(declaration: VariableLikeDeclaration) { + function getTypeForDeclarationFromJSDocComment(declaration: Node ) { const jsdocType = getJSDocType(declaration); if (jsdocType) { return getTypeFromTypeNode(jsdocType); @@ -3282,7 +3282,7 @@ namespace ts { // If this is a variable in a JavaScript file, then use the JSDoc type (if it has // one as its type), otherwise fallback to the below standard TS codepaths to // try to figure it out. - const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration); + const type = getTypeForDeclarationFromJSDocComment(declaration); if (type && type !== unknownType) { return type; } @@ -3377,6 +3377,27 @@ namespace ts { return undefined; } + // Return the inferred type for a variable, parameter, or property declaration + function getWidenedTypeForJSSpecialPropertyDeclaration(declaration: Declaration): Type { + const expression = declaration.kind === SyntaxKind.BinaryExpression ? declaration : + declaration.kind === SyntaxKind.PropertyAccessExpression ? getAncestor(declaration, SyntaxKind.BinaryExpression) : + undefined; + + if (!expression) { + return unknownType; + } + + if (expression.flags & NodeFlags.JavaScriptFile) { + // If there is a JSDoc type, use it + const type = getTypeForDeclarationFromJSDocComment(expression.parent); + if (type && type !== unknownType) { + return getWidenedType(type); + } + } + + return getWidenedType(getWidenedLiteralType(checkExpressionCached(expression.right))); + } + // Return the type implied by a binding pattern element. This is the type of the initializer of the element if // one is present. Otherwise, if the element is itself a binding pattern, it is the type implied by the binding // pattern. Otherwise, it is the type any. @@ -3531,18 +3552,7 @@ namespace ts { // * className.prototype.method = expr if (declaration.kind === SyntaxKind.BinaryExpression || declaration.kind === SyntaxKind.PropertyAccessExpression && declaration.parent.kind === SyntaxKind.BinaryExpression) { - // Use JS Doc type if present on parent expression statement - if (declaration.flags & NodeFlags.JavaScriptFile) { - const jsdocType = getJSDocType(declaration.parent); - if (jsdocType) { - return links.type = getTypeFromTypeNode(jsdocType); - } - } - const declaredTypes = map(symbol.declarations, - decl => decl.kind === SyntaxKind.BinaryExpression ? - checkExpressionCached((decl).right) : - checkExpressionCached((decl.parent).right)); - type = getUnionType(declaredTypes, /*subtypeReduction*/ true); + type = getUnionType(map(symbol.declarations, getWidenedTypeForJSSpecialPropertyDeclaration), /*subtypeReduction*/ true); } else { type = getWidenedTypeForVariableLikeDeclaration(declaration, /*reportErrors*/ true); @@ -3585,7 +3595,7 @@ namespace ts { const setter = getDeclarationOfKind(symbol, SyntaxKind.SetAccessor); if (getter && getter.flags & NodeFlags.JavaScriptFile) { - const jsDocType = getTypeForVariableLikeDeclarationFromJSDocComment(getter); + const jsDocType = getTypeForDeclarationFromJSDocComment(getter); if (jsDocType) { return links.type = jsDocType; } diff --git a/tests/baselines/reference/jsFileClassPropertyType.errors.txt b/tests/baselines/reference/jsFileClassPropertyType.errors.txt new file mode 100644 index 00000000000..f3767175710 --- /dev/null +++ b/tests/baselines/reference/jsFileClassPropertyType.errors.txt @@ -0,0 +1,17 @@ +tests/cases/compiler/bar.ts(2,1): error TS2322: Type '"string"' is not assignable to type 'number'. + + +==== tests/cases/compiler/foo.js (0 errors) ==== + + class C { + constructor () { + this.p = 0; + } + } + +==== tests/cases/compiler/bar.ts (1 errors) ==== + + (new C()).p = "string"; + ~~~~~~~~~~~ +!!! error TS2322: Type '"string"' is not assignable to type 'number'. + \ No newline at end of file diff --git a/tests/baselines/reference/jsFileClassPropertyType2.errors.txt b/tests/baselines/reference/jsFileClassPropertyType2.errors.txt new file mode 100644 index 00000000000..d0d138f7f48 --- /dev/null +++ b/tests/baselines/reference/jsFileClassPropertyType2.errors.txt @@ -0,0 +1,18 @@ +tests/cases/compiler/bar.ts(2,18): error TS2345: Argument of type '"string"' is not assignable to parameter of type 'number'. + + +==== tests/cases/compiler/foo.js (0 errors) ==== + + class C { + constructor() { + /** @type {number[]}*/ + this.p = []; + } + } + +==== tests/cases/compiler/bar.ts (1 errors) ==== + + (new C()).p.push("string"); + ~~~~~~~~ +!!! error TS2345: Argument of type '"string"' is not assignable to parameter of type 'number'. + \ No newline at end of file diff --git a/tests/baselines/reference/multipleDeclarations.types b/tests/baselines/reference/multipleDeclarations.types index 195bdc2ad0d..c11bd13dffe 100644 --- a/tests/baselines/reference/multipleDeclarations.types +++ b/tests/baselines/reference/multipleDeclarations.types @@ -21,7 +21,7 @@ C.prototype.m = function() { this.nothing(); >this.nothing() : any >this.nothing : any ->this : { m: () => void; } +>this : { m: any; } >nothing : any } class X { diff --git a/tests/baselines/reference/signaturesUseJSDocForOptionalParameters.types b/tests/baselines/reference/signaturesUseJSDocForOptionalParameters.types index f8822397306..99bf5e15fa7 100644 --- a/tests/baselines/reference/signaturesUseJSDocForOptionalParameters.types +++ b/tests/baselines/reference/signaturesUseJSDocForOptionalParameters.types @@ -15,39 +15,39 @@ function MyClass() { * @returns {MyClass} */ MyClass.prototype.optionalParam = function(required, notRequired) { ->MyClass.prototype.optionalParam = function(required, notRequired) { return this;} : (required: string, notRequired?: string) => { prop: null; optionalParam: any; } +>MyClass.prototype.optionalParam = function(required, notRequired) { return this;} : (required: string, notRequired?: string) => { prop: any; optionalParam: any; } >MyClass.prototype.optionalParam : any >MyClass.prototype : any >MyClass : () => void >prototype : any >optionalParam : any ->function(required, notRequired) { return this;} : (required: string, notRequired?: string) => { prop: null; optionalParam: any; } +>function(required, notRequired) { return this;} : (required: string, notRequired?: string) => { prop: any; optionalParam: any; } >required : string >notRequired : string return this; ->this : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>this : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } }; let pInst = new MyClass(); ->pInst : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } ->new MyClass() : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>pInst : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>new MyClass() : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } >MyClass : () => void let c1 = pInst.optionalParam('hello') ->c1 : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } ->pInst.optionalParam('hello') : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } ->pInst.optionalParam : (required: string, notRequired?: string) => { prop: null; optionalParam: any; } ->pInst : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } ->optionalParam : (required: string, notRequired?: string) => { prop: null; optionalParam: any; } +>c1 : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>pInst.optionalParam('hello') : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>pInst.optionalParam : (required: string, notRequired?: string) => { prop: any; optionalParam: any; } +>pInst : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>optionalParam : (required: string, notRequired?: string) => { prop: any; optionalParam: any; } >'hello' : "hello" let c2 = pInst.optionalParam('hello', null) ->c2 : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } ->pInst.optionalParam('hello', null) : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } ->pInst.optionalParam : (required: string, notRequired?: string) => { prop: null; optionalParam: any; } ->pInst : { prop: null; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } ->optionalParam : (required: string, notRequired?: string) => { prop: null; optionalParam: any; } +>c2 : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>pInst.optionalParam('hello', null) : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>pInst.optionalParam : (required: string, notRequired?: string) => { prop: any; optionalParam: any; } +>pInst : { prop: any; optionalParam: (required: string, notRequired?: string) => typeof MyClass; } +>optionalParam : (required: string, notRequired?: string) => { prop: any; optionalParam: any; } >'hello' : "hello" >null : null diff --git a/tests/cases/compiler/jsFileClassPropertyType.ts b/tests/cases/compiler/jsFileClassPropertyType.ts new file mode 100644 index 00000000000..53540096721 --- /dev/null +++ b/tests/cases/compiler/jsFileClassPropertyType.ts @@ -0,0 +1,13 @@ +// @allowJs: true +// @noEmit: true + +// @filename: foo.js +class C { + constructor () { + this.p = 0; + } +} + +// @filename: bar.ts + +(new C()).p = "string"; diff --git a/tests/cases/compiler/jsFileClassPropertyType2.ts b/tests/cases/compiler/jsFileClassPropertyType2.ts new file mode 100644 index 00000000000..82c36f53147 --- /dev/null +++ b/tests/cases/compiler/jsFileClassPropertyType2.ts @@ -0,0 +1,14 @@ +// @allowJs: true +// @noEmit: true + +// @filename: foo.js +class C { + constructor() { + /** @type {number[]}*/ + this.p = []; + } +} + +// @filename: bar.ts + +(new C()).p.push("string"); diff --git a/tests/cases/fourslash/getJavaScriptSyntacticDiagnostics24.ts b/tests/cases/fourslash/getJavaScriptSyntacticDiagnostics24.ts index d93ec4c1914..59be947e3a1 100644 --- a/tests/cases/fourslash/getJavaScriptSyntacticDiagnostics24.ts +++ b/tests/cases/fourslash/getJavaScriptSyntacticDiagnostics24.ts @@ -12,4 +12,4 @@ //// let x = new Person(100); //// x.canVote/**/; -verify.quickInfoAt("", "(property) Person.canVote: true | 23"); +verify.quickInfoAt("", "(property) Person.canVote: number | boolean"); From 24ddbe4b60a2d755a604a5f485206eb704e4599b Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Thu, 9 Feb 2017 14:55:07 -0800 Subject: [PATCH 2/2] Widen after sub-type-reduction took place --- src/compiler/checker.ts | 6 ++--- .../jsFileClassPropertyType3.errors.txt | 22 +++++++++++++++++++ .../reference/multipleDeclarations.types | 2 +- .../compiler/jsFileClassPropertyType3.ts | 18 +++++++++++++++ 4 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 tests/baselines/reference/jsFileClassPropertyType3.errors.txt create mode 100644 tests/cases/compiler/jsFileClassPropertyType3.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 10c5f5422e3..831126adfa1 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -3378,7 +3378,7 @@ namespace ts { } // Return the inferred type for a variable, parameter, or property declaration - function getWidenedTypeForJSSpecialPropertyDeclaration(declaration: Declaration): Type { + function getTypeForJSSpecialPropertyDeclaration(declaration: Declaration): Type { const expression = declaration.kind === SyntaxKind.BinaryExpression ? declaration : declaration.kind === SyntaxKind.PropertyAccessExpression ? getAncestor(declaration, SyntaxKind.BinaryExpression) : undefined; @@ -3395,7 +3395,7 @@ namespace ts { } } - return getWidenedType(getWidenedLiteralType(checkExpressionCached(expression.right))); + return getWidenedLiteralType(checkExpressionCached(expression.right)); } // Return the type implied by a binding pattern element. This is the type of the initializer of the element if @@ -3552,7 +3552,7 @@ namespace ts { // * className.prototype.method = expr if (declaration.kind === SyntaxKind.BinaryExpression || declaration.kind === SyntaxKind.PropertyAccessExpression && declaration.parent.kind === SyntaxKind.BinaryExpression) { - type = getUnionType(map(symbol.declarations, getWidenedTypeForJSSpecialPropertyDeclaration), /*subtypeReduction*/ true); + type = getWidenedType(getUnionType(map(symbol.declarations, getTypeForJSSpecialPropertyDeclaration), /*subtypeReduction*/ true)); } else { type = getWidenedTypeForVariableLikeDeclaration(declaration, /*reportErrors*/ true); diff --git a/tests/baselines/reference/jsFileClassPropertyType3.errors.txt b/tests/baselines/reference/jsFileClassPropertyType3.errors.txt new file mode 100644 index 00000000000..bf225095b01 --- /dev/null +++ b/tests/baselines/reference/jsFileClassPropertyType3.errors.txt @@ -0,0 +1,22 @@ +tests/cases/compiler/bar.ts(2,1): error TS2322: Type '"string"' is not assignable to type 'number'. + + +==== tests/cases/compiler/foo.js (0 errors) ==== + + class C { + constructor() { + if (cond) { + this.p = null; + } + else { + this.p = 0; + } + } + } + +==== tests/cases/compiler/bar.ts (1 errors) ==== + + (new C()).p = "string"; // Error + ~~~~~~~~~~~ +!!! error TS2322: Type '"string"' is not assignable to type 'number'. + \ No newline at end of file diff --git a/tests/baselines/reference/multipleDeclarations.types b/tests/baselines/reference/multipleDeclarations.types index c11bd13dffe..195bdc2ad0d 100644 --- a/tests/baselines/reference/multipleDeclarations.types +++ b/tests/baselines/reference/multipleDeclarations.types @@ -21,7 +21,7 @@ C.prototype.m = function() { this.nothing(); >this.nothing() : any >this.nothing : any ->this : { m: any; } +>this : { m: () => void; } >nothing : any } class X { diff --git a/tests/cases/compiler/jsFileClassPropertyType3.ts b/tests/cases/compiler/jsFileClassPropertyType3.ts new file mode 100644 index 00000000000..7c240e942fb --- /dev/null +++ b/tests/cases/compiler/jsFileClassPropertyType3.ts @@ -0,0 +1,18 @@ +// @allowJs: true +// @noEmit: true + +// @filename: foo.js +class C { + constructor() { + if (cond) { + this.p = null; + } + else { + this.p = 0; + } + } +} + +// @filename: bar.ts + +(new C()).p = "string"; // Error