From 1357c2029d2068aab1f0179c73ecba713f4cc1f1 Mon Sep 17 00:00:00 2001 From: Moosieus Date: Tue, 30 Jul 2024 12:08:16 -0400 Subject: [PATCH 1/5] provide next best definition when no exact match is present. --- apps/common/lib/lexical/ast/analysis.ex | 14 +++++ apps/common/lib/lexical/ast/analysis/scope.ex | 4 ++ .../code_intelligence/definition.ex | 61 ++++++++++++++----- .../code_intelligence/definition_test.exs | 22 ++++++- .../lexical_shared/lib/lexical/formats.ex | 29 +++++++++ 5 files changed, 115 insertions(+), 15 deletions(-) diff --git a/apps/common/lib/lexical/ast/analysis.ex b/apps/common/lib/lexical/ast/analysis.ex index 9d87a043f..b3a289550 100644 --- a/apps/common/lib/lexical/ast/analysis.ex +++ b/apps/common/lib/lexical/ast/analysis.ex @@ -116,6 +116,20 @@ defmodule Lexical.Ast.Analysis do end) end + def module_scope(%__MODULE__{} = analysis, %Position{} = pos) do + enclosing_scopes = scopes_at(analysis, pos) + + first_scope = List.first(enclosing_scopes) + + Enum.reduce_while(enclosing_scopes, first_scope, fn + %Scope{module: same} = current, %Scope{module: same} -> + {:cont, current} + + _, current -> + {:halt, current} + end) + end + defp enclosing_scopes(scopes, range) do Enum.filter(scopes, fn scope -> Range.contains?(scope.range, range.start) diff --git a/apps/common/lib/lexical/ast/analysis/scope.ex b/apps/common/lib/lexical/ast/analysis/scope.ex index 26f9ebd28..1b85797cc 100644 --- a/apps/common/lib/lexical/ast/analysis/scope.ex +++ b/apps/common/lib/lexical/ast/analysis/scope.ex @@ -83,4 +83,8 @@ defmodule Lexical.Ast.Analysis.Scope do def end_line(%__MODULE__{} = scope, :end), do: scope.range.end.line def end_line(_, %Position{} = position), do: position.line def end_line(_, line) when is_integer(line), do: line + + def module_alias(%__MODULE__{module: module}) do + Module.concat(module) + end end diff --git a/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex b/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex index b854f00b8..57f50f5c7 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex @@ -3,6 +3,7 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do alias Future.Code alias Lexical.Ast alias Lexical.Ast.Analysis + alias Lexical.Ast.Analysis.Scope alias Lexical.Document alias Lexical.Document.Location alias Lexical.Document.Position @@ -49,20 +50,33 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do %Position{} = position ) do mfa = Formats.mfa(module, function, arity) + definitions = query_search_index_exact(mfa, subtype: :definition) definitions = - mfa - |> query_search_index(subtype: :definition) - |> Stream.flat_map(fn entry -> - case entry do - %Entry{type: {:function, :delegate}} -> - mfa = get_in(entry, [:metadata, :original_mfa]) - query_search_index(mfa, subtype: :definition) ++ [entry] - - _ -> - [entry] - end - end) + case definitions do + [_ | _] -> + definitions + + _ -> + # feat: search for next best definition when no exact match is present. + module_at_position = + analysis + |> Analysis.module_scope(position) + |> Scope.module_alias() + + call_prefix = Formats.mf(module, function) + definitions = query_search_index_prefix(call_prefix, subtype: :definition) + + if module == module_at_position do + definitions + else + Stream.reject(definitions, &(&1.type == {:function, :private})) + end + end + + definitions = + definitions + |> Stream.flat_map(&resolve_defdelegate/1) |> Stream.uniq_by(& &1.subject) locations = @@ -80,6 +94,15 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do elixir_sense_definition(analysis, position) end + def resolve_defdelegate(%Entry{type: {:function, :delegate}} = entry) do + mfa = get_in(entry, [:metadata, :original_mfa]) + query_search_index_exact(mfa, subtype: :definition) ++ [entry] + end + + def resolve_defdelegate(entry) do + [entry] + end + defp maybe_fallback_to_elixir_sense(resolved, locations, analysis, position) do case locations do [] -> @@ -171,8 +194,18 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do end end - defp query_search_index(subject, condition) do - case Store.exact(subject, condition) do + defp query_search_index_exact(subject, constraints) do + case Store.exact(subject, constraints) do + {:ok, entries} -> + entries + + _ -> + [] + end + end + + defp query_search_index_prefix(subject, constraints) do + case Store.prefix(subject, constraints) do {:ok, entries} -> entries diff --git a/apps/remote_control/test/lexical/remote_control/code_intelligence/definition_test.exs b/apps/remote_control/test/lexical/remote_control/code_intelligence/definition_test.exs index fc286bf75..de3ac3c3b 100644 --- a/apps/remote_control/test/lexical/remote_control/code_intelligence/definition_test.exs +++ b/apps/remote_control/test/lexical/remote_control/code_intelligence/definition_test.exs @@ -402,7 +402,6 @@ defmodule Lexical.RemoteControl.CodeIntelligence.DefinitionTest do assert definition_line =~ ~S[«binary_to_atom»(Binary)] end end - describe "definition/2 when making local call to a delegated function" do setup [:with_referenced_file] @@ -433,6 +432,27 @@ defmodule Lexical.RemoteControl.CodeIntelligence.DefinitionTest do end end + describe "definition/2 when no exact is available" do + setup [:with_referenced_file] + + test "", %{project: project, uri: referenced_uri} do + subject_module = ~q[ + defmodule UsesRemoteFunction do + use MyDefinition + + def uses_greet() do + gree|t("World", "Bad Arity") + end + end + ] + + assert {:ok, ^referenced_uri, definition_line} = + definition(project, subject_module, referenced_uri) + + assert definition_line == ~S[ def «greet»(name) do] + end + end + describe "edge cases" do setup [:with_referenced_file] diff --git a/projects/lexical_shared/lib/lexical/formats.ex b/projects/lexical_shared/lib/lexical/formats.ex index f4fc6c9ba..d77613d16 100644 --- a/projects/lexical_shared/lib/lexical/formats.ex +++ b/projects/lexical_shared/lib/lexical/formats.ex @@ -90,6 +90,7 @@ defmodule Lexical.Formats do millis end + @spec plural(integer(), String.t(), String.t()) :: String.t() def plural(count, singular, plural) do case count do 0 -> templatize(count, plural) @@ -98,10 +99,38 @@ defmodule Lexical.Formats do end end + @doc """ + Formats a module, function, and arity into a string. + + ## Examples + + iex> alias LXical.Formats + LXical.Formats + iex> mfa(Formats, :mfa, 3) + "LXical.Formats.mfa/3" + iex> mfa("Formats", "mfa", 3) + "LXical.Formats.mfa/3" + + """ + @spec mfa(atom() | binary(), any(), any()) :: String.t() def mfa(module, function, arity) do "#{module(module)}.#{function}/#{arity}" end + @doc """ + Formats a module and function without arity. + + ## Examples + + iex> mf(LXical.Formats, mf) + "LXical.Formats.mf/" + + """ + @spec mf(atom() | binary(), any()) :: String.t() + def mf(module, function) do + "#{module(module)}.#{function}/" + end + defp templatize(count, template) do count_string = Integer.to_string(count) String.replace(template, "${count}", count_string) From 921384d246403428d4e3f0db974c185882061af8 Mon Sep 17 00:00:00 2001 From: Moosieus Date: Wed, 31 Jul 2024 01:36:22 -0400 Subject: [PATCH 2/5] Use `Analyzer.current_module/2` and fix test --- apps/common/lib/lexical/ast/analysis/scope.ex | 4 ---- .../remote_control/code_intelligence/definition.ex | 7 ++----- .../code_intelligence/definition_test.exs | 11 ++++++----- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/apps/common/lib/lexical/ast/analysis/scope.ex b/apps/common/lib/lexical/ast/analysis/scope.ex index 1b85797cc..26f9ebd28 100644 --- a/apps/common/lib/lexical/ast/analysis/scope.ex +++ b/apps/common/lib/lexical/ast/analysis/scope.ex @@ -83,8 +83,4 @@ defmodule Lexical.Ast.Analysis.Scope do def end_line(%__MODULE__{} = scope, :end), do: scope.range.end.line def end_line(_, %Position{} = position), do: position.line def end_line(_, line) when is_integer(line), do: line - - def module_alias(%__MODULE__{module: module}) do - Module.concat(module) - end end diff --git a/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex b/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex index 57f50f5c7..5f1040c6b 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex @@ -3,11 +3,11 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do alias Future.Code alias Lexical.Ast alias Lexical.Ast.Analysis - alias Lexical.Ast.Analysis.Scope alias Lexical.Document alias Lexical.Document.Location alias Lexical.Document.Position alias Lexical.Formats + alias Lexical.RemoteControl.Analyzer alias Lexical.RemoteControl.CodeIntelligence.Entity alias Lexical.RemoteControl.Search.Indexer.Entry alias Lexical.RemoteControl.Search.Store @@ -59,10 +59,7 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do _ -> # feat: search for next best definition when no exact match is present. - module_at_position = - analysis - |> Analysis.module_scope(position) - |> Scope.module_alias() + {:ok, module_at_position} = Analyzer.current_module(analysis, position) call_prefix = Formats.mf(module, function) definitions = query_search_index_prefix(call_prefix, subtype: :definition) diff --git a/apps/remote_control/test/lexical/remote_control/code_intelligence/definition_test.exs b/apps/remote_control/test/lexical/remote_control/code_intelligence/definition_test.exs index de3ac3c3b..75e6c1f44 100644 --- a/apps/remote_control/test/lexical/remote_control/code_intelligence/definition_test.exs +++ b/apps/remote_control/test/lexical/remote_control/code_intelligence/definition_test.exs @@ -402,6 +402,7 @@ defmodule Lexical.RemoteControl.CodeIntelligence.DefinitionTest do assert definition_line =~ ~S[«binary_to_atom»(Binary)] end end + describe "definition/2 when making local call to a delegated function" do setup [:with_referenced_file] @@ -435,21 +436,21 @@ defmodule Lexical.RemoteControl.CodeIntelligence.DefinitionTest do describe "definition/2 when no exact is available" do setup [:with_referenced_file] - test "", %{project: project, uri: referenced_uri} do + test "find the definition of a remote function call", %{project: project, uri: referenced_uri} do subject_module = ~q[ defmodule UsesRemoteFunction do - use MyDefinition + alias MyDefinition def uses_greet() do - gree|t("World", "Bad Arity") + MyDefinition.gree|t("World", "Bad", "Arity") end end ] assert {:ok, ^referenced_uri, definition_line} = - definition(project, subject_module, referenced_uri) + definition(project, subject_module, referenced_uri) - assert definition_line == ~S[ def «greet»(name) do] + assert definition_line == ~S[ def «greet(name)» do] end end From 94ebbf28b7db3d698610adee55123a623a7eca7f Mon Sep 17 00:00:00 2001 From: Moosieus Date: Sun, 4 Aug 2024 00:20:49 -0400 Subject: [PATCH 3/5] Remove extraneous module_scope function --- apps/common/lib/lexical/ast/analysis.ex | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/apps/common/lib/lexical/ast/analysis.ex b/apps/common/lib/lexical/ast/analysis.ex index b3a289550..9d87a043f 100644 --- a/apps/common/lib/lexical/ast/analysis.ex +++ b/apps/common/lib/lexical/ast/analysis.ex @@ -116,20 +116,6 @@ defmodule Lexical.Ast.Analysis do end) end - def module_scope(%__MODULE__{} = analysis, %Position{} = pos) do - enclosing_scopes = scopes_at(analysis, pos) - - first_scope = List.first(enclosing_scopes) - - Enum.reduce_while(enclosing_scopes, first_scope, fn - %Scope{module: same} = current, %Scope{module: same} -> - {:cont, current} - - _, current -> - {:halt, current} - end) - end - defp enclosing_scopes(scopes, range) do Enum.filter(scopes, fn scope -> Range.contains?(scope.range, range.start) From 16ce0e172b0f3a78929f1d49e42deacff45434c1 Mon Sep 17 00:00:00 2001 From: Moosieus Date: Sun, 4 Aug 2024 00:24:09 -0400 Subject: [PATCH 4/5] * Organize fetch functions using `with`. * Prevent exact_lookup from returning private definitions for public calls. * Jump to lowest-arity in next best definition function. --- .../code_intelligence/definition.ex | 153 +++++++++++------- .../fixtures/navigations/lib/my_definition.ex | 4 + 2 files changed, 96 insertions(+), 61 deletions(-) diff --git a/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex b/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex index 5f1040c6b..0c7273b23 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex @@ -23,72 +23,95 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do end end - defp fetch_definition({type, entity} = resolved, %Analysis{} = analysis, %Position{} = position) + defp fetch_definition({type, _entity} = resolved, %Analysis{} = analysis, %Position{} = pos) when type in [:struct, :module] do + with {:ok, nil} <- exact_module_definition(resolved) do + elixir_sense_definition(analysis, pos) + end + end + + defp fetch_definition({:call, _m, _f, _a} = resolved, %Analysis{} = nlss, %Position{} = pos) do + with {:ok, nil} <- exact_call_definition(resolved, nlss, pos), + {:ok, nil} <- elixir_sense_definition(nlss, pos) do + nearest_arity_call_definition(resolved, nlss, pos) + end + end + + defp fetch_definition(_, %Analysis{} = analysis, %Position{} = position) do + elixir_sense_definition(analysis, position) + end + + defp exact_module_definition({type, entity} = resolved) do module = Formats.module(entity) locations = - case Store.exact(module, type: type, subtype: :definition) do - {:ok, entries} -> - for entry <- entries, - result = to_location(entry), - match?({:ok, _}, result) do - {:ok, location} = result - location - end - - _ -> - [] - end - - maybe_fallback_to_elixir_sense(resolved, locations, analysis, position) - end - - defp fetch_definition( - {:call, module, function, arity} = resolved, - %Analysis{} = analysis, - %Position{} = position - ) do - mfa = Formats.mfa(module, function, arity) - definitions = query_search_index_exact(mfa, subtype: :definition) + module + |> query_search_index_exact(type: type, subtype: :definition) + |> entries_to_locations() - definitions = - case definitions do - [_ | _] -> - definitions + case locations do + [location] -> + {:ok, location} - _ -> - # feat: search for next best definition when no exact match is present. - {:ok, module_at_position} = Analyzer.current_module(analysis, position) + [_ | _] -> + {:ok, locations} - call_prefix = Formats.mf(module, function) - definitions = query_search_index_prefix(call_prefix, subtype: :definition) + [] -> + Logger.info("No definition found for #{inspect(resolved)} with Indexer.") + {:ok, nil} + end + end - if module == module_at_position do - definitions - else - Stream.reject(definitions, &(&1.type == {:function, :private})) - end - end + defp exact_call_definition({:call, module, function, arity} = resolved, analysis, position) do + mfa = Formats.mfa(module, function, arity) - definitions = - definitions + locations = + mfa + |> query_search_index_exact(subtype: :definition) |> Stream.flat_map(&resolve_defdelegate/1) |> Stream.uniq_by(& &1.subject) + |> maybe_reject_private_defs(module, analysis, position) + |> entries_to_locations() - locations = - for entry <- definitions, - result = to_location(entry), - match?({:ok, _}, result) do - {:ok, location} = result - location - end + case locations do + [location] -> + {:ok, location} - maybe_fallback_to_elixir_sense(resolved, locations, analysis, position) + [_ | _] -> + {:ok, locations} + + [] -> + Logger.info("No definition found for #{inspect(resolved)} with Indexer.") + {:ok, nil} + end end - defp fetch_definition(_, %Analysis{} = analysis, %Position{} = position) do - elixir_sense_definition(analysis, position) + defp nearest_arity_call_definition({:call, m, f, _a} = resolved, nlss, pos) do + mf_prefix = Formats.mf(m, f) + + locations = + mf_prefix + |> query_search_index_prefix(subtype: :definition) + |> Stream.flat_map(&resolve_defdelegate/1) + |> Stream.uniq_by(& &1.subject) + |> maybe_reject_private_defs(m, nlss, pos) + |> Enum.sort(fn %Entry{} = a, %Entry{} = b -> + String.last(a.subject) < String.last(b.subject) + end) + |> Enum.take(1) + |> entries_to_locations() + + case locations do + [location] -> + {:ok, location} + + [_ | _] -> + {:ok, locations} + + [] -> + Logger.info("No nearest-arity definition found for #{inspect(resolved)} with Indexer.") + {:ok, nil} + end end def resolve_defdelegate(%Entry{type: {:function, :delegate}} = entry) do @@ -100,18 +123,26 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do [entry] end - defp maybe_fallback_to_elixir_sense(resolved, locations, analysis, position) do - case locations do - [] -> - Logger.info("No definition found for #{inspect(resolved)} with Indexer.") - - elixir_sense_definition(analysis, position) + defp maybe_reject_private_defs(entries, module, analysis, position) do + case Analyzer.current_module(analysis, position) do + {:ok, module_at_position} -> + if module != module_at_position do + Stream.reject(entries, &(&1.type == {:function, :private})) + else + entries + end - [location] -> - {:ok, location} + :error -> + entries + end + end - _ -> - {:ok, locations} + defp entries_to_locations(entries) do + for entry <- entries, + result = to_location(entry), + match?({:ok, _}, result) do + {:ok, location} = result + location end end diff --git a/apps/remote_control/test/fixtures/navigations/lib/my_definition.ex b/apps/remote_control/test/fixtures/navigations/lib/my_definition.ex index 28d9d4e54..f40056b68 100644 --- a/apps/remote_control/test/fixtures/navigations/lib/my_definition.ex +++ b/apps/remote_control/test/fixtures/navigations/lib/my_definition.ex @@ -18,6 +18,10 @@ defmodule MyDefinition do "Hello, #{name}!" end + def greet(name, name2) do + "Hello, #{name} and #{name2}!" + end + defmacro print_hello do quote do IO.puts("Hello, world!") From dc6e91d76da7f81fc5a964ea28e507fe1b9212a4 Mon Sep 17 00:00:00 2001 From: Moosieus Date: Thu, 14 Nov 2024 20:06:24 -0500 Subject: [PATCH 5/5] Add code comment where we take the next-best arity (currently lowest) --- .../lib/lexical/remote_control/code_intelligence/definition.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex b/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex index 0c7273b23..1dcca5e31 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_intelligence/definition.ex @@ -95,6 +95,7 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Definition do |> Stream.flat_map(&resolve_defdelegate/1) |> Stream.uniq_by(& &1.subject) |> maybe_reject_private_defs(m, nlss, pos) + # sort by arity and take the lowest. |> Enum.sort(fn %Entry{} = a, %Entry{} = b -> String.last(a.subject) < String.last(b.subject) end)