elixir-lsp / elixir-ls

A frontend-independent IDE "smartness" server for Elixir. Implements the "Language Server Protocol" standard and provides debugger support via the "Debug Adapter Protocol"
https://elixir-lsp.github.io/elixir-ls/
Apache License 2.0
1.46k stars 192 forks source link

Quickfix for importing modules #161

Open imerkle opened 4 years ago

imerkle commented 4 years ago

It would be great to have quickfix suggestions for importing modules.

axelson commented 4 years ago

Can you give some example scenarios when a quickfix suggestion for importing a module would be shown? Although, it might make more sense to provide a quickfix suggestion for aliasing a module.

lukaszsamson commented 4 years ago

Quickfix import, alas and require would definitely be useful. Unfortunately I don't think it's doable without compilation traces (which means elixir >= 1.10).

hworld commented 4 years ago

Was just gonna make an issue for this one. I'd also love to type out the last part of a module, ie the part after the last dot, and have it suggest all modules that might match that. Completion would add the alias.

PHP has a similar system to Elixir with the new-ish namespaces feature. It gets really tedious typing out the full name, so you usually "use" it at the top like an alias. Here's an example of what it looks like in VSCode. Screenshot from 2020-03-26 22-10-31

If we had the data available (seems like that compiler tracing PR would need to get merged first?), I wouldn't mind taking a stab at implementing. I think it would save a ton of time for me while writing Elixir code.

axelson commented 4 years ago

@lukaszsamson couldn't we support a quickfix for alias using a similar module list as the one that we used for the workspace symbols provider? Although compiler tracing might make it easier, but I think it will be a while before we want to require that users run their code with Elixir 1.10. Also I think adding a require quickfix would involve detecting the you must require Logger before invoking the macro type warning and generating a require Logger statement near the top of the file. I think that actually might be the easiest quickfix to start with, although the alias example that @hworld gives would be the most useful. I'm not personally sold on a quickfix for import because there's a huge list of functions that you could import and I also think that it's easy to overly rely on import which can then cause recompilation times to increase so I don't think it's something that the tooling should be encouraging (although I'm open to changing my mind on this).

axelson commented 4 years ago

Also @hworld a PR would be very welcome that provided similar functionality to the php "use" that you show, although that be implemented by providing a TextEdit to additionalTextEdits of a CompletionItem (Completion Request docs), and a PR (and associated issue) would be very welcome :+1:

lukaszsamson commented 3 years ago

@lukaszsamson couldn't we support a quickfix for alias using a similar module list as the one that we used for the workspace symbols provider?

@axelson Yep, it might be doable. We'd need to parse compiler warnings/errors, extract the module thet misses requre/alias, do the search and find place to insert code.

I wouldn't mind taking a stab at implementing. I think it would save a ton of time for me while writing Elixir code.

@hworld If you would like to take it up then open a PR to SuggestionsProvider in https://github.com/elixir-lsp/elixir_sense

ajayvigneshk commented 2 years ago

I've opened a draft elixir-sense PR regarding this.

I'm also working on an PR for elixir-ls. One thing that I would like to get some suggestions on is, how to find the line where the alias should be added. I'm guessing this has to be preferrably at a module scope. The closest idea that I've is using the {:alias,...} compiler trace event to get the first line at a module scope and insert before that(as multi-aliases that spawn multiple lines are listed by the event as belonging to the first line). But what about modules that don't have any alias es at all? Would inserting at a line before the current cursor be okay for it?

dylan-chong commented 2 years ago

You could place it before any module attributes, but after any @moduledoc, use or import calls? That's how I've always styled it personally. (Although if you put the alias before use/import, you can then alias the module getting imported/used, if that's something you want)

ajayvigneshk commented 2 years ago

Yeah, what am not sure is how to programmatically get the line that's safe to insert for the alias. Because the module attributes could be multi-line, I don't think it would be safe to insert say on the next line of a module attribute always

dylan-chong commented 2 years ago

What about inserting before what ever comes after a module attribute

On 9/05/2022, at 4:31 PM, Ajay @.***> wrote:

 Yeah, what am not sure is how to programmatically get the line that's safe to insert for the alias. Because the module attributes could be multi-line, I don't think it would be safe to insert say on the next line of a module attribute always

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

ajayvigneshk commented 2 years ago

That would be nice. I just don't know how to figure out the line that comes after a module attribute (considering multiline strings and such).

dylan-chong commented 2 years ago

Maybe you can use quote do inside iex to help you look at the AST

iex(4)> quote do
...(4)>     defmodule Example do
...(4)>         alias Bla
...(4)>
...(4)>         @moduledoc """
...(4)>         woooo
...(4)>         """
...(4)>
...(4)>         def fun, do: 1
...(4)>     end
...(4)> end
{:defmodule, [context: Elixir, import: Kernel],
 [
   {:__aliases__, [alias: false], [:Example]},
   [
     do: {:__block__, [],
      [
        {:alias, [context: Elixir], [{:__aliases__, [alias: false], [:Bla]}]},
        {:@, [context: Elixir, import: Kernel],
         [{:moduledoc, [context: Elixir], ["woooo\n"]}]},
        {:def, [context: Elixir, import: Kernel],
         [{:fun, [if_undefined: :apply, context: Elixir], Elixir}, [do: 1]]}         
      ]}
   ]
 ]}
iex(5)>

On 10/05/2022, at 2:53 AM, Ajay @.***> wrote:

That would be nice. I just don't know how to figure out the line that comes after a module attribute (considering multiline strings and such).

— Reply to this email directly, view it on GitHub https://github.com/elixir-lsp/elixir-ls/issues/161#issuecomment-1121208051, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFME3MKHTRHOKJYX34MXBTVJERFLANCNFSM4LTZLKQQ. You are receiving this because you are subscribed to this thread.

dylan-chong commented 2 years ago

There’d normally be a line: integer keyword arg in the 2nd element of every 3-element tuple.

Inside a module is a block type, which is just a list of statements. Eg @moduledoc would be a statement, so just find the statement after that.

Anyway, i have no idea what is exposed to Elixir LS or how it works