Merge pull request #1210 from elixir-lsp/codex/modify-handle_call-to-return-error-tuple

Handle mix project cache before load

Author: lukaszsamson

apps/language_server/lib/language_server/mix_project_cache.ex

@@ -20,17 +20,20 @@ defmodule ElixirLS.LanguageServer.MixProjectCache do
     GenServer.call(__MODULE__, :loaded?)
   end
 
-  @spec get() :: module | nil
+  @spec get() :: {:ok, module | nil} | {:error, :not_loaded}
   def get do
     GenServer.call(__MODULE__, {:get, :get})
   end
 
   @spec get!() :: module
   def get! do
-    get() || raise Mix.NoProjectError, []
+    case get() do
+      {:ok, project} when not is_nil(project) -> project
+      _ -> raise Mix.NoProjectError, []
+    end
   end
 
-  @spec project_file() :: binary | nil
+  @spec project_file() :: {:ok, binary | nil} | {:error, :not_loaded}
   def project_file() do
     GenServer.call(__MODULE__, {:get, :project_file})
   end
@@ -39,47 +42,47 @@ defmodule ElixirLS.LanguageServer.MixProjectCache do
   # @spec parent_umbrella_project_file() :: binary | nil
   # defdelegate parent_umbrella_project_file(), to: Mix.ProjectStack
 
-  @spec config() :: keyword
+  @spec config() :: {:ok, keyword} | {:error, :not_loaded}
   def config do
     GenServer.call(__MODULE__, {:get, :config})
   end
 
-  @spec config_files() :: [Path.t()]
+  @spec config_files() :: {:ok, [Path.t()]} | {:error, :not_loaded}
   def config_files do
     GenServer.call(__MODULE__, {:get, :config_files})
   end
 
-  @spec config_mtime() :: posix_mtime when posix_mtime: integer()
+  @spec config_mtime() :: {:ok, posix_mtime} | {:error, :not_loaded} when posix_mtime: integer()
   def config_mtime do
     GenServer.call(__MODULE__, {:get, :config_mtime})
   end
 
-  @spec umbrella?() :: boolean
+  @spec umbrella?() :: {:ok, boolean} | {:error, :not_loaded}
   def umbrella?() do
     GenServer.call(__MODULE__, {:get, :umbrella?})
   end
 
-  @spec apps_paths() :: %{optional(atom) => Path.t()} | nil
+  @spec apps_paths() :: {:ok, %{optional(atom) => Path.t()} | nil} | {:error, :not_loaded}
   def apps_paths() do
     GenServer.call(__MODULE__, {:get, :apps_paths})
   end
 
-  @spec deps_path() :: Path.t()
+  @spec deps_path() :: {:ok, Path.t()} | {:error, :not_loaded}
   def deps_path() do
     GenServer.call(__MODULE__, {:get, :deps_path})
   end
 
-  @spec deps_apps() :: [atom()]
+  @spec deps_apps() :: {:ok, [atom()]} | {:error, :not_loaded}
   def deps_apps() do
     GenServer.call(__MODULE__, {:get, :deps_apps})
   end
 
-  @spec deps_scms() :: %{optional(atom) => Mix.SCM.t()}
+  @spec deps_scms() :: {:ok, %{optional(atom) => Mix.SCM.t()}} | {:error, :not_loaded}
   def deps_scms() do
     GenServer.call(__MODULE__, {:get, :deps_scms})
   end
 
-  @spec deps_paths() :: %{optional(atom) => Path.t()}
+  @spec deps_paths() :: {:ok, %{optional(atom) => Path.t()}} | {:error, :not_loaded}
   def deps_paths() do
     GenServer.call(__MODULE__, {:get, :deps_paths})
   end
@@ -90,24 +93,25 @@ defmodule ElixirLS.LanguageServer.MixProjectCache do
   #   traverse_deps(opts, fn %{deps: deps} -> Enum.map(deps, & &1.app) end)
   # end
 
-  @spec build_path() :: Path.t()
+  @spec build_path() :: {:ok, Path.t()} | {:error, :not_loaded}
   def build_path() do
     GenServer.call(__MODULE__, {:get, :build_path})
   end
 
-  @spec manifest_path() :: Path.t()
+  @spec manifest_path() :: {:ok, Path.t()} | {:error, :not_loaded}
   def manifest_path() do
     GenServer.call(__MODULE__, {:get, :manifest_path})
   end
 
-  @spec app_path() :: Path.t()
+  @spec app_path() :: {:ok, Path.t()} | {:error, :not_loaded}
   def app_path() do
-    config = config()
+    {:ok, config} = config()
 
     config[:deps_app_path] ||
       cond do
         app = config[:app] ->
-          Path.join([build_path(), "lib", Atom.to_string(app)])
+          {:ok, build_path} = build_path()
+          Path.join([build_path, "lib", Atom.to_string(app)])
 
         config[:apps_path] ->
           raise "trying to access Mix.Project.app_path/1 for an umbrella project but umbrellas have no app"
@@ -121,9 +125,11 @@ defmodule ElixirLS.LanguageServer.MixProjectCache do
       end
   end
 
-  @spec compile_path() :: Path.t()
+  @spec compile_path() :: {:ok, Path.t()} | {:error, :not_loaded}
   def compile_path() do
-    Path.join(app_path(), "ebin")
+    with {:ok, app_path} <- app_path() do
+      {:ok, Path.join(app_path, "ebin")}
+    end
   end
 
   # @spec consolidation_path() :: Path.t()
@@ -168,8 +174,12 @@ defmodule ElixirLS.LanguageServer.MixProjectCache do
   end
 
   @impl GenServer
+  def handle_call({:get, _key}, _from, nil = state) do
+    {:reply, {:error, :not_loaded}, state}
+  end
+
   def handle_call({:get, key}, _from, state) do
