smart-rename is missing some renames

[milahu]

Describe the bug

smart-rename is missing some renames

Input code

let input = [];
let output = [];
for (let { id: aa, data: bb } of input) {
  output.push({
    a: aa,
    b: bb.y,
  });
}

Reproduction

No response

Steps to reproduce

No response

Expected behavior

rename aa to id

 let input = [];
 let output = [];
-for (let { id: aa, data: bb } of input) {
+for (let { id, data } of input) {
   output.push({
-    a: aa,
-    b: bb.y,
+    a: id,
+    b: data.y,
   });
 }

Actual behavior

smart-rename preserves the rebinding from id to aa
but there is no reason, the identifier id is not taken
so renaming aa to id does not shadow an existing id identifier

 let input = [];
 let output = [];
-for (let { id: aa, data: bb } of input) {
+for (let { id: aa, data } of input) {
   output.push({
     a: aa,
-    b: bb.y,
+    b: data.y,
   });
 }

[pionxzh]

smart-rename will check if the new name is longer than the original one to ensure that the meaningful or, let's say, the longer name won't be eliminated accidentally. This rule might not be reasonable when both names are relatively short. I think we can adjust the rule to be:

1. replace when key length > value length. It means the key should be more expressive
2. replace when both key and value are short (e.g. <= 3). It means the value might be a temporary variable, and the key should be more expressive or readable in most cases
3. don't replace when key length <= value length. It means the key is not more expressive than the value, or the author wants to provide a more expressive name for the value

[pionxzh]

you can update id to idd, and you will see aa becomes idd 😄

[milahu]

i dont care about the length, i just want to remove the rebindings

rebindings should only be preserved if there is a collision with other identifiers
and these other identifiers cannot be renamed

[0xdevalias]

@milahu I believe the length is being used as a method to figure what is a minified variable or not.

If you have a better method of consistently determining that in an automated fashion; feel free to suggest it.

--

I haven't thought about this too deeply, but perhaps if a variable is being destructured, and the key it is being destructured from doesn't clash with any existing variables, then use that key as the variable name (regardless of length)

[pionxzh]

Rephrase the updated rule I mentioned: Only keep the destructured name when the variable's length is >= 4 to avoid removing non-minified variables. However, this may not be a good strategy for obfuscated input. Then, we can also consider simply ignoring the length.

[milahu]

this may not be a good strategy for obfuscated input

exactly

[pionxzh]

The length limitation has been removed.