diff --git a/apps/language_server/lib/language_server/build.ex b/apps/language_server/lib/language_server/build.ex index bcb73916c..1901bce9e 100644 --- a/apps/language_server/lib/language_server/build.ex +++ b/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} -> diff --git a/apps/language_server/lib/language_server/diagnostics.ex b/apps/language_server/lib/language_server/diagnostics.ex new file mode 100644 index 000000000..4a10c3bee --- /dev/null +++ b/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 diff --git a/apps/language_server/lib/language_server/server.ex b/apps/language_server/lib/language_server/server.ex index 0cfa6a288..f20724875 100644 --- a/apps/language_server/lib/language_server/server.ex +++ b/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"} ] }) diff --git a/apps/language_server/test/diagnostics_test.exs b/apps/language_server/test/diagnostics_test.exs new file mode 100644 index 000000000..d8aac1c28 --- /dev/null +++ b/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 diff --git a/apps/language_server/test/fixtures/build_errors_on_external_resource/lib/has_error.ex b/apps/language_server/test/fixtures/build_errors_on_external_resource/lib/has_error.ex new file mode 100644 index 000000000..fbd93fc9f --- /dev/null +++ b/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 diff --git a/apps/language_server/test/fixtures/build_errors_on_external_resource/lib/template.eex b/apps/language_server/test/fixtures/build_errors_on_external_resource/lib/template.eex new file mode 100644 index 000000000..e1b537028 --- /dev/null +++ b/apps/language_server/test/fixtures/build_errors_on_external_resource/lib/template.eex @@ -0,0 +1,3 @@ +line 1 +<%= , %> +line 3 \ No newline at end of file diff --git a/apps/language_server/test/fixtures/build_errors_on_external_resource/mix.exs b/apps/language_server/test/fixtures/build_errors_on_external_resource/mix.exs new file mode 100644 index 000000000..4e4d39da1 --- /dev/null +++ b/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 diff --git a/apps/language_server/test/server_test.exs b/apps/language_server/test/server_test.exs index 529c9180d..449dff6b5 100644 --- a/apps/language_server/test/server_test.exs +++ b/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")