-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Function and class properties #13648
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 3 commits
b8329a0
39b3ecb
e8a2173
793d8be
1eb7b91
67957f0
4b8396b
0aa8a6e
90eef89
db0e376
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 |
---|---|---|
|
@@ -265,6 +265,7 @@ namespace ts { | |
return "export="; | ||
case SpecialPropertyAssignmentKind.ExportsProperty: | ||
case SpecialPropertyAssignmentKind.ThisProperty: | ||
case SpecialPropertyAssignmentKind.Property: | ||
// exports.x = ... or this.y = ... | ||
return ((node as BinaryExpression).left as PropertyAccessExpression).name.text; | ||
case SpecialPropertyAssignmentKind.PrototypeProperty: | ||
|
@@ -1921,6 +1922,9 @@ namespace ts { | |
case SpecialPropertyAssignmentKind.ThisProperty: | ||
bindThisPropertyAssignment(<BinaryExpression>node); | ||
break; | ||
case SpecialPropertyAssignmentKind.Property: | ||
bindPropertyAssignment(<BinaryExpression>node); | ||
break; | ||
case SpecialPropertyAssignmentKind.None: | ||
// Nothing to do | ||
break; | ||
|
@@ -2211,8 +2215,12 @@ namespace ts { | |
constructorFunction.parent = classPrototype; | ||
classPrototype.parent = leftSideOfAssignment; | ||
|
||
const funcSymbol = container.locals.get(constructorFunction.text); | ||
if (!funcSymbol || !(funcSymbol.flags & SymbolFlags.Function || isDeclarationOfFunctionExpression(funcSymbol))) { | ||
let funcSymbol = container.locals.get(constructorFunction.text); | ||
if (isDeclarationOfFunctionOrClassExpression(funcSymbol)) { | ||
funcSymbol = (funcSymbol.valueDeclaration as VariableDeclaration).initializer.symbol; | ||
} | ||
|
||
if (!funcSymbol || !(funcSymbol.flags & (SymbolFlags.Function | SymbolFlags.Class))) { | ||
return; | ||
} | ||
|
||
|
@@ -2225,6 +2233,36 @@ namespace ts { | |
declareSymbol(funcSymbol.members, funcSymbol, leftSideOfAssignment, SymbolFlags.Property, SymbolFlags.PropertyExcludes); | ||
} | ||
|
||
function bindPropertyAssignment(node: BinaryExpression) { | ||
// We saw a node of the form 'x.y = z'. Declare a 'member' y on x if x was a function. | ||
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. This comment should be JSDoc comment |
||
|
||
// Look up the function in the local scope, since prototype assignments should | ||
// follow the function declaration | ||
const leftSideOfAssignment = node.left as PropertyAccessExpression; | ||
const target = leftSideOfAssignment.expression as Identifier; | ||
|
||
// Fix up parent pointers since we're going to use these nodes before we bind into them | ||
leftSideOfAssignment.parent = node; | ||
target.parent = leftSideOfAssignment; | ||
|
||
let funcSymbol = container.locals.get(target.text); | ||
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. So this symbol is not necessary only for function declaration but class expression as well? if so, could you rename the variable? |
||
if (isDeclarationOfFunctionOrClassExpression(funcSymbol)) { | ||
funcSymbol = (funcSymbol.valueDeclaration as VariableDeclaration).initializer.symbol; | ||
} | ||
|
||
if (!funcSymbol || !(funcSymbol.flags & (SymbolFlags.Function | SymbolFlags.Class))) { | ||
return; | ||
} | ||
|
||
// Set up the members collection if it doesn't exist already | ||
if (!funcSymbol.exports) { | ||
funcSymbol.exports = createMap<Symbol>(); | ||
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. why you are using 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. Is it because this is the static side of the class, not the instance side? Exports is a confusing name in this case, then. 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. that is where we keep the static properties on classes. members has the instance properties. |
||
} | ||
|
||
// Declare the method/property | ||
declareSymbol(funcSymbol.exports, funcSymbol, leftSideOfAssignment, SymbolFlags.Property, SymbolFlags.PropertyExcludes); | ||
} | ||
|
||
function bindCallExpression(node: CallExpression) { | ||
// We're only inspecting call expressions to detect CommonJS modules, so we can skip | ||
// this check if we've already seen the module indicator | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1366,10 +1366,10 @@ namespace ts { | |
* Returns true if the node is a variable declaration whose initializer is a function expression. | ||
* This function does not test if the node is in a JavaScript file or not. | ||
*/ | ||
export function isDeclarationOfFunctionExpression(s: Symbol) { | ||
export function isDeclarationOfFunctionOrClassExpression(s: Symbol) { | ||
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. Comment is not updated on line 1402 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. fixed. |
||
if (s.valueDeclaration && s.valueDeclaration.kind === SyntaxKind.VariableDeclaration) { | ||
const declaration = s.valueDeclaration as VariableDeclaration; | ||
return declaration.initializer && declaration.initializer.kind === SyntaxKind.FunctionExpression; | ||
return declaration.initializer && (declaration.initializer.kind === SyntaxKind.FunctionExpression || declaration.initializer.kind === SyntaxKind.ClassExpression); | ||
} | ||
return false; | ||
} | ||
|
@@ -1398,6 +1398,10 @@ namespace ts { | |
// module.exports = expr | ||
return SpecialPropertyAssignmentKind.ModuleExports; | ||
} | ||
else { | ||
// F.x = expr | ||
return SpecialPropertyAssignmentKind.Property; | ||
} | ||
} | ||
else if (lhs.expression.kind === SyntaxKind.ThisKeyword) { | ||
return SpecialPropertyAssignmentKind.ThisProperty; | ||
|
@@ -1417,6 +1421,7 @@ namespace ts { | |
} | ||
} | ||
|
||
|
||
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. don't need this extra whitespace 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. nope |
||
return SpecialPropertyAssignmentKind.None; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,22 @@ | ||
=== tests/cases/conformance/classes/classExpressions/classExpression4.ts === | ||
let C = class { | ||
>C : typeof (Anonymous class) | ||
>class { foo() { return new C(); }} : typeof (Anonymous class) | ||
>C : typeof C | ||
>class { foo() { return new C(); }} : typeof C | ||
|
||
foo() { | ||
>foo : () => (Anonymous class) | ||
>foo : () => C | ||
|
||
return new C(); | ||
>new C() : (Anonymous class) | ||
>C : typeof (Anonymous class) | ||
>new C() : C | ||
>C : typeof C | ||
} | ||
}; | ||
let x = (new C).foo(); | ||
>x : (Anonymous class) | ||
>(new C).foo() : (Anonymous class) | ||
>(new C).foo : () => (Anonymous class) | ||
>(new C) : (Anonymous class) | ||
>new C : (Anonymous class) | ||
>C : typeof (Anonymous class) | ||
>foo : () => (Anonymous class) | ||
>x : C | ||
>(new C).foo() : C | ||
>(new C).foo : () => C | ||
>(new C) : C | ||
>new C : C | ||
>C : typeof C | ||
>foo : () => C | ||
|
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.
bindPrototypePropertyAssignment and bindPropertyAssignment share lots of code now, so I think they should either merge or delegate to a third function that does most of the work.