Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code actions for unknown modules and structs #891

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sheldak
Copy link
Contributor

@sheldak sheldak commented May 22, 2023

I propose another two code actions that can be applied when we get either:

  • a compile error because the struct is not defined
  • a warning because we are calling a function from a module that is not known

Replacing modules

The first code action proposes replacing an unknown module with its full name. For example:

defmodule MyModule do
  def foo do
    %ExampleStruct{}
  end
end

will be replaced with

defmodule MyModule do
  def foo do
    %ElixirLS.LanguageServer.Fixtures.ExampleStruct{}
  end
end

if we have module ElixirLS.LanguageServer.Fixtures.ExampleStruct available. Similarly

defmodule MyModule do
  def foo do
    ExampleDefaultArgs.my_func("text")
  end
end

can be replaced with

defmodule MyModule do
  def foo do
    ElixirLS.LanguageServer.Fixtures.ExampleDefaultArgs.my_func("text")
  end
end

if ElixirLS.LanguageServer.Fixtures.ExampleDefaultArgs is available.

Adding an alias

The second code action adds an alias that is required to make the code work. For example:

defmodule MyModule do
  def foo do
    %ExampleStruct{}
  end
end

will be replaced with

defmodule MyModule do
  alias ElixirLS.LanguageServer.Fixtures.ExampleStruct

  def foo do
    %ExampleStruct{}
  end
end

and

defmodule MyModule do
  def foo do
    ExampleDefaultArgs.my_func("text")
  end
end

will be replaced with

defmodule MyModule do
  alias ElixirLS.LanguageServer.Fixtures.ExampleDefaultArgs

  def foo do
    ExampleDefaultArgs.my_func("text")
  end
end

Notes

  • We always get 2 code actions (replacement and alias) for every suggested module
  • There are at most 3 suggested modules (so at most 6 code actions)
  • Suggested modules that are from the same namespace (first atom from full module name) as the module in which we are doing code action have priority
  • To find a place for the alias I've used ElixirSense.Core.Metadata.get_position_to_insert_alias/2 function which has two limitations that may affect the usability of the code action:
    • if there are already aliases in the module, the new alias will be added as the first one so the aliases may stop being ordered alphabetically
    • if there are no aliases in the module, the new alias will be added as a first directive just below @moduledoc which may not be a perfect place

@sheldak
Copy link
Contributor Author

sheldak commented Jul 10, 2023

@lukaszsamson Could you give me some feedback on that feature?

@lukaszsamson
Copy link
Collaborator

Sorry this is taking time but work on experimental server has stalled. I'm thinking of removing it. What do you think about migrating this PR back to original server code? There was already a PR adding some code action (now reverted due to many problems in implementation) #718

@sheldak
Copy link
Contributor Author

sheldak commented Aug 11, 2023

@lukaszsamson I see. I can definitely move this PR to the original code. However, I think many additional helpers introduced in the experimental server were very useful for working on AST. What about starting with migrating the existing code actions with all that helpers to the original server before introducing this new code action?

@lukaszsamson
Copy link
Collaborator

Sure, we should salvage the good stuff

@lukaszsamson
Copy link
Collaborator

@sheldak #1057 is merged now, would you consider to rework this PR on top of the current master?

@sheldak
Copy link
Contributor Author

sheldak commented Mar 12, 2024

@sheldak #1057 is merged now, would you consider to rework this PR on top of the current master?

Sure, I will definitely go back to that at some point. However, currently, I have other priorities so it may take some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants