Conversation
|
Thanks for your contribution. This PR has failing CI tests and can not be merged in at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to merge this code in, please open a new PR that has been merged and rebased with the |
|
|
||
| const fileTextChanges: FileTextChanges[] = []; | ||
| const namePos: number = variableDeclaration.name.pos; | ||
| let variableInitializerText: string = variableDeclaration.initializer.getText(); |
There was a problem hiding this comment.
This is not going to work. the text from the initializer is not guaranteed to work in all places where the variable would be used. the initializer expression can contain names that have been shadowed later on and will need to be qualified accordingly . consider:
namespace N {
export var x = 0;
var temp = x;
function inner(x: string) {
console.log(temp);
}
}renaming temp to x is not correct, since x has a different meaning in this scope.
|
looking back at this PR, the infrastructure for refactoring has been added in #15569. the two additional refactorings added here do not seem salvageable either. I am not sure we even need to add a new JSDoc as a refactor, and inline temp needs to address in lining an expression correctly; this is not a simple change, and seems like a complete rewrite. closing this PR. |
3 participants
Adds a refactoring to add jsdoc to classes/modules.
Builds on top of #11913