-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
feat: add hover language feature for modules and module function calls #104
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,207 @@ | ||
defmodule NextLS.Hover do | ||
alias GenLSP.Structures.{ | ||
Hover, | ||
MarkupContent, | ||
Position, | ||
Range | ||
} | ||
|
||
alias NextLS.ReferenceTable | ||
alias NextLS.Runtime | ||
|
||
@spec fetch(lsp :: GenLSP.LSP.t(), uri :: String.t(), position :: Position.t()) :: Hover.t() | nil | ||
def fetch(lsp, uri, position) do | ||
position = {position.line + 1, position.character + 1} | ||
document = Enum.join(lsp.assigns.documents[uri], "\n") | ||
|
||
with {module, function, range} <- find_reference(lsp, document, uri, position), | ||
docs when is_binary(docs) <- fetch_docs(lsp, document, module, function) do | ||
%Hover{ | ||
contents: %MarkupContent{ | ||
kind: "markdown", | ||
value: docs | ||
}, | ||
range: range | ||
} | ||
end | ||
end | ||
|
||
defp find_reference(lsp, document, uri, position) do | ||
surround_context = Code.Fragment.surround_context(document, position) | ||
|
||
if surround_context == :none do | ||
nil | ||
else | ||
case ReferenceTable.reference(lsp.assigns.reference_table, URI.parse(uri).path, position) do | ||
[%{type: :function, module: module, identifier: function} | _] -> | ||
{module, function, build_range(surround_context)} | ||
|
||
[%{type: :alias, module: module} | _] -> | ||
{module, nil, build_range(surround_context)} | ||
|
||
_other -> | ||
find_in_context(surround_context) | ||
end | ||
end | ||
end | ||
|
||
defp find_in_context(%{context: {:alias, module}} = context) do | ||
{to_module(module), nil, build_range(context)} | ||
end | ||
|
||
defp find_in_context(%{context: {:unquoted_atom, erlang_module}} = context) do | ||
{to_atom(erlang_module), nil, build_range(context)} | ||
end | ||
|
||
defp find_in_context(_context) do | ||
nil | ||
end | ||
|
||
defp fetch_docs(lsp, document, module, nil) do | ||
with {:ok, {_, _, _, _, docs, _, _} = docs_v1} <- request_docs(lsp, document, module) do | ||
print_doc(module, nil, nil, docs, docs_v1) | ||
end | ||
end | ||
|
||
defp fetch_docs(lsp, document, module, function) do | ||
with {:ok, {_, _, _, _, _, _, functions_docs} = docs_v1} <- request_docs(lsp, document, module), | ||
{_, _, [spec], function_docs, _} <- find_function_docs(functions_docs, function) do | ||
print_doc(module, function, spec, function_docs, docs_v1) | ||
end | ||
end | ||
|
||
defp request_docs(lsp, document, module, attempt \\ 1) do | ||
case send_fetch_docs_request(lsp, module) do | ||
{:error, :not_ready} -> | ||
nil | ||
|
||
{:ok, {:error, :module_not_found}} -> | ||
if attempt < 2 do | ||
aliased_module = find_aliased_module(document, module) | ||
|
||
if aliased_module do | ||
request_docs(lsp, document, from_quoted_module_to_module(aliased_module), attempt + 1) | ||
end | ||
end | ||
|
||
{:ok, {:error, :chunk_not_found}} -> | ||
nil | ||
|
||
other -> | ||
other | ||
end | ||
end | ||
|
||
defp find_aliased_module(document, module) do | ||
module = to_quoted_module(module) | ||
|
||
ast = | ||
Code.string_to_quoted(document, | ||
unescape: false, | ||
token_metadata: true, | ||
columns: true | ||
) | ||
|
||
{_ast, aliased_module} = | ||
Macro.prewalk(ast, nil, fn | ||
# alias A, as: B | ||
{:alias, _, [{:__aliases__, _, aliased_module}, [as: {:__aliases__, _, ^module}]]} = expr, _ -> | ||
{expr, aliased_module} | ||
|
||
# alias A.{B, C} | ||
{:alias, _, [{{:., _, [{:__aliases__, _, namespace}, :{}]}, _, aliases}]} = expr, acc -> | ||
aliases = Enum.map(aliases, fn {:__aliases__, _, md} -> md end) | ||
|
||
if module in aliases do | ||
{expr, namespace ++ module} | ||
else | ||
{expr, acc} | ||
end | ||
|
||
# alias A.B.C | ||
{:alias, _, [{:__aliases__, _, aliased_module}]} = expr, acc -> | ||
offset = length(aliased_module) - length(module) | ||
|
||
if Enum.slice(aliased_module, offset..-1) == module do | ||
{expr, aliased_module} | ||
else | ||
{expr, acc} | ||
end | ||
|
||
expr, acc -> | ||
{expr, acc} | ||
end) | ||
|
||
aliased_module | ||
end | ||
|
||
defp send_fetch_docs_request(lsp, module) do | ||
Runtime.call(lsp.assigns.runtime, {Code, :fetch_docs, [module]}) | ||
end | ||
|
||
defp build_range(%{begin: {line, start}, end: {_, finish}}) do | ||
%Range{ | ||
start: %Position{line: line - 1, character: start - 1}, | ||
end: %Position{line: line - 1, character: finish - 1} | ||
} | ||
end | ||
|
||
defp find_function_docs(docs, function) do | ||
Enum.find(docs, fn | ||
{{type, ^function, _}, _, _, _, _} when type in [:function, :macro] -> true | ||
_ -> false | ||
end) | ||
end | ||
|
||
defp print_doc(_module, _function, _spec, :none, _docs_v1) do | ||
nil | ||
end | ||
|
||
defp print_doc(_module, _function, _spec, :hidden, _docs_v1) do | ||
nil | ||
end | ||
|
||
defp print_doc(_module, nil, _spec, doc, {_, _, _, "text/markdown", _, _, _}) do | ||
doc["en"] | ||
end | ||
|
||
defp print_doc(_module, _function, spec, doc, {_, _, _, "text/markdown", _, _, _}) do | ||
print_function_spec(spec) <> doc["en"] | ||
end | ||
|
||
defp print_doc(module, nil, _spec, _doc, {_, _, :erlang, "application/erlang+html", _, _, _} = docs_v1) do | ||
:shell_docs.render(module, docs_v1, %{ansi: false}) |> Enum.join() | ||
end | ||
|
||
defp print_doc(module, function, spec, _doc, {_, _, :erlang, "application/erlang+html", _, _, _} = docs_v1) do | ||
print_function_spec(spec) <> (:shell_docs.render(module, function, docs_v1, %{ansi: false}) |> Enum.join()) | ||
end | ||
|
||
defp print_function_spec(spec) do | ||
"### " <> spec <> "\n\n" | ||
end | ||
|
||
defp from_quoted_module_to_module(quoted_module) do | ||
quoted_module | ||
|> Enum.map(&Atom.to_string/1) | ||
|> Enum.join(".") | ||
|> to_charlist() | ||
|> to_module() | ||
end | ||
|
||
defp to_module(charlist) when is_list(charlist) do | ||
String.to_atom("Elixir." <> to_string(charlist)) | ||
end | ||
|
||
defp to_atom(charlist) when is_list(charlist) do | ||
charlist |> to_string() |> String.to_atom() | ||
end | ||
|
||
defp to_quoted_module(module) when is_atom(module) do | ||
module | ||
|> Atom.to_string() | ||
|> String.replace("Elixir.", "") | ||
|> String.split(".") | ||
|> Enum.map(&String.to_atom/1) | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is potentially better to extend tracer and collect all aliases instead of analyzing it when hovering. Let me know if this is a blocker and needs to be rearranged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intuition for how to implement this feature was going to be using the reference table to find the right thing and then look up the docs (or, just stick the docs in the symbol table), but i did have concerns about being able to lookup docs for code you've written but hasn't compiled yet, which using your implementation would solve for.
I'll probably get time tonight to review this, but even if we want to use the ref and symbol tables, we can probably merge this and change it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds a better idea. My approach doesn't support imported functions which you can find quite often in files using
Phoenix.LiveView
andPhoenix.Controller
. But the reference table has this info OOTB.As for the code not yet compiled I don't see it a big problem as long as user needs to just save the file. If you don't agree there may be two steps:
Code.Fragment.surround_context/2
I'm moving it back to draft but waiting for your feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think moving to the reference table is a good idea, so you can move forward with that if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not quite satisfied with results: only 6 out of 15 assertions that I added for the hovering test were passed. I may need to extend code in Tracer a bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could make it feel at least "ok" using two-step approach. Data from reference may not produce good results. For example, I was lucky using
Atom.to_string/1
in my example code as there is no reference for such function (there is a reference but for the:erlang.atom_to_binary
function).I also found that there is no event when Erlang module is being referenced. There may be a workaround for it when Erlang function is found though.
I also tried to register references for aliases but it doesn't work well for multi aliases. For example this
Will produce two events with the same line and column information (both start and end). In such case it is problematic to decide what reference to use without additional code around it.
I have only one good news that now imported functions are supported when hovering.
One more thing to note: Erlang docs are not in the build unless the
KERL_BUILD_DOCS=yes
is set before compiling the source.