-    {:reply, Map.fetch!(state, key), state}
+    {:reply, {:ok, Map.fetch!(state, key)}, state}
   end
 
   def handle_call({:store, state}, _from, _state) do

apps/language_server/lib/language_server/server.ex

@@ -1690,11 +1690,13 @@ defmodule ElixirLS.LanguageServer.Server do
         Map.get(state.settings || %{}, "enableTestLenses", false)
 
     if state.mix_project? and enabled? and MixProjectCache.loaded?() do
+      {:ok, umbrella?} = MixProjectCache.umbrella?()
+
       get_test_code_lenses(
         state,
         uri,
         source_file,
-        MixProjectCache.umbrella?()
+        umbrella?
       )
     else
       {:ok, []}
@@ -1711,16 +1713,23 @@ defmodule ElixirLS.LanguageServer.Server do
     file_path = SourceFile.Path.from_uri(uri)
 
     MixProjectCache.apps_paths()
-    |> Enum.find(fn {_app, app_path} -> under_app?(file_path, project_dir, app_path) end)
     |> case do
-      nil ->
+      {:error, :not_loaded} ->
         {:ok, []}
 
-      {app, app_path} ->
-        if is_test_file?(file_path, state, app, app_path) do
-          CodeLens.test_code_lens(parser_context, Path.join(project_dir, app_path))
-        else
-          {:ok, []}
+      {:ok, apps_paths} ->
+        apps_paths
+        |> Enum.find(fn {_app, app_path} -> under_app?(file_path, project_dir, app_path) end)
+        |> case do
+          nil ->
+            {:ok, []}
+
+          {app, app_path} ->
+            if is_test_file?(file_path, state, app, app_path) do
+              CodeLens.test_code_lens(parser_context, Path.join(project_dir, app_path))
+            else
+              {:ok, []}
+            end
         end
     end
   end
@@ -1764,15 +1773,20 @@ defmodule ElixirLS.LanguageServer.Server do
   end
 
   defp is_test_file?(file_path, project_dir) do
-    config = MixProjectCache.config()
-    test_paths = config[:test_paths] || ["test"]
-    test_pattern = config[:test_pattern] || "*_test.exs"
+    case MixProjectCache.config() do
+      {:error, :not_loaded} ->
+        false
 
-    file_path = SourceFile.Path.expand(file_path, project_dir)
+      {:ok, config} ->
+        test_paths = config[:test_paths] || ["test"]
+        test_pattern = config[:test_pattern] || "*_test.exs"
 
-    Mix.Utils.extract_files(test_paths, test_pattern)
-    |> Enum.map(&SourceFile.Path.absname(&1, project_dir))
-    |> Enum.any?(&(&1 == file_path))
+        file_path = SourceFile.Path.expand(file_path, project_dir)
+
+        Mix.Utils.extract_files(test_paths, test_pattern)
+        |> Enum.map(&SourceFile.Path.absname(&1, project_dir))
+        |> Enum.any?(&(&1 == file_path))
+    end
   end
 
   defp under_app?(file_path, project_dir, app_path) do

apps/language_server/lib/language_server/source_file.ex

@@ -246,11 +246,16 @@ defmodule ElixirLS.LanguageServer.SourceFile do
 
       if mix_project? do
         if MixProjectCache.loaded?() do
+          {:ok, deps_paths} = MixProjectCache.deps_paths()
+          {:ok, manifest_path} = MixProjectCache.manifest_path()
+          {:ok, config_mtime} = MixProjectCache.config_mtime()
+          {:ok, mix_project} = MixProjectCache.get()
+
           opts = [
-            deps_paths: MixProjectCache.deps_paths(),
-            manifest_path: MixProjectCache.manifest_path(),
-            config_mtime: MixProjectCache.config_mtime(),
-            mix_project: MixProjectCache.get(),
+            deps_paths: deps_paths,
+            manifest_path: manifest_path,
+            config_mtime: config_mtime,
+            mix_project: mix_project,
             root: project_dir,
             plugin_loader: fn plugins ->
               for plugin <- plugins do

apps/language_server/test/mix_project_cache_test.exs

@@ -0,0 +1,16 @@
+defmodule ElixirLS.LanguageServer.MixProjectCacheTest do
+  use ExUnit.Case, async: true
+
+  alias ElixirLS.LanguageServer.MixProjectCache
+
+  setup do
+    {:ok, pid} = start_supervised(MixProjectCache)
+    %{pid: pid}
+  end
+
+  test "returns not_loaded when state is nil", %{pid: pid} do
+    assert {:error, :not_loaded} = MixProjectCache.get()
+    assert {:error, :not_loaded} = MixProjectCache.config()
+    assert Process.alive?(pid)
+  end
+end

apps/language_server/test/server_test.exs

@@ -2253,6 +2253,24 @@ defmodule ElixirLS.LanguageServer.ServerTest do
     end
   end
 
+  describe "MixProjectCache not loaded" do
+    @tag :fixture
+    test "code lens request before project load returns empty list", %{server: server} do
+      in_fixture(__DIR__, "no_mixfile", fn ->
+        initialize(server, %{"enableTestLenses" => true, "dialyzerEnabled" => false})
+
+        file_path = "a.ex"
+        file_uri = SourceFile.Path.to_uri(file_path)
+        text = File.read!(file_path)
+
+        Server.receive_packet(server, did_open(file_uri, "elixir", 1, text))
+        Server.receive_packet(server, code_lens_req(4, file_uri))
+
+        assert_receive(%{"id" => 4, "result" => []}, 1000)
+      end)
+    end
+  end
+
   defp with_new_server(packet_capture, func) do
     server = start_supervised!({Server, nil})
     {:ok, mix_project} = start_supervised(MixProjectCache)