Skip to content

Infer extracted local variable name from property name #37902

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

Merged
merged 1 commit into from
Mar 17, 2022

Conversation

dtinth
Copy link
Contributor

@dtinth dtinth commented Apr 11, 2020

Fixes #37898

This PR makes TypeScript infer newLocal name from property name when performing an "Extract to constant in enclosing scope" refactoring on a PropertyAccessExpression.

Kapture 2020-04-11 at 3 38 16

@@ -1009,7 +1009,9 @@ namespace ts.refactor.extractSymbol {

// Make a unique name for the extracted variable
const file = scope.getSourceFile();
const localNameText = getUniqueName(isClassLike(scope) ? "newProperty" : "newLocal", file);
const localNameText = isPropertyAccessExpression(node) && !isClassLike(scope) && !checker.resolveName(node.name.text, node, SymbolFlags.Value, /*excludeGlobals*/ false) && !isPrivateIdentifier(node.name) && !isKeyword(node.name.originalKeywordKind!)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that this line is long… 😂

I did not extract it into a function as my initial goal was to minimize the amount of structural change required.

…but to pave a way for more inference to take place, I did some refactoring and extract them to getLocalNameText function. If these diffs look good, I can cherry pick them into this PR.


…and with the above refactoring, I was able to make it also infer local variable name by the expression’s type (diff). If this sounds like a good idea, I can open an another issue and submit that as another PR. (I prefer to limit the scope of this PR to just PropertyAccess, but if you prefer I can put them into this PR as well.)

Screen recording
Example of inferring variable name based on expression’s type (not included in this PR)

@sandersn sandersn added the For Backlog Bug PRs that fix a backlog bug label May 5, 2020
@sandersn
Copy link
Member

sandersn commented May 5, 2020

@minestarks @jessetrinity same on this refactoring PR

@sandersn sandersn assigned gabritto and unassigned minestarks Mar 17, 2022
@sandersn sandersn requested review from gabritto and jakebailey and removed request for minestarks and jessetrinity March 17, 2022 18:25
@RyanCavanaugh
Copy link
Member

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

TypeScript should infer newLocal name from property name when performing "Extract to constant in enclosing scope" on a PropertyAccessExpression
5 participants