Add "Assign statement to new local variable" (CTRL+2L) quick fix

I've started working on this.

Have a look at org.eclipse.jdt.core.NamingConventions.suggestVariableNames(int, int, String, IJavaProject, int, String[], boolean).

Not all of the fields will be relevant, and as we mentioned, there is no need to infer the type of the variable.

One thing that we didn't mention is that it is not a good idea to propose any names that are already in scope. However, as a first pass at this, let's not worry about it.

Lastly, we need to figure out how to hook this into the existing keybindings, but first get the basic working and then we will work on keybindings.

Something I just thought about was multiple assignment. Not sure if this is easy to support or should be supported, but it is something to think about.

For example:

[1,2]//CTRL + 2L

by way of the refactoring could produce:

def (x, y) = [1,2]

Similarly:

List f() {
  [1, 2]
}
...
f()//CTRL + 2L

by way of the refactoring could produce:

List f() {
  [1, 2]
}
...
def (x, y) = f()

Also, should the refactoring ask the user if the local variable should be typed? Or should it always use the inferred type?

Here's the quick assist. A few points:

This assist is not intelligent about lists - it just assigns one name to the list and selects this new name so the user can change it if desired. If this is a highly desired feature, perhaps this choice can be re-evaluated.

This fix makes a reasonable guess of a name based on the statement.

Additionally, this assist is not available for statements that are return statements - either explicitly or implicitly.

In terms of the key-binding, I sunk a few hours into it but I had no luck with it. Since the end of term is approaching, I wanted to share what I have.

Finally, this fix is dependent on a helper function contained in the patch attached to GRE1346.

Stephanie

I finally got a chance to look at your patch (apologies, still on vacation, so I must do it during nap time). This looks quite good, but there are some comments:

1. The test suite looks good and it passes for me.
2. The class comments on the AssignStatementToNewLocalProposal and AssignStatementToNewLocalRefactoring have not been updated.
3. For list and map literals, I'd rather see the name of the extracted variable be list and map respectively. Currently, it is temp.
4. The missing functionality is fine, but you should raise new issues for the following:
   1. The key-binding issue
   2. Lack of extraction at return statements
   3. Type inferencing of the extracted variable type
   4. Linked editing mode for the extracted variable name and its type (see the same quick assist in Java).

I am committing our current patch. So, can you make sure that any changes you make are based on top of head?

Thanks, Andrew!

Here's another patch. It addresses the following issues:

• comments in AssignStatementToNewLocalProposal and AssignStatementToNewLocalRefactoring are fixed
• guessed names for lists and maps are more intelligent (list and map, as requested)

About the return statement thing: doing so changes the meaning of the code in a fairly insidious way - because of Groovy's implicit return statement, a method will suddenly return void that perhaps had a more meaningful return value. Are we sure this is something we want to do? The Java version ignores return statements. I could go either way on this.

As for type-inferencing, linked editing mode and the key-binding, I will address those shortly. Seperate issues for each?

Thanks,
Stephanie

Resolving this issue. The final patch looks good.

Please remember to raise the issues for the uncompleted work.