Refactoring Idea: Use variable for expressions

[hediet]

It would be awesome to have a refactoring that can replace expressions with a variable.

In this screenshot, I would like to replace all occurences of flip(sourceNmbr) with targetNmbr:

const targetNmbr = flip(sourceNmbr);

if (firstBefore && firstBefore.getInputRange(sourceNmbr).contains(topLineNumber)) {
    sourceRange = firstBefore.getInputRange(sourceNmbr);
    targetRange = firstBefore.getInputRange(flip(sourceNmbr));
} else if (firstBefore && firstBefore.getInputRange(sourceNmbr).isEmpty && firstBefore.getInputRange(sourceNmbr).startLineNumber === topLineNumber) {
    sourceRange = firstBefore.getInputRange(sourceNmbr).deltaEnd(1);
    targetRange = firstBefore.getInputRange(flip(sourceNmbr)).deltaEnd(1);
} else {
    const delta = firstBefore ? firstBefore.getInputRange(flip(sourceNmbr)).endLineNumberExclusive - firstBefore.getInputRange(sourceNmbr).endLineNumberExclusive : 0;
    sourceRange = new LineRange(topLineNumber, 1);
    targetRange = new LineRange(topLineNumber + delta, 1);
}

This would basically be the second step after extracting a constant.

[lgrammel]

Awesome, thanks for sharing the refactoring idea! This is somewhat similar to extract so it should be doable. How would you imagine the interaction, i.e. where would you want to trigger the refactoring?

[lgrammel]

Just realized that this is already possible with a refactoring combo:

1. Extract variable with flip(sourceNumber) into e.g. targetNumber2
2. Inline variable targetNumber
3. Rename targetNumber2 to targetNumber

[lgrammel]

v1.112.0 now has a "replace with existing variable" refactoring that can be invoked on an expression. If there is a variable with the same expression, the expression will be replaced with the variable.

@hediet you said this would be a "second step after extracting a constant", which indicates to me that extract constant did not work as expected. The P42 extract constant should already extract all occurrences. Was there a specific case where the refactoring did not find all occurrences as expected?

[hediet]

The P42 extract constant should already extract all occurrences.

Oh, I was not aware of this. I used the typescript one, as it was suggested first I think. I really recommend hiding the obsolete typescript refactorings by default (and adding a setting to show them again)!

[lgrammel]

I really recommend hiding the obsolete typescript refactorings by default (and adding a setting to show them again)!

I agree, you convinced me this is the best way to proceed.

The typescript extract const refactoring is not fully replaced by the JavaScript Assistant yet, I need to add an option to extract a single occurrence and to determine the target scope.