From d9bacac213817fbfaa7a6edd58518b251f4dbd98 Mon Sep 17 00:00:00 2001 From: Mitchell Hanberg Date: Sat, 22 Jul 2023 21:46:53 -0400 Subject: [PATCH 1/2] fix: create the symbol table in the workspace path --- lib/next_ls.ex | 149 +++++++++++++---------------- lib/next_ls/lsp_supervisor.ex | 9 -- lib/next_ls/runtime.ex | 23 ++++- lib/next_ls/runtime/sidecar.ex | 25 +++++ lib/next_ls/runtime/supervisor.ex | 29 ++++++ lib/next_ls/runtime_supervisor.ex | 18 ---- lib/next_ls/symbol_table.ex | 12 ++- test/next_ls/symbol_table_test.exs | 11 ++- test/next_ls_test.exs | 4 +- 9 files changed, 157 insertions(+), 123 deletions(-) create mode 100644 lib/next_ls/runtime/sidecar.ex create mode 100644 lib/next_ls/runtime/supervisor.ex delete mode 100644 lib/next_ls/runtime_supervisor.ex diff --git a/lib/next_ls.ex b/lib/next_ls.ex index a0762bef..b575a943 100644 --- a/lib/next_ls.ex +++ b/lib/next_ls.ex @@ -42,8 +42,7 @@ defmodule NextLS do :runtime_task_supervisor, :dynamic_supervisor, :extensions, - :registry, - :symbol_table + :registry ]) GenLSP.start_link(__MODULE__, args, opts) @@ -58,7 +57,6 @@ defmodule NextLS do registry = Keyword.fetch!(args, :registry) extensions = Keyword.get(args, :extensions, [NextLS.ElixirExtension]) cache = Keyword.fetch!(args, :cache) - symbol_table = Keyword.fetch!(args, :symbol_table) {:ok, logger} = DynamicSupervisor.start_child(dynamic_supervisor, {NextLS.Logger, lsp: lsp}) {:ok, @@ -68,7 +66,6 @@ defmodule NextLS do refresh_refs: %{}, cache: cache, logger: logger, - symbol_table: symbol_table, task_supervisor: task_supervisor, runtime_task_supervisor: runtime_task_supervisor, dynamic_supervisor: dynamic_supervisor, @@ -118,35 +115,39 @@ defmodule NextLS do def handle_request(%TextDocumentDefinition{params: %{text_document: %{uri: uri}, position: position}}, lsp) do result = - case Definition.fetch( - URI.parse(uri).path, - {position.line + 1, position.character + 1}, - :symbol_table, - :reference_table - ) do - nil -> - nil - - [] -> - nil - - [{file, line, column} | _] -> - %Location{ - uri: "file://#{file}", - range: %Range{ - start: %Position{ - line: line - 1, - character: column - 1 - }, - end: %Position{ - line: line - 1, - character: column - 1 + dispatch(lsp.assigns.registry, :symbol_tables, fn entries -> + for {_, %{symbol_table: symbol_table, reference_table: ref_table}} <- entries do + case Definition.fetch( + URI.parse(uri).path, + {position.line + 1, position.character + 1}, + symbol_table, + ref_table + ) do + nil -> + nil + + [] -> + nil + + [{file, line, column} | _] -> + %Location{ + uri: "file://#{file}", + range: %Range{ + start: %Position{ + line: line - 1, + character: column - 1 + }, + end: %Position{ + line: line - 1, + character: column - 1 + } + } } - } - } - end + end + end + end) - {:reply, result, lsp} + {:reply, List.first(result), lsp} end def handle_request(%TextDocumentDocumentSymbol{params: %{text_document: %{uri: uri}}}, lsp) do @@ -174,32 +175,34 @@ defmodule NextLS do end symbols = - for %SymbolTable.Symbol{} = symbol <- SymbolTable.symbols(lsp.assigns.symbol_table), filter.(symbol.name) do - name = - if symbol.type != :defstruct do - "#{symbol.type} #{symbol.name}" - else - "#{symbol.name}" - end - - %SymbolInformation{ - name: name, - kind: elixir_kind_to_lsp_kind(symbol.type), - location: %Location{ - uri: "file://#{symbol.file}", - range: %Range{ - start: %Position{ - line: symbol.line - 1, - character: symbol.col - 1 - }, - end: %Position{ - line: symbol.line - 1, - character: symbol.col - 1 + dispatch(lsp.assigns.registry, :symbol_tables, fn entries -> + for {pid, _} <- entries, %SymbolTable.Symbol{} = symbol <- SymbolTable.symbols(pid), filter.(symbol.name) do + name = + if symbol.type != :defstruct do + "#{symbol.type} #{symbol.name}" + else + "#{symbol.name}" + end + + %SymbolInformation{ + name: name, + kind: elixir_kind_to_lsp_kind(symbol.type), + location: %Location{ + uri: "file://#{symbol.file}", + range: %Range{ + start: %Position{ + line: symbol.line - 1, + character: symbol.col - 1 + }, + end: %Position{ + line: symbol.line - 1, + character: symbol.col - 1 + } } } } - } - end + end + end) {:reply, symbols, lsp} end @@ -248,7 +251,9 @@ defmodule NextLS do end def handle_request(%Shutdown{}, lsp) do - SymbolTable.close(lsp.assigns.symbol_table) + dispatch(lsp.assigns.registry, :symbol_tables, fn entries -> + for {pid, _} <- entries, do: SymbolTable.close(pid) + end) {:reply, nil, assign(lsp, exit_code: 0)} end @@ -281,18 +286,19 @@ defmodule NextLS do token = token() Progress.start(lsp, token, "Initializing NextLS runtime for folder #{name}...") parent = self() + working_dir = URI.parse(uri).path {:ok, runtime} = DynamicSupervisor.start_child( lsp.assigns.dynamic_supervisor, - {NextLS.RuntimeSupervisor, + {NextLS.Runtime.Supervisor, + path: Path.join(working_dir, ".elixir-tools"), + name: name, + registry: lsp.assigns.registry, runtime: [ - name: name, task_supervisor: lsp.assigns.runtime_task_supervisor, - registry: lsp.assigns.registry, - working_dir: URI.parse(uri).path, + working_dir: working_dir, uri: uri, - parent: parent, on_initialized: fn status -> if status == :ready do Progress.stop(lsp, token, "NextLS runtime for folder #{name} has initialized!") @@ -307,10 +313,6 @@ defmodule NextLS do ]} ) - ref = Process.monitor(runtime) - - Process.put(ref, name) - {name, %{uri: uri, runtime: runtime}} end @@ -387,16 +389,6 @@ defmodule NextLS do {:noreply, lsp} end - def handle_info({:tracer, payload}, lsp) do - SymbolTable.put_symbols(lsp.assigns.symbol_table, payload) - {:noreply, lsp} - end - - def handle_info({{:tracer, :reference}, payload}, lsp) do - SymbolTable.put_reference(lsp.assigns.symbol_table, payload) - {:noreply, lsp} - end - def handle_info(:publish, lsp) do GenLSP.log(lsp, "[NextLS] Compiled!") @@ -449,15 +441,6 @@ defmodule NextLS do {:noreply, assign(lsp, refresh_refs: refs)} end - def handle_info({:DOWN, ref, :process, _runtime, _reason}, lsp) do - name = Process.get(ref) - Process.delete(ref) - - GenLSP.error(lsp, "[NextLS] The runtime for #{name} has crashed") - - {:noreply, lsp} - end - def handle_info(message, lsp) do GenLSP.log(lsp, "[NextLS] Unhandled message: #{inspect(message)}") {:noreply, lsp} diff --git a/lib/next_ls/lsp_supervisor.ex b/lib/next_ls/lsp_supervisor.ex index 46515228..69fc3c56 100644 --- a/lib/next_ls/lsp_supervisor.ex +++ b/lib/next_ls/lsp_supervisor.ex @@ -51,24 +51,15 @@ defmodule NextLS.LSPSupervisor do raise OptionsError, invalid end - # FIXME: this directory should be inside the workspace, which will is not determined until - # the LSP has begun initialization - # The symbol table may need to started dynamically, like the extensions and the runtimes - hidden_folder = Path.expand(".elixir-tools") - File.mkdir_p!(hidden_folder) - File.write!(Path.join(hidden_folder, ".gitignore"), "*\n") - children = [ {DynamicSupervisor, name: NextLS.DynamicSupervisor}, {Task.Supervisor, name: NextLS.TaskSupervisor}, {Task.Supervisor, name: :runtime_task_supervisor}, {GenLSP.Buffer, buffer_opts}, {NextLS.DiagnosticCache, name: :diagnostic_cache}, - {NextLS.SymbolTable, name: :symbol_table, path: hidden_folder}, {Registry, name: NextLS.Registry, keys: :duplicate}, {NextLS, cache: :diagnostic_cache, - symbol_table: :symbol_table, task_supervisor: NextLS.TaskSupervisor, runtime_task_supervisor: :runtime_task_supervisor, dynamic_supervisor: NextLS.DynamicSupervisor, diff --git a/lib/next_ls/runtime.ex b/lib/next_ls/runtime.ex index 1179a217..b4ec38ab 100644 --- a/lib/next_ls/runtime.ex +++ b/lib/next_ls/runtime.ex @@ -53,6 +53,18 @@ defmodule NextLS.Runtime do Registry.register(registry, :runtimes, %{name: name, uri: uri}) + pid = + cond do + is_pid(parent) -> parent + is_atom(parent) -> Process.whereis(parent) + end + + parent = + pid + |> :erlang.term_to_binary() + |> Base.encode64() + |> String.to_charlist() + port = Port.open( {:spawn_executable, @exe}, @@ -64,7 +76,7 @@ defmodule NextLS.Runtime do cd: working_dir, env: [ {~c"LSP", ~c"nextls"}, - {~c"NEXTLS_PARENT_PID", parent |> :erlang.term_to_binary() |> Base.encode64() |> String.to_charlist()}, + {~c"NEXTLS_PARENT_PID", parent}, {~c"MIX_ENV", ~c"dev"}, {~c"MIX_BUILD_ROOT", ~c".elixir-tools/_build"} ], @@ -85,6 +97,15 @@ defmodule NextLS.Runtime do me = self() + Task.Supervisor.async_nolink(task_supervisor, fn -> + ref = Process.monitor(me) + + receive do + {:DOWN, ^ref, :process, ^me, _reason} -> + NextLS.Logger.error(logger, "[NextLS] The runtime for #{name} has crashed") + end + end) + Task.start_link(fn -> with {:ok, host} <- :inet.gethostname(), node <- :"#{sname}@#{host}", diff --git a/lib/next_ls/runtime/sidecar.ex b/lib/next_ls/runtime/sidecar.ex new file mode 100644 index 00000000..6ab4720b --- /dev/null +++ b/lib/next_ls/runtime/sidecar.ex @@ -0,0 +1,25 @@ +defmodule NextLS.Runtime.Sidecar do + @moduledoc false + use GenServer + + alias NextLS.SymbolTable + + def start_link(args) do + GenServer.start_link(__MODULE__, Keyword.take(args, [:symbol_table]), Keyword.take(args, [:name])) + end + + def init(args) do + symbol_table = Keyword.fetch!(args, :symbol_table) + {:ok, %{symbol_table: symbol_table}} + end + + def handle_info({:tracer, payload}, state) do + SymbolTable.put_symbols(state.symbol_table, payload) + {:noreply, state} + end + + def handle_info({{:tracer, :reference}, payload}, state) do + SymbolTable.put_reference(state.symbol_table, payload) + {:noreply, state} + end +end diff --git a/lib/next_ls/runtime/supervisor.ex b/lib/next_ls/runtime/supervisor.ex new file mode 100644 index 00000000..f38ee6c7 --- /dev/null +++ b/lib/next_ls/runtime/supervisor.ex @@ -0,0 +1,29 @@ +defmodule NextLS.Runtime.Supervisor do + @moduledoc false + + use Supervisor + + def start_link(init_arg) do + Supervisor.start_link(__MODULE__, init_arg) + end + + @impl true + def init(init_arg) do + name = init_arg[:name] + registry = init_arg[:registry] + hidden_folder = init_arg[:path] + File.mkdir_p!(hidden_folder) + File.write!(Path.join(hidden_folder, ".gitignore"), "*\n") + + symbol_table_name = :"symbol-table-#{name}" + sidecar_name = :"sidecar-#{name}" + + children = [ + {NextLS.SymbolTable, workspace: name, path: hidden_folder, registry: registry, name: symbol_table_name}, + {NextLS.Runtime.Sidecar, name: sidecar_name, symbol_table: symbol_table_name}, + {NextLS.Runtime, init_arg[:runtime] ++ [name: name, registry: registry, parent: sidecar_name]} + ] + + Supervisor.init(children, strategy: :one_for_one) + end +end diff --git a/lib/next_ls/runtime_supervisor.ex b/lib/next_ls/runtime_supervisor.ex deleted file mode 100644 index c27301fe..00000000 --- a/lib/next_ls/runtime_supervisor.ex +++ /dev/null @@ -1,18 +0,0 @@ -defmodule NextLS.RuntimeSupervisor do - @moduledoc false - - use Supervisor - - def start_link(init_arg) do - Supervisor.start_link(__MODULE__, init_arg) - end - - @impl true - def init(init_arg) do - children = [ - {NextLS.Runtime, init_arg[:runtime]} - ] - - Supervisor.init(children, strategy: :one_for_one) - end -end diff --git a/lib/next_ls/symbol_table.ex b/lib/next_ls/symbol_table.ex index 7d6b97af..249f2c67 100644 --- a/lib/next_ls/symbol_table.ex +++ b/lib/next_ls/symbol_table.ex @@ -17,7 +17,7 @@ defmodule NextLS.SymbolTable do end def start_link(args) do - GenServer.start_link(__MODULE__, Keyword.take(args, [:path]), Keyword.take(args, [:name])) + GenServer.start_link(__MODULE__, Keyword.take(args, [:path, :workspace, :registry]), Keyword.take(args, [:name])) end @spec put_symbols(pid() | atom(), list(tuple())) :: :ok @@ -37,21 +37,23 @@ defmodule NextLS.SymbolTable do def init(args) do path = Keyword.fetch!(args, :path) - symbol_table_name = Keyword.get(args, :symbol_table_name, :symbol_table) - reference_table_name = Keyword.get(args, :reference_table_name, :reference_table) + name = Keyword.fetch!(args, :workspace) + registry = Keyword.fetch!(args, :registry) {:ok, name} = - :dets.open_file(symbol_table_name, + :dets.open_file(:"symbol-table-#{name}", file: path |> Path.join("symbol_table.dets") |> String.to_charlist(), type: :duplicate_bag ) {:ok, ref_name} = - :dets.open_file(reference_table_name, + :dets.open_file(:"reference-table-#{name}", file: path |> Path.join("reference_table.dets") |> String.to_charlist(), type: :duplicate_bag ) + Registry.register(registry, :symbol_tables, %{symbol_table: name, reference_table: ref_name}) + {:ok, %{table: name, reference_table: ref_name}} end diff --git a/test/next_ls/symbol_table_test.exs b/test/next_ls/symbol_table_test.exs index b9f689ec..9cf55b61 100644 --- a/test/next_ls/symbol_table_test.exs +++ b/test/next_ls/symbol_table_test.exs @@ -5,11 +5,13 @@ defmodule NextLS.SymbolTableTest do @moduletag :tmp_dir - setup %{tmp_dir: dir} do + setup %{tmp_dir: dir} = cxt do File.mkdir_p!(dir) + start_supervised!({Registry, [keys: :duplicate, name: Registry.SymbolTableTest.Registry]}) # this fails with `{:error, incompatible_arguments}` on CI a lot, and I have no idea why - pid = try_start_supervised({SymbolTable, [path: dir]}, 10) + pid = + try_start_supervised({SymbolTable, [path: dir, workspace: cxt.test, registry: Registry.SymbolTableTest.Registry]}, 10) Process.link(pid) [pid: pid, dir: dir] @@ -27,9 +29,10 @@ defmodule NextLS.SymbolTableTest do try_start_supervised(spec, num - 1) end - test "creates a dets table", %{dir: dir, pid: pid} do + test "creates a dets table", %{dir: dir, pid: pid} = cxt do assert File.exists?(Path.join([dir, "symbol_table.dets"])) - assert :sys.get_state(pid).table == :symbol_table + assert File.exists?(Path.join([dir, "reference_table.dets"])) + assert :sys.get_state(pid).table == :"symbol-table-#{cxt.test}" end test "builds the symbol table", %{pid: pid} do diff --git a/test/next_ls_test.exs b/test/next_ls_test.exs index acb2725a..d890bcb5 100644 --- a/test/next_ls_test.exs +++ b/test/next_ls_test.exs @@ -870,7 +870,6 @@ defmodule NextLSTest do start_supervised!({Registry, [keys: :duplicate, name: Registry.NextLSTest.Registry]}) extensions = [NextLS.ElixirExtension] cache = start_supervised!(NextLS.DiagnosticCache) - symbol_table = start_supervised!({NextLS.SymbolTable, path: tmp_dir}) server = server(NextLS, @@ -879,8 +878,7 @@ defmodule NextLSTest do dynamic_supervisor: rvisor, registry: Registry.NextLSTest.Registry, extensions: extensions, - cache: cache, - symbol_table: symbol_table + cache: cache ) Process.link(server.lsp) From cec717376d5b0a9a4ce0672dd79bbbf0ae213371 Mon Sep 17 00:00:00 2001 From: Mitchell Hanberg Date: Sun, 23 Jul 2023 23:54:00 -0400 Subject: [PATCH 2/2] fixup! fix: create the symbol table in the workspace path --- test/next_ls/symbol_table_test.exs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/next_ls/symbol_table_test.exs b/test/next_ls/symbol_table_test.exs index 9cf55b61..0a47ca27 100644 --- a/test/next_ls/symbol_table_test.exs +++ b/test/next_ls/symbol_table_test.exs @@ -11,7 +11,10 @@ defmodule NextLS.SymbolTableTest do start_supervised!({Registry, [keys: :duplicate, name: Registry.SymbolTableTest.Registry]}) # this fails with `{:error, incompatible_arguments}` on CI a lot, and I have no idea why pid = - try_start_supervised({SymbolTable, [path: dir, workspace: cxt.test, registry: Registry.SymbolTableTest.Registry]}, 10) + try_start_supervised( + {SymbolTable, [path: dir, workspace: cxt.test, registry: Registry.SymbolTableTest.Registry]}, + 10 + ) Process.link(pid) [pid: pid, dir: dir]