feat: (code actions): add/from destructure

[Ilanaya]

fixes #167

Some tests to play with

[Ilanaya]

It Looks like it is working!

[zardoy]

I think its good for its purpose.
didn't really look at the code, I think its fine. However, to destructure should also avoid name collisions eg:

const a = []
const b = {}
b.a

from destruct will result in broken code, a should be named as a2 e.g. a: a2

[Ilanaya]

I think its good for its purpose. didn't really look at the code, I think its fine. However, to destructure should also avoid name collisions eg:

const a = []
const b = {}
b.a

from destruct will result in broken code, a should be named as a2 e.g. a: a2

Given this code:

function fn(newVariable) {
  const something = newVariable.bar
}

Source file gonna contain this list of identifiers:

So if try getting unique name

 const uniqueAccessorName = tsFull.getUniqueName('bar', sourceFile as unknown as FullSourceFile)

It will always generate bar_1 because it treats .bar in access expression newVariable.bar as unique identifier.

Hope, this was a clear explanation :) For now I will drop support unique names in this case, or maybe there is some way to handle it?

[zardoy]

Thanks for clarifying, we need to remove any getUniqueName usage in favor of typeChecker.resolveName(name, <block body node>, SymbolFlags.Value, /*excludeGlobals*/ true) (needs full type checker) for checking for name collisions instead (I remember it was also used in extract type, thus need to use type SymbolFlag). Would be possible to make this?

[Ilanaya]

Thanks for clarifying, we need to remove any getUniqueName usage in favor of typeChecker.resolveName(name, <block body node>, SymbolFlags.Value, /*excludeGlobals*/ true) (needs full type checker) for checking for name collisions instead (I remember it was also used in extract type, thus need to use type SymbolFlag). Would be possible to make this?

Yeah, I'll replace it everywhere in the codebase in a separate PR if you approve the way I implemented this logic here.

[zardoy]

This is getting great! So excited to see it in repo, @Ilanaya is it ready to merge now?

[Ilanaya]

Yep, I think it is. I'll recheck it again soon and merge if u don't mind

[zardoy]

@Ilanaya ok, I've got time to check how it works and I must admit there are some improvements needed:

1. need to handle case if property is reserved (e.g. class or default might be pretty common):

const a = {
    default: 1,
}

a.default
// results in broken code:
const { class } = { // class is resorved token!
    class: 1,
}

class

If you use require in js file you will have a quickfix to convert the file to es modules eg:

module.exports.default = 5

will be converted to

const _default = 5
export { _default as default }

it just uses isIdentifierANonContextualKeyword: https://github.com/a-tarasyuk/typescript/blob/8dc7ee8c2b3eadcd4d1130afb9c3a9df927ef6a7/src/services/codefixes/convertToEsModule.ts#L165

2. And also, have we ever discussed how we should handle index access? e.g.

const a = {
    a: 1,
}

a.a
a['b']
a['c-d']