Normalize diagnostic messages

apps/language_server/lib/language_server/build.ex

@@ -1,5 +1,5 @@
 defmodule ElixirLS.LanguageServer.Build do
-  alias ElixirLS.LanguageServer.{Server, JsonRpc, SourceFile}
+  alias ElixirLS.LanguageServer.{Server, JsonRpc, SourceFile, Diagnostics}
 
   def build(parent, root_path, fetch_deps?) do
     if Path.absname(File.cwd!()) != Path.absname(root_path) do
@@ -22,6 +22,7 @@ defmodule ElixirLS.LanguageServer.Build do
                     do: fetch_deps()
 
                   {status, diagnostics} = compile()
+                  diagnostics = Diagnostics.normalize(diagnostics, root_path)
                   Server.build_finished(parent, {status, mixfile_diagnostics ++ diagnostics})
 
                 {:error, mixfile_diagnostics} ->

apps/language_server/lib/language_server/diagnostics.ex

@@ -0,0 +1,151 @@
+defmodule ElixirLS.LanguageServer.Diagnostics do
+  def normalize(diagnostics, root_path) do
+    for diagnostic <- diagnostics do
+      {type, file, line, description, stacktrace} =
+        extract_message_info(diagnostic.message, root_path)
+
+      diagnostic
+      |> update_message(type, description, stacktrace)
+      |> maybe_update_file(file)
+      |> maybe_update_position(line, stacktrace)
+    end
+  end
+
+  defp extract_message_info(list, root_path) when is_list(list) do
+    list
+    |> Enum.join()
+    |> extract_message_info(root_path)
+  end
+
+  defp extract_message_info(diagnostic_message, root_path) do
+    {reversed_stacktrace, reversed_description} =
+      diagnostic_message
+      |> String.trim_trailing()
+      |> String.split("\n")
+      |> Enum.reverse()
+      |> Enum.split_while(&is_stack?/1)
+
+    message = reversed_description |> Enum.reverse() |> Enum.join("\n") |> String.trim()
+    stacktrace = reversed_stacktrace |> Enum.map(&String.trim/1) |> Enum.reverse()
+
+    {type, message_without_type} = split_type_and_message(message)
+    {file, line, description} = split_file_and_description(message_without_type, root_path)
+
+    {type, file, line, description, stacktrace}
+  end
+
+  defp update_message(diagnostic, type, description, stacktrace) do
+    description =
+      if type do
+        "(#{type}) #{description}"
+      else
+        description
+      end
+
+    message =
+      if stacktrace != [] do
+        stacktrace =
+          stacktrace
+          |> Enum.map(&"  │ #{&1}")
+          |> Enum.join("\n")
+          |> String.trim_trailing()
+
+        description <> "\n\n" <> "Stacktrace:\n" <> stacktrace
+      else
+        description
+      end
+
+    Map.put(diagnostic, :message, message)
+  end
+
+  defp maybe_update_file(diagnostic, path) do
+    if path do
+      Map.put(diagnostic, :file, path)
+    else
+      diagnostic
+    end
+  end
+
+  defp maybe_update_position(diagnostic, line, stacktrace) do
+    cond do
+      line ->
+        %{diagnostic | position: line}
+
+      diagnostic.position ->
+        diagnostic
+
+      true ->
+        line = extract_line_from_stacktrace(diagnostic.file, stacktrace)
+        %{diagnostic | position: line}
+    end
+  end
+
+  defp split_type_and_message(message) do
+    case Regex.run(~r/^\*\* \(([\w\.]+?)?\) (.*)/s, message) do
+      [_, type, rest] ->
+        {type, rest}
+
+      _ ->
+        {nil, message}
+    end
+  end
+
+  defp split_file_and_description(message, root_path) do
+    with [_, file, line, description] <- Regex.run(~r/^(.*?):(\d+): (.*)/s, message),
+         {:ok, path} <- file_path(file, root_path) do
+      {path, String.to_integer(line), description}
+    else
+      _ ->
+        {nil, nil, message}
+    end
+  end
+
+  defp file_path(nil, _root_path) do
+    {:error, :file_not_found}
+  end
+
+  defp file_path(file, root_path) do
+    path = Path.join([root_path, file])
+
+    if File.exists?(path) do
+      {:ok, path}
+    else
+      file_path_in_umbrella(file, root_path)
+    end
+  end
+
+  defp file_path_in_umbrella(file, root_path) do
+    case [root_path, "apps", "*", file] |> Path.join() |> Path.wildcard() do
+      [] ->
+        {:error, :file_not_found}
+
+      [path] ->
+        {:ok, path}
+
+      _ ->
+        {:error, :more_than_one_file_found}
+    end
+  end
+
+  defp is_stack?("    " <> str) do
+    Regex.match?(~r/.*\.(ex|erl):\d+: /, str) ||
+      Regex.match?(~r/.*expanding macro: /, str)
+  end
+
+  defp is_stack?(_) do
+    false
+  end
+
+  defp extract_line_from_stacktrace(file, stacktrace) do
+    Enum.find_value(stacktrace, fn stack_item ->
+      with [_, _, file_relative, line] <-
+             Regex.run(~r/(\(.+?\)\s+)?(.*\.ex):(\d+): /, stack_item),
+           true <- String.ends_with?(file, file_relative) do
+        String.to_integer(line)
+      else
+        _ ->
+          nil
+      end
+    end)
+  end
+end

apps/language_server/lib/language_server/server.ex

@@ -143,7 +143,9 @@ defmodule ElixirLS.LanguageServer.Server do
     JsonRpc.register_capability_request("workspace/didChangeWatchedFiles", %{
       "watchers" => [
         %{"globPattern" => "**/*.ex"},
-        %{"globPattern" => "**/*.exs"}
+        %{"globPattern" => "**/*.exs"},
+        %{"globPattern" => "**/*.eex"},
+        %{"globPattern" => "**/*.leex"}
       ]
     })
 

apps/language_server/test/diagnostics_test.exs

@@ -0,0 +1,147 @@
+defmodule ElixirLS.LanguageServer.DiagnosticsTest do
+  alias ElixirLS.LanguageServer.Diagnostics
+  use ExUnit.Case
+
+  describe "normalize/2" do
+    test "extract the stacktrace from the message and format it" do
+      root_path = Path.join(__DIR__, "fixtures/build_errors")
+      file = Path.join(root_path, "lib/has_error.ex")
+      position = 2
+
+      message = """
+      ** (CompileError) some message
+
+          Hint: Some hint
+          (elixir 1.10.1) lib/macro.ex:304: Macro.pipe/3
+          (stdlib 3.7.1) lists.erl:1263: :lists.foldl/3
+          (elixir 1.10.1) expanding macro: Kernel.|>/2
+          expanding macro: SomeModule.sigil_L/2
+          lib/my_app/my_module.ex:10: MyApp.MyModule.render/1
+      """
+
+      [diagnostic | _] =
+        [build_diagnostic(message, file, position)]
+        |> Diagnostics.normalize(root_path)
+
+      assert diagnostic.message == """
+             (CompileError) some message
+
+                 Hint: Some hint
+
+             Stacktrace:
+               │ (elixir 1.10.1) lib/macro.ex:304: Macro.pipe/3
+               │ (stdlib 3.7.1) lists.erl:1263: :lists.foldl/3
+               │ (elixir 1.10.1) expanding macro: Kernel.|>/2
+               │ expanding macro: SomeModule.sigil_L/2
+               │ lib/my_app/my_module.ex:10: MyApp.MyModule.render/1\
+             """
+    end
+
+    test "update file and position if file is present in the message" do
+      root_path = Path.join(__DIR__, "fixtures/build_errors")
+      file = Path.join(root_path, "lib/has_error.ex")
+      position = 2
+
+      message = """
+      ** (CompileError) lib/has_error.ex:3: some message
+          lib/my_app/my_module.ex:10: MyApp.MyModule.render/1
+      """
+
+      [diagnostic | _] =
+        [build_diagnostic(message, file, position)]
+        |> Diagnostics.normalize(root_path)
+
+      assert diagnostic.message == """
+             (CompileError) some message
+
+             Stacktrace:
+               │ lib/my_app/my_module.ex:10: MyApp.MyModule.render/1\
+             """
+
+      assert diagnostic.position == 3
+    end
+
+    test "update file and position if file is present in the message (umbrella)" do
+      root_path = Path.join(__DIR__, "fixtures/umbrella")
+      file = Path.join(root_path, "lib/file_to_be_replaced.ex")
+      position = 3
+
+      message = """
+      ** (CompileError) lib/app2.ex:5: some message
+          (elixir 1.10.1) lib/macro.ex:304: Macro.pipe/3
+          lib/my_app/my_module.ex:10: MyApp.MyModule.render/1
+      """
+
+      [diagnostic | _] =
+        [build_diagnostic(message, file, position)]
+        |> Diagnostics.normalize(root_path)
+
+      assert diagnostic.message =~ "(CompileError) some message"
+      assert diagnostic.file =~ "umbrella/apps/app2/lib/app2.ex"
+      assert diagnostic.position == 5
+    end
+
+    test "don't update file nor position if file in message does not exist" do
+      root_path = Path.join(__DIR__, "fixtures/build_errors_on_external_resource")
+      file = Path.join(root_path, "lib/has_error.ex")
+      position = 2
+
+      message = """
+      ** (CompileError) lib/non_existing.ex:3: some message
+          lib/my_app/my_module.ex:10: MyApp.MyModule.render/1
+      """
+
+      [diagnostic | _] =
+        [build_diagnostic(message, file, position)]
+        |> Diagnostics.normalize(root_path)
+
+      assert diagnostic.message == """
+             (CompileError) lib/non_existing.ex:3: some message
+
+             Stacktrace:
+               │ lib/my_app/my_module.ex:10: MyApp.MyModule.render/1\
+             """
+
+      assert diagnostic.position == 2
+    end
+
+    test "if position is nil, try to retrieve it info from the stacktrace" do
+      root_path = Path.join(__DIR__, "fixtures/build_errors")
+      file = Path.join(root_path, "lib/demo_web/router.ex")
+      position = nil
+
+      message = """
+      ** (FunctionClauseError) no function clause matching in Phoenix.Router.Scope.pipeline/2
+
+      The following arguments were given to Phoenix.Router.Scope.pipeline/2:
+
+          # 1
+          DemoWeb.Router
+
+          # 2
+          "api"
+
+          (phoenix 1.5.1) lib/phoenix/router/scope.ex:66: Phoenix.Router.Scope.pipeline/2
+          lib/demo_web/router.ex:13: (module)
+          (stdlib 3.7.1) erl_eval.erl:680: :erl_eval.do_apply/6
+      """
+
+      [diagnostic | _] =
+        [build_diagnostic(message, file, position)]
+        |> Diagnostics.normalize(root_path)
+
+      assert diagnostic.position == 13
+    end
+
+    defp build_diagnostic(message, file, position) do
+      %Mix.Task.Compiler.Diagnostic{
+        compiler_name: "Elixir",
+        details: nil,
+        file: file,
+        message: message,
+        position: position,
+        severity: :error
+      }
+    end
+  end
+end

apps/language_server/test/fixtures/build_errors_on_external_resource/lib/has_error.ex

@@ -0,0 +1,3 @@
+defmodule ElixirLS.LanguageServer.Fixtures.BuildErrorsOnExternalResource.HasError do
+  EEx.compile_file("lib/template.eex", line: 1)
+end

apps/language_server/test/fixtures/build_errors_on_external_resource/lib/template.eex

@@ -0,0 +1,3 @@
+line 1
+<%= , %>
+line 3

apps/language_server/test/fixtures/build_errors_on_external_resource/mix.exs

@@ -0,0 +1,11 @@
+defmodule ElixirLS.LanguageServer.Fixtures.BuildErrorsOnExternalResource.Mixfile do
+  use Mix.Project
+
+  def project do
+    [app: :els_build_errors_test, version: "0.1.0"]
+  end
+
+  def application do
+    []
+  end
+end

apps/language_server/test/server_test.exs

@@ -256,7 +256,7 @@ defmodule ElixirLS.LanguageServer.ServerTest do
                        "diagnostics" => [
                          %{
                            "message" =>
-                             "** (CompileError) lib/has_error.ex:4: undefined function does_not_exist" <>
+                             "(CompileError) undefined function does_not_exist" <>
                                _,
                            "range" => %{"end" => %{"line" => 3}, "start" => %{"line" => 3}},
                            "severity" => 1
@@ -266,6 +266,27 @@ defmodule ElixirLS.LanguageServer.ServerTest do
     end)
   end
 
+  test "reports build diagnostics on external resources", %{server: server} do
+    in_fixture(__DIR__, "build_errors_on_external_resource", fn ->
+      error_file = SourceFile.path_to_uri("lib/template.eex")
+
+      initialize(server)
+
+      assert_receive notification("textDocument/publishDiagnostics", %{
+                       "uri" => ^error_file,
+                       "diagnostics" => [
+                         %{
+                           "message" =>
+                             "(SyntaxError) syntax error before: ','" <>
+                               _,
+                           "range" => %{"end" => %{"line" => 1}, "start" => %{"line" => 1}},
+                           "severity" => 1
+                         }
+                       ]
+                     })
+    end)
+  end
+
   test "reports error if no mixfile", %{server: server} do
     in_fixture(__DIR__, "no_mixfile", fn ->
       mixfile_uri = SourceFile.path_to_uri("mix.exs")