-
Notifications
You must be signed in to change notification settings - Fork 13k
JavaScript prototype class inference #5876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
b31b45f
bc3d95c
3b72131
a3a5c16
eaeeb1f
fb83ee0
c3b59d1
c4b0b62
fabc43d
6bb62d6
fcd00a5
c97dfff
50892ac
fcfc424
37f3ff8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2664,9 +2664,18 @@ namespace ts { | |
if (declaration.kind === SyntaxKind.BinaryExpression) { | ||
return links.type = checkExpression((<BinaryExpression>declaration).right); | ||
} | ||
// Handle exports.p = expr | ||
if (declaration.kind === SyntaxKind.PropertyAccessExpression) { | ||
return checkExpressionCached((<BinaryExpression>declaration.parent).right); | ||
// Declarations only exist for property access expressions for certain | ||
// special assignment kinds | ||
if (declaration.parent.kind === SyntaxKind.BinaryExpression) { | ||
// Handle exports.p = expr or this.p = expr or className.prototype.method = expr | ||
return links.type = checkExpressionCached((<BinaryExpression>declaration.parent).right); | ||
} | ||
else { | ||
// Declaration for className.prototype in inferred JS class | ||
const type = createAnonymousType(symbol, symbol.members, emptyArray, emptyArray, undefined, undefined); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my previous comment about caching. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I'm not sure I understand what case this code is handling. |
||
return links.type = type; | ||
} | ||
} | ||
// Handle variable, parameter or property | ||
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) { | ||
|
@@ -6888,6 +6897,23 @@ namespace ts { | |
const symbol = getSymbolOfNode(container.parent); | ||
return container.flags & NodeFlags.Static ? getTypeOfSymbol(symbol) : (<InterfaceType>getDeclaredTypeOfSymbol(symbol)).thisType; | ||
} | ||
|
||
// If this is a function in a JS file, it might be a class method. Check if it's the RHS | ||
// of a x.prototype.y = function [name]() { .... } | ||
if (isInJavaScriptFile(node) && container.kind === SyntaxKind.FunctionExpression) { | ||
if (getSpecialPropertyAssignmentKind(container.parent) === SpecialPropertyAssignmentKind.PrototypeProperty) { | ||
// Get the 'x' of 'x.prototype.y = f' (here, 'f' is 'container') | ||
const className = (((container.parent as BinaryExpression) // x.protoype.y = f | ||
.left as PropertyAccessExpression) // x.prototype.y | ||
.expression as PropertyAccessExpression) // x.prototype | ||
.expression; // x | ||
const classSymbol = checkExpression(className).symbol; | ||
if (classSymbol.members) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this needs to be |
||
return createAnonymousType(undefined, classSymbol.members, emptyArray, emptyArray, /*stringIndexType*/ undefined, /*numberIndexType*/ undefined); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my previous comment about caching. |
||
} | ||
} | ||
} | ||
|
||
return anyType; | ||
} | ||
|
||
|
@@ -9630,8 +9656,14 @@ namespace ts { | |
declaration.kind !== SyntaxKind.ConstructSignature && | ||
declaration.kind !== SyntaxKind.ConstructorType) { | ||
|
||
// When resolved signature is a call signature (and not a construct signature) the result type is any | ||
if (compilerOptions.noImplicitAny) { | ||
// When resolved signature is a call signature (and not a construct signature) the result type is any, unless | ||
// the declaring function had members created through 'x.prototype.y = expr' or 'this.y = expr' psuedodeclarations | ||
// in a JS file | ||
const funcSymbol = checkExpression(node.expression).symbol; | ||
if (funcSymbol && funcSymbol.members && (funcSymbol.flags & SymbolFlags.Function)) { | ||
return createAnonymousType(undefined, funcSymbol.members, emptyArray, emptyArray, /*stringIndexType*/ undefined, /*numberIndexType*/ undefined); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to create this type once and cache it. Right now you're creating a brand new type every time which allocates a lot of objects and defeats all the type comparison caching we do. |
||
} | ||
else if (compilerOptions.noImplicitAny) { | ||
error(node, Diagnostics.new_expression_whose_target_lacks_a_construct_signature_implicitly_has_an_any_type); | ||
} | ||
return anyType; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
///<reference path="fourslash.ts" /> | ||
|
||
// Assignments to the 'prototype' property of a function create a class | ||
|
||
// @allowNonTsExtensions: true | ||
// @Filename: myMod.js | ||
//// function myCtor(x) { | ||
//// } | ||
//// myCtor.prototype.foo = function() { return 32 }; | ||
//// myCtor.prototype.bar = function() { return '' }; | ||
//// | ||
//// var m = new myCtor(10); | ||
//// m/*1*/ | ||
//// var a = m.foo; | ||
//// a/*2*/ | ||
//// var b = a(); | ||
//// b/*3*/ | ||
//// var c = m.bar(); | ||
//// c/*4*/ | ||
|
||
|
||
// Members of the class instance | ||
goTo.marker('1'); | ||
edit.insert('.'); | ||
verify.memberListContains('foo', undefined, undefined, 'property'); | ||
verify.memberListContains('bar', undefined, undefined, 'property'); | ||
edit.backspace(); | ||
|
||
// Members of a class method (1) | ||
goTo.marker('2'); | ||
edit.insert('.'); | ||
verify.memberListContains('length', undefined, undefined, 'property'); | ||
edit.backspace(); | ||
|
||
// Members of the invocation of a class method (1) | ||
goTo.marker('3'); | ||
edit.insert('.'); | ||
verify.memberListContains('toFixed', undefined, undefined, 'method'); | ||
verify.not.memberListContains('substr', undefined, undefined, 'method'); | ||
edit.backspace(); | ||
|
||
// Members of the invocation of a class method (2) | ||
goTo.marker('4'); | ||
edit.insert('.'); | ||
verify.memberListContains('substr', undefined, undefined, 'method'); | ||
verify.not.memberListContains('toFixed', undefined, undefined, 'method'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
///<reference path="fourslash.ts" /> | ||
|
||
// Assignments to 'this' in the constructorish body create | ||
// properties with those names | ||
|
||
// @allowNonTsExtensions: true | ||
// @Filename: myMod.js | ||
//// function myCtor(x) { | ||
//// this.qua = 10; | ||
//// } | ||
//// myCtor.prototype.foo = function() { return 32 }; | ||
//// myCtor.prototype.bar = function() { return '' }; | ||
//// | ||
//// var m = new myCtor(10); | ||
//// m/*1*/ | ||
//// var x = m.qua; | ||
//// x/*2*/ | ||
//// myCtor/*3*/ | ||
|
||
// Verify the instance property exists | ||
goTo.marker('1'); | ||
edit.insert('.'); | ||
verify.completionListContains('qua', undefined, undefined, 'property'); | ||
edit.backspace(); | ||
|
||
// Verify the type of the instance property | ||
goTo.marker('2'); | ||
edit.insert('.'); | ||
verify.completionListContains('toFixed', undefined, undefined, 'method'); | ||
|
||
goTo.marker('3'); | ||
edit.insert('.'); | ||
// Make sure symbols don't leak out into the constructor | ||
verify.completionListContains('qua', undefined, undefined, 'warning'); | ||
verify.completionListContains('foo', undefined, undefined, 'warning'); | ||
verify.completionListContains('bar', undefined, undefined, 'warning'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
///<reference path="fourslash.ts" /> | ||
|
||
// Inside an inferred method body, the type of 'this' is the class type | ||
|
||
// @allowNonTsExtensions: true | ||
// @Filename: myMod.js | ||
//// function myCtor(x) { | ||
//// this.qua = 10; | ||
//// } | ||
//// myCtor.prototype.foo = function() { return this/**/; }; | ||
//// myCtor.prototype.bar = function() { return '' }; | ||
//// | ||
|
||
goTo.marker(); | ||
edit.insert('.'); | ||
|
||
// Check members of the function | ||
verify.completionListContains('foo', undefined, undefined, 'property'); | ||
verify.completionListContains('bar', undefined, undefined, 'property'); | ||
verify.completionListContains('qua', undefined, undefined, 'property'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't guarantee that the prototype assignment immediately follows. Also, what if
classId
denotes a non-function, such as a variable or an interface declaration? I'm thinking you should instead check that the prototype assignment is immediately contained in a statement list and that the immediately preceding statement is a function declaration.