Function and class properties #13648
Conversation
There was a problem hiding this comment.
Could add tests for this "Also for anonymous function and class expression, that are defined in a variable declaration, e.g. var x = function() {..} use the name of the variable (e.g. x) for error reporting purposes." I didn't see it updated in tests (aside from symbol baselines)
src/compiler/binder.ts
Outdated
| @@ -2230,6 +2238,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.
This comment should be JSDoc comment
| ////} | ||
| ////C.prototype.[|z|] = 1; | ||
| ////var t = new C(12); | ||
| ////t./**/[|z|] = 11; |
There was a problem hiding this comment.
should there be support for "gotoDefinition"?
| @@ -1402,10 +1402,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.
Comment is not updated on line 1402
src/compiler/binder.ts
Outdated
| leftSideOfAssignment.parent = node; | ||
| target.parent = leftSideOfAssignment; | ||
|
|
||
| let funcSymbol = container.locals.get(target.text); |
There was a problem hiding this comment.
So this symbol is not necessary only for function declaration but class expression as well? if so, could you rename the variable?
src/compiler/binder.ts
Outdated
|
|
||
| // 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.
why you are using exports ? if there is more use to exports than "module exports" as the comment suggest (types.ts Line 2695) could you fix up the comment ?
There was a problem hiding this comment.
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.
that is where we keep the static properties on classes. members has the instance properties.
| @@ -1417,6 +1421,7 @@ namespace ts { | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
don't need this extra whitespace
src/compiler/binder.ts
Outdated
|
|
||
| // 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.
Is it because this is the static side of the class, not the instance side? Exports is a confusing name in this case, then.
src/compiler/binder.ts
Outdated
| @@ -2216,8 +2220,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); | |||
There was a problem hiding this comment.
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.
|
@sandersn any comments? |
This PR adds support for detecting property assignments to functions in .js files, for example:
The property assignment is tracked like prototype assignments.
The change also adds support for handling static and prototype properties on class expressions.
Also for anonymous function and class expression, that are defined in a variable declaration, e.g.
var x = function() {..}use the name of the variable (e.g.x) for error reporting purposes.