Make getNameOfDeclaration public

[sandersn]

Move getNameOfDeclaration to the non-internal namespace in utilities.

[ghost]

Mark return type as DeclarationName | undefined.

[sandersn]

done

[ghost]

How does getNameOfDeclaration differ from getDeclarationName? When should you prefer one over the other?

[ghost]

lhs as Identifier

[sandersn]

The answer to both your comments is that this code is closely couple to the result of getSpecialPropertyAssignmentKind, which returns ExportsProperty in two cases.

In this case, the code is wrong and should be checking lhs.expression.kind. I will fix it.

[ghost]

How do you know lhs.expression is itself a PropertyAccessExpression?

[sandersn]

If the special property assignment kind is ExportsProperty, lhs is known to be a PropertyAccessExpression and lhs.expression is either an Identifier or PropertyAccessExpression.

[sandersn]

Furthermore, in the case that lhs.expression is Identifier, lhs.expression is "exports" and the name is lhs.name (from exports.blah = ...). If it's a property access expression, then lhs.expression.expression is "module", lhs.expression.name is "exports", and lhs.name is the name we're interested in (from module.exports.blah = ...).

So actually this code is wrong too. Changing it doesn't break any tests either, which is very suspicious. I don't think our checkJs coverage is very good.

[sandersn]

getDeclarationName is only used in transforms, and in fact delegates to getNameOfDeclaration.

Other differences:
getDeclarationName always generates a name; a new one if none exists. If one already exists, it clones it and sets some EmitFlags.

getDeclarationName should really only be accessible to transforms, but it's actually available to anything in the entire ts namespace. I'm not what is the best way to indicate that getNameOfDeclaration is the simpler function to be used most of the time.

[ghost]

There are actually 2 getDeclarationName functions, I was looking at the one in binder.ts.
😱

[sandersn]

That one also delegates to getNameOfDeclaration and also always returns a name, but instead of generating a name for unnamed things, asserts that computed property names are constant (or well-known) and returns well known names for everything else (eg "exports=").

[ghost]

Seems like the one in factory.ts is the wrongly named one since it always creates a new node rather than merely getting one. But that one's public too so it would be a breaking change to rename.

[sandersn]

OK, your comments resulted in a considerably cleaned up and fixed getNameOfDeclaration. Unfortunately, they also expose the fact that we have almost no code coverage for special assignment kinds.

[sandersn]

I think this is ready to go in. getNameOfDeclaration actually handles more cases than it ever will see, because the binder has special code paths for prototype property assignments and exports property assignments that end up delegating to normal property assignments. But since it's publicly accessible now, I think that it should correctly retrieve the name for all kinds of special property assignments.

I did get rid of some now-dead special-case code in the binder.