Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add code actions for unknown modules and structs #891

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

Open
sheldak wants to merge 1 commit into elixir-lsp:master
base: master
Choose a base branch
Loading
from Qarma-inspect:unknown-module

Conversation

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

Copy link
Contributor Author

sheldak commented Jul 10, 2023

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

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

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?

Copy link
Collaborator

Sure, we should salvage the good stuff

Copy link
Collaborator

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

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.

lukaszsamson reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /