[feat] integrate refactorex

[gp-pereira]

After a lot of consideration, I've decided to initially bring Refactorex as a dependency to the project. That way, I was able to quickly integrate all the refactorings I've built last year at once, which will gain us some time to incorporate them one by one in the near future. The code action handler behaviour is pretty similar to the controller I had on Refactorex.

The downside of this approach is that GenLSP as also brought as a dependency to remote control. I've tried to avoid this by turning it into an optional dependency on Refactorex, but the lib couldn't compile after that. I hope that is not such a big problem though.

Two things I've noticed and would like to address are that:

1. Expert currently suggests and performs the refactorings in a single step, instead of the two step approach where first the available refactorings are shown to the user and then only the chosen one is performed. With a few changes, Refactorex was also able to work like that but I think that can be a performance hit, but nothing I personally felt while testing.
2. Expert does not differentiate automatic refactorings (suggested by just placing the cursor somewhere) and deliberate refactorings (user asked to refactor), which can also cause performance problems, especially considering the point above.

I've commented a few formatter options because they were bugging the refactorings (still don't fully understand why, but I believe converting an AST to string uses the project formatter and the injected remote_control doesn't have common, proto, etc as dependencies). I've also commented two tests that were broken in main, but I can fix them if they are important tests :)

From my testing, all refactorings from Refactorex are indeed working, so I think we can start planning our next steps.

[scohen]

Refactorex adds capability for refactorings, but these need to be reported on boot.
Currently, the supported refactorings are listed in apps/server/lib/lexical/server/state.ex in a module attribute. I believe this needs to be updated.

  @supported_code_actions [
    :quick_fix,
    :source_organize_imports
  ]

[scohen]

I hope that is not such a big problem though.

It kind of is. There was a pretty strong desire to keep LSP concepts out of the injected app. I'm not going to die on that hill, but having our own way of handling things internally is nice for contributors, though since they come in as a transitive dep, maybe it's not a big deal.

[scohen]

After a lot of consideration, I've decided to initially bring Refactorex as a dependency to the project.

This is great, I should have expressed that the whole design of remote_control is so that we can bring in any dependencies we want.

[gp-pereira]

Refactorex adds capability for refactorings, but these need to be reported on boot. Currently, the supported refactorings are listed in apps/server/lib/lexical/server/state.ex in a module attribute. I believe this needs to be updated.

  @supported_code_actions [
    :quick_fix,
    :source_organize_imports
  ]

They work without it, I think that is mostly for reference, but I'll add them, no problem!

I hope that is not such a big problem though.

It kind of is. There was a pretty strong desire to keep LSP concepts out of the injected app. I'm not going to die on that hill, but having our own way of handling things internally is nice for contributors, though since they come in as a transitive dep, maybe it's not a big deal.

Absolutely. The GenLSP usage on Refactorex is for it working in its own, since only the refactorings are being used, I also would like to remove this dependency is the near future. Can we move along and refactor this later?

Besides all that, how do you think we should move forward? keep the refactorings (edits in the ASTs) apart from the LS and use a combination of it and the features from the LS for some specific refactors or bring all of them in and have them only here? I think #2 is more powerful/easier for the future, but #1 could be cool for other projects to also use them.

[scohen]

Can we move along and refactor this later?

Of course.

Besides all that, how do you think we should move forward?

My bias is for power and ease of contributions, which I think is #2, and I suspect that involves deeper integration with things like the AST modules that are defined in Expert's core project. We might be able to publish that, as a library, as it's a completely separate mix project.

[scohen]

Note: I tried this branch and saw the same errors coming out of the formatter.
Does refactorex do anything at the project level?

[gp-pereira]

Note: I tried this branch and saw the same errors coming out of the formatter. Does refactorex do anything at the project level?

Not explicitly. I use Sourceror.to_string, which I think uses Elixir.Code that might use the project formatter.
But did you manage to get the refactorings working for you on emacs?

What do you think is missing for this branch to be merged?