Fix suggest contracts windows regression (#531)

* Dialyzer.suggest_contracts expects absnames

on windows we need to convert from "c:\\some\\other.ex" to "c:/some/other.ex"

Fixes #528

* fix test on windows

* add suggest contracts tests

* apply code review suggestion

* revert non windows compatible change

Author: lukaszsamson

apps/language_server/lib/language_server/providers/formatting.ex

@@ -37,7 +37,7 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
   # If in an umbrella project, the cwd might be set to a sub-app if it's being compiled. This is
   # fine if the file we're trying to format is in that app. Otherwise, we return an error.
   defp can_format?(file_uri = "file:" <> _, project_dir) do
-    file_path = file_uri |> SourceFile.path_from_uri() |> Path.absname()
+    file_path = file_uri |> SourceFile.abs_path_from_uri()
 
     String.starts_with?(file_path, Path.absname(project_dir)) or
       String.starts_with?(file_path, File.cwd!())
@@ -46,7 +46,7 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
   defp can_format?(_uri, _project_dir), do: false
 
   def should_format?(file_uri, project_dir, inputs) when is_list(inputs) do
-    file_path = file_uri |> SourceFile.path_from_uri() |> Path.absname()
+    file_path = file_uri |> SourceFile.abs_path_from_uri()
 
     inputs
     |> Stream.flat_map(fn glob ->

apps/language_server/lib/language_server/server.ex

@@ -75,7 +75,7 @@ defmodule ElixirLS.LanguageServer.Server do
   ## Client API
 
   def start_link(name \\ nil) do
-    GenServer.start_link(__MODULE__, :ok, name: name)
+    GenServer.start_link(__MODULE__, :ok, name: name || __MODULE__)
   end
 
   def receive_packet(server \\ __MODULE__, packet) do
@@ -122,7 +122,11 @@ defmodule ElixirLS.LanguageServer.Server do
   def handle_call({:suggest_contracts, uri = "file:" <> _}, from, state) do
     case state do
       %{analysis_ready?: true, source_files: %{^uri => %{dirty?: false}}} ->
-        {:reply, Dialyzer.suggest_contracts([SourceFile.path_from_uri(uri)]), state}
+        abs_path =
+          uri
+          |> SourceFile.abs_path_from_uri()
+
+        {:reply, Dialyzer.suggest_contracts([abs_path]), state}
 
       _ ->
         awaiting_contracts = reject_awaiting_contracts(state.awaiting_contracts, uri)
@@ -504,7 +508,7 @@ defmodule ElixirLS.LanguageServer.Server do
     state =
       case root_uri do
         "file://" <> _ ->
-          root_path = SourceFile.path_from_uri(root_uri)
+          root_path = SourceFile.abs_path_from_uri(root_uri)
           File.cd!(root_path)
           %{state | root_uri: root_uri}
 
@@ -1040,7 +1044,7 @@ defmodule ElixirLS.LanguageServer.Server do
 
   defp set_project_dir(%{project_dir: prev_project_dir, root_uri: root_uri} = state, project_dir)
        when is_binary(root_uri) do
-    root_dir = root_uri |> SourceFile.path_from_uri() |> Path.absname()
+    root_dir = root_uri |> SourceFile.abs_path_from_uri()
 
     project_dir =
       if is_binary(project_dir) do

apps/language_server/lib/language_server/source_file.ex

@@ -157,6 +157,10 @@ defmodule ElixirLS.LanguageServer.SourceFile do
   defp downcase(char) when char >= ?A and char <= ?Z, do: char + 32
   defp downcase(char), do: char
 
+  def abs_path_from_uri(uri) do
+    uri |> path_from_uri |> Path.absname()
+  end
+
   def full_range(source_file) do
     lines = lines(source_file)
 

apps/language_server/test/dialyzer_test.exs

@@ -1,7 +1,7 @@
 defmodule ElixirLS.LanguageServer.DialyzerTest do
   # TODO: Test loading and saving manifest
 
-  alias ElixirLS.LanguageServer.{Dialyzer, Server, Protocol, SourceFile}
+  alias ElixirLS.LanguageServer.{Dialyzer, Server, Protocol, SourceFile, JsonRpc}
   import ExUnit.CaptureLog
   use ElixirLS.Utils.MixTest.Case, async: false
   use Protocol
@@ -442,4 +442,146 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
       refute_receive notification("textDocument/publishDiagnostics", _), 3000
     end)
   end
+
+  @tag slow: true, fixture: true
+  test "do not suggests contracts if not enabled", %{server: server} do
+    in_fixture(__DIR__, "dialyzer", fn ->
+      file_c = SourceFile.path_to_uri(Path.absname("lib/c.ex"))
+
+      capture_log(fn ->
+        root_uri = SourceFile.path_to_uri(File.cwd!())
+        Server.receive_packet(server, initialize_req(1, root_uri, %{}))
+
+        Server.receive_packet(
+          server,
+          did_change_configuration(%{
+            "elixirLS" => %{"dialyzerEnabled" => true, "dialyzerFormat" => "dialyxir_long"}
+          })
+        )
+
+        message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
+
+        assert publish_diagnostics_notif(_, _) = message
+
+        Server.receive_packet(
+          server,
+          did_open(file_c, "elixir", 2, File.read!(Path.absname("lib/c.ex")))
+        )
+
+        Server.receive_packet(
+          server,
+          code_lens_req(3, file_c)
+        )
+
+        resp = assert_receive(%{"id" => 3}, 5000)
+
+        assert response(3, []) == resp
+      end)
+    end)
+  end
+
+  @tag slow: true, fixture: true
+  test "suggests contracts if enabled and applies suggestion", %{server: server} do
+    in_fixture(__DIR__, "dialyzer", fn ->
+      file_c = SourceFile.path_to_uri(Path.absname("lib/c.ex"))
+
+      capture_log(fn ->
+        root_uri = SourceFile.path_to_uri(File.cwd!())
+        Server.receive_packet(server, initialize_req(1, root_uri, %{}))
+
+        Server.receive_packet(
+          server,
+          did_change_configuration(%{
+            "elixirLS" => %{
+              "dialyzerEnabled" => true,
+              "dialyzerFormat" => "dialyxir_long",
+              "suggestSpecs" => true
+            }
+          })
+        )
+
+        message = assert_receive %{"method" => "textDocument/publishDiagnostics"}, 20000
+
+        assert publish_diagnostics_notif(_, _) = message
+
+        Server.receive_packet(
+          server,
+          did_open(file_c, "elixir", 2, File.read!(Path.absname("lib/c.ex")))
+        )
+
+        Server.receive_packet(
+          server,
+          code_lens_req(3, file_c)
+        )
+
+        resp = assert_receive(%{"id" => 3}, 5000)
+
+        assert response(3, [
+                 %{
+                   "command" => %{
+                     "arguments" =>
+                       args = [
+                         %{
+                           "arity" => 0,
+                           "fun" => "myfun",
+                           "line" => 2,
+                           "mod" => "Elixir.C",
+                           "spec" => "myfun :: 1",
+                           "uri" => ^file_c
+                         }
+                       ],
+                     "command" => command = "spec:" <> _,
+                     "title" => "@spec myfun :: 1"
+                   },
+                   "range" => %{
+                     "end" => %{"character" => 0, "line" => 1},
+                     "start" => %{"character" => 0, "line" => 1}
+                   }
+                 }
+               ]) = resp
+
+        Server.receive_packet(
+          server,
+          execute_command_req(4, command, args)
+        )
+
+        assert_receive(%{
+          "id" => 1,
+          "method" => "workspace/applyEdit",
+          "params" => %{
+            "edit" => %{
+              "changes" => %{
+                ^file_c => [
+                  %{
+                    "newText" => "  @spec myfun :: 1\n",
+                    "range" => %{
+                      "end" => %{"character" => 0, "line" => 1},
+                      "start" => %{"character" => 0, "line" => 1}
+                    }
+                  }
+                ]
+              }
+            },
+            "label" => "Add @spec to Elixir.C.myfun/0"
+          }
+        })
+
+        # TODO something is broken in packet capture
+        # using JsonRpc.receive_packet causes the packet to be delivered to LanguageServer
+        # which crashes with no match error
+        # JsonRpc.receive_packet(
+        #   server,
+        #   response(1, %{"applied" => true})
+        # )
+        # instead we fake a callback in JsonRpc server that forwards the response as needed
+        JsonRpc.handle_call(
+          {:packet, response(1, %{"applied" => true})},
+          nil,
+          :sys.get_state(JsonRpc)
+        )
+
+        assert_receive(%{"id" => 4, "result" => nil}, 5000)
+      end)
+    end)
+  end
 end

apps/language_server/test/server_test.exs

@@ -1189,9 +1189,14 @@ defmodule ElixirLS.LanguageServer.ServerTest do
     in_fixture(__DIR__, "test_code_lens", fn ->
       file_path = "test/fixture_test.exs"
       file_uri = SourceFile.path_to_uri(file_path)
+      # this is not an abs path as returned by Path.absname
+      # on Windows it's c:\asdf instead of c:/asdf
       file_absolute_path = SourceFile.path_from_uri(file_uri)
       text = File.read!(file_path)
-      project_dir = SourceFile.path_from_uri(root_uri())
+
+      project_dir =
+        root_uri()
+        |> SourceFile.abs_path_from_uri()
 
       initialize(server)