Skip to content

Commit 5fbf589

Browse files
msaraivamsaraiva
authored
Normalize diagnostic messages (#241)
1 parent c9b095f commit 5fbf589

File tree

8 files changed

+342
-3
lines changed

8 files changed

+342
-3
lines changed

‎apps/language_server/lib/language_server/build.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
defmodule ElixirLS.LanguageServer.Build do
2-
alias ElixirLS.LanguageServer.{Server, JsonRpc, SourceFile}
2+
alias ElixirLS.LanguageServer.{Server, JsonRpc, SourceFile, Diagnostics}
33

44
def build(parent, root_path, fetch_deps?) do
55
if Path.absname(File.cwd!()) != Path.absname(root_path) do
@@ -22,6 +22,7 @@ defmodule ElixirLS.LanguageServer.Build do
2222
do: fetch_deps()
2323

2424
{status, diagnostics} = compile()
25+
diagnostics = Diagnostics.normalize(diagnostics, root_path)
2526
Server.build_finished(parent, {status, mixfile_diagnostics ++ diagnostics})
2627

2728
{:error, mixfile_diagnostics} ->

‎apps/language_server/lib/language_server/diagnostics.ex

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
defmodule ElixirLS.LanguageServer.Diagnostics do
2+
def normalize(diagnostics, root_path) do
3+
for diagnostic <- diagnostics do
4+
{type, file, line, description, stacktrace} =
5+
extract_message_info(diagnostic.message, root_path)
6+
7+
diagnostic
8+
|> update_message(type, description, stacktrace)
9+
|> maybe_update_file(file)
10+
|> maybe_update_position(line, stacktrace)
11+
end
12+
end
13+
14+
defp extract_message_info(list, root_path) when is_list(list) do
15+
list
16+
|> Enum.join()
17+
|> extract_message_info(root_path)
18+
end
19+
20+
defp extract_message_info(diagnostic_message, root_path) do
21+
{reversed_stacktrace, reversed_description} =
22+
diagnostic_message
23+
|> String.trim_trailing()
24+
|> String.split("\n")
25+
|> Enum.reverse()
26+
|> Enum.split_while(&is_stack?/1)
27+
28+
message = reversed_description |> Enum.reverse() |> Enum.join("\n") |> String.trim()
29+
stacktrace = reversed_stacktrace |> Enum.map(&String.trim/1) |> Enum.reverse()
30+
31+
{type, message_without_type} = split_type_and_message(message)
32+
{file, line, description} = split_file_and_description(message_without_type, root_path)
33+
34+
{type, file, line, description, stacktrace}
35+
end
36+
37+
defp update_message(diagnostic, type, description, stacktrace) do
38+
description =
39+
if type do
40+
"(#{type}) #{description}"
41+
else
42+
description
43+
end
44+
45+
message =
46+
if stacktrace != [] do
47+
stacktrace =
48+
stacktrace
49+
|> Enum.map(&" │ #{&1}")
50+
|> Enum.join("\n")
51+
|> String.trim_trailing()
52+
53+
description <> "\n\n" <> "Stacktrace:\n" <> stacktrace
54+
else
55+
description
56+
end
57+
58+
Map.put(diagnostic, :message, message)
59+
end
60+
61+
defp maybe_update_file(diagnostic, path) do
62+
if path do
63+
Map.put(diagnostic, :file, path)
64+
else
65+
diagnostic
66+
end
67+
end
68+
69+
defp maybe_update_position(diagnostic, line, stacktrace) do
70+
cond do
71+
line ->
72+
%{diagnostic | position: line}
73+
74+
diagnostic.position ->
75+
diagnostic
76+
77+
true ->
78+
line = extract_line_from_stacktrace(diagnostic.file, stacktrace)
79+
%{diagnostic | position: line}
80+
end
81+
end
82+
83+
defp split_type_and_message(message) do
84+
case Regex.run(~r/^\*\* \(([\w\.]+?)?\) (.*)/s, message) do
85+
[_, type, rest] ->
86+
{type, rest}
87+
88+
_ ->
89+
{nil, message}
90+
end
91+
end
92+
93+
defp split_file_and_description(message, root_path) do
94+
with [_, file, line, description] <- Regex.run(~r/^(.*?):(\d+): (.*)/s, message),
95+
{:ok, path} <- file_path(file, root_path) do
96+
{path, String.to_integer(line), description}
97+
else
98+
_ ->
99+
{nil, nil, message}
100+
end
101+
end
102+
103+
defp file_path(nil, _root_path) do
104+
{:error, :file_not_found}
105+
end
106+
107+
defp file_path(file, root_path) do
108+
path = Path.join([root_path, file])
109+
110+
if File.exists?(path) do
111+
{:ok, path}
112+
else
113+
file_path_in_umbrella(file, root_path)
114+
end
115+
end
116+
117+
defp file_path_in_umbrella(file, root_path) do
118+
case [root_path, "apps", "*", file] |> Path.join() |> Path.wildcard() do
119+
[] ->
120+
{:error, :file_not_found}
121+
122+
[path] ->
123+
{:ok, path}
124+
125+
_ ->
126+
{:error, :more_than_one_file_found}
127+
end
128+
end
129+
130+
defp is_stack?(" " <> str) do
131+
Regex.match?(~r/.*\.(ex|erl):\d+: /, str) ||
132+
Regex.match?(~r/.*expanding macro: /, str)
133+
end
134+
135+
defp is_stack?(_) do
136+
false
137+
end
138+
139+
defp extract_line_from_stacktrace(file, stacktrace) do
140+
Enum.find_value(stacktrace, fn stack_item ->
141+
with [_, _, file_relative, line] <-
142+
Regex.run(~r/(\(.+?\)\s+)?(.*\.ex):(\d+): /, stack_item),
143+
true <- String.ends_with?(file, file_relative) do
144+
String.to_integer(line)
145+
else
146+
_ ->
147+
nil
148+
end
149+
end)
150+
end
151+
end

‎apps/language_server/lib/language_server/server.ex

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,9 @@ defmodule ElixirLS.LanguageServer.Server do
143143
JsonRpc.register_capability_request("workspace/didChangeWatchedFiles", %{
144144
"watchers" => [
145145
%{"globPattern" => "**/*.ex"},
146-
%{"globPattern" => "**/*.exs"}
146+
%{"globPattern" => "**/*.exs"},
147+
%{"globPattern" => "**/*.eex"},
148+
%{"globPattern" => "**/*.leex"}
147149
]
148150
})
149151

‎apps/language_server/test/diagnostics_test.exs

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
defmodule ElixirLS.LanguageServer.DiagnosticsTest do
2+
alias ElixirLS.LanguageServer.Diagnostics
3+
use ExUnit.Case
4+
5+
describe "normalize/2" do
6+
test "extract the stacktrace from the message and format it" do
7+
root_path = Path.join(__DIR__, "fixtures/build_errors")
8+
file = Path.join(root_path, "lib/has_error.ex")
9+
position = 2
10+
11+
message = """
12+
** (CompileError) some message
13+
14+
Hint: Some hint
15+
(elixir 1.10.1) lib/macro.ex:304: Macro.pipe/3
16+
(stdlib 3.7.1) lists.erl:1263: :lists.foldl/3
17+
(elixir 1.10.1) expanding macro: Kernel.|>/2
18+
expanding macro: SomeModule.sigil_L/2
19+
lib/my_app/my_module.ex:10: MyApp.MyModule.render/1
20+
"""
21+
22+
[diagnostic | _] =
23+
[build_diagnostic(message, file, position)]
24+
|> Diagnostics.normalize(root_path)
25+
26+
assert diagnostic.message == """
27+
(CompileError) some message
28+
29+
Hint: Some hint
30+
31+
Stacktrace:
32+
│ (elixir 1.10.1) lib/macro.ex:304: Macro.pipe/3
33+
│ (stdlib 3.7.1) lists.erl:1263: :lists.foldl/3
34+
│ (elixir 1.10.1) expanding macro: Kernel.|>/2
35+
│ expanding macro: SomeModule.sigil_L/2
36+
│ lib/my_app/my_module.ex:10: MyApp.MyModule.render/1\
37+
"""
38+
end
39+
40+
test "update file and position if file is present in the message" do
41+
root_path = Path.join(__DIR__, "fixtures/build_errors")
42+
file = Path.join(root_path, "lib/has_error.ex")
43+
position = 2
44+
45+
message = """
46+
** (CompileError) lib/has_error.ex:3: some message
47+
lib/my_app/my_module.ex:10: MyApp.MyModule.render/1
48+
"""
49+
50+
[diagnostic | _] =
51+
[build_diagnostic(message, file, position)]
52+
|> Diagnostics.normalize(root_path)
53+
54+
assert diagnostic.message == """
55+
(CompileError) some message
56+
57+
Stacktrace:
58+
│ lib/my_app/my_module.ex:10: MyApp.MyModule.render/1\
59+
"""
60+
61+
assert diagnostic.position == 3
62+
end
63+
64+
test "update file and position if file is present in the message (umbrella)" do
65+
root_path = Path.join(__DIR__, "fixtures/umbrella")
66+
file = Path.join(root_path, "lib/file_to_be_replaced.ex")
67+
position = 3
68+
69+
message = """
70+
** (CompileError) lib/app2.ex:5: some message
71+
(elixir 1.10.1) lib/macro.ex:304: Macro.pipe/3
72+
lib/my_app/my_module.ex:10: MyApp.MyModule.render/1
73+
"""
74+
75+
[diagnostic | _] =
76+
[build_diagnostic(message, file, position)]
77+
|> Diagnostics.normalize(root_path)
78+
79+
assert diagnostic.message =~ "(CompileError) some message"
80+
assert diagnostic.file =~ "umbrella/apps/app2/lib/app2.ex"
81+
assert diagnostic.position == 5
82+
end
83+
84+
test "don't update file nor position if file in message does not exist" do
85+
root_path = Path.join(__DIR__, "fixtures/build_errors_on_external_resource")
86+
file = Path.join(root_path, "lib/has_error.ex")
87+
position = 2
88+
89+
message = """
90+
** (CompileError) lib/non_existing.ex:3: some message
91+
lib/my_app/my_module.ex:10: MyApp.MyModule.render/1
92+
"""
93+
94+
[diagnostic | _] =
95+
[build_diagnostic(message, file, position)]
96+
|> Diagnostics.normalize(root_path)
97+
98+
assert diagnostic.message == """
99+
(CompileError) lib/non_existing.ex:3: some message
100+
101+
Stacktrace:
102+
│ lib/my_app/my_module.ex:10: MyApp.MyModule.render/1\
103+
"""
104+
105+
assert diagnostic.position == 2
106+
end
107+
108+
test "if position is nil, try to retrieve it info from the stacktrace" do
109+
root_path = Path.join(__DIR__, "fixtures/build_errors")
110+
file = Path.join(root_path, "lib/demo_web/router.ex")
111+
position = nil
112+
113+
message = """
114+
** (FunctionClauseError) no function clause matching in Phoenix.Router.Scope.pipeline/2
115+
116+
The following arguments were given to Phoenix.Router.Scope.pipeline/2:
117+
118+
# 1
119+
DemoWeb.Router
120+
121+
# 2
122+
"api"
123+
124+
(phoenix 1.5.1) lib/phoenix/router/scope.ex:66: Phoenix.Router.Scope.pipeline/2
125+
lib/demo_web/router.ex:13: (module)
126+
(stdlib 3.7.1) erl_eval.erl:680: :erl_eval.do_apply/6
127+
"""
128+
129+
[diagnostic | _] =
130+
[build_diagnostic(message, file, position)]
131+
|> Diagnostics.normalize(root_path)
132+
133+
assert diagnostic.position == 13
134+
end
135+
136+
defp build_diagnostic(message, file, position) do
137+
%Mix.Task.Compiler.Diagnostic{
138+
compiler_name: "Elixir",
139+
details: nil,
140+
file: file,
141+
message: message,
142+
position: position,
143+
severity: :error
144+
}
145+
end
146+
end
147+
end

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
defmodule ElixirLS.LanguageServer.Fixtures.BuildErrorsOnExternalResource.HasError do
2+
EEx.compile_file("lib/template.eex", line: 1)
3+
end

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
line 1
2+
<%= , %>
3+
line 3

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
defmodule ElixirLS.LanguageServer.Fixtures.BuildErrorsOnExternalResource.Mixfile do
2+
use Mix.Project
3+
4+
def project do
5+
[app: :els_build_errors_test, version: "0.1.0"]
6+
end
7+
8+
def application do
9+
[]
10+
end
11+
end

‎apps/language_server/test/server_test.exs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ defmodule ElixirLS.LanguageServer.ServerTest do
256256
"diagnostics" => [
257257
%{
258258
"message" =>
259-
"** (CompileError) lib/has_error.ex:4: undefined function does_not_exist" <>
259+
"(CompileError) undefined function does_not_exist" <>
260260
_,
261261
"range" => %{"end" => %{"line" => 3}, "start" => %{"line" => 3}},
262262
"severity" => 1
@@ -266,6 +266,27 @@ defmodule ElixirLS.LanguageServer.ServerTest do
266266
end)
267267
end
268268

269+
test "reports build diagnostics on external resources", %{server: server} do
270+
in_fixture(__DIR__, "build_errors_on_external_resource", fn ->
271+
error_file = SourceFile.path_to_uri("lib/template.eex")
272+
273+
initialize(server)
274+
275+
assert_receive notification("textDocument/publishDiagnostics", %{
276+
"uri" => ^error_file,
277+
"diagnostics" => [
278+
%{
279+
"message" =>
280+
"(SyntaxError) syntax error before: ','" <>
281+
_,
282+
"range" => %{"end" => %{"line" => 1}, "start" => %{"line" => 1}},
283+
"severity" => 1
284+
}
285+
]
286+
})
287+
end)
288+
end
289+
269290
test "reports error if no mixfile", %{server: server} do
270291
in_fixture(__DIR__, "no_mixfile", fn ->
271292
mixfile_uri = SourceFile.path_to_uri("mix.exs")

0 commit comments

Comments
 (0)