From f51019e53405bb4182f140a3c772858d190c9db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Samson?= Date: Thu, 21 Dec 2023 23:59:16 +0100 Subject: [PATCH] Diagnostics refactor (#1040) * handle uppercase hint * extract line in one more case * wip * wip * build release assets with recent version * fix tests on < 1.16 * format * improve rendering of TokenMissingError for heredocs * fix warning * format * deduplicate rendered messages * try to get line from stacktrace if position unknown * add tests --- .github/workflows/release-asset.yml | 4 +- .../debug_adapter/lib/debug_adapter/server.ex | 25 +- apps/debug_adapter/test/debugger_test.exs | 6 +- .../lib/language_server/build.ex | 75 +- .../lib/language_server/diagnostics.ex | 542 ++++++++---- .../lib/language_server/dialyzer.ex | 4 +- .../lib/language_server/parser.ex | 31 +- .../lib/language_server/server.ex | 28 +- .../language_server/test/diagnostics_test.exs | 819 ++++++++++++++---- apps/language_server/test/dialyzer_test.exs | 1 - 10 files changed, 1133 insertions(+), 402 deletions(-) diff --git a/.github/workflows/release-asset.yml b/.github/workflows/release-asset.yml index f2d61bd46..d146413a1 100644 --- a/.github/workflows/release-asset.yml +++ b/.github/workflows/release-asset.yml @@ -42,8 +42,8 @@ jobs: - name: Set up BEAM uses: erlef/setup-beam@v1 with: - elixir-version: 1.14.x - otp-version: 25.x + elixir-version: 1.15.x + otp-version: 26.x - name: Install dependencies run: mix deps.get diff --git a/apps/debug_adapter/lib/debug_adapter/server.ex b/apps/debug_adapter/lib/debug_adapter/server.ex index 49bfeede9..0eb4a35a1 100644 --- a/apps/debug_adapter/lib/debug_adapter/server.ex +++ b/apps/debug_adapter/lib/debug_adapter/server.ex @@ -1101,15 +1101,16 @@ defmodule ElixirLS.DebugAdapter.Server do "expensive" => false } - args_scope = if frame.args != :undefined do - %{ - "name" => "arguments", - "variablesReference" => args_id, - "namedVariables" => 0, - "indexedVariables" => Enum.count(frame.args), - "expensive" => false - } - end + args_scope = + if frame.args != :undefined do + %{ + "name" => "arguments", + "variablesReference" => args_id, + "namedVariables" => 0, + "indexedVariables" => Enum.count(frame.args), + "expensive" => false + } + end messages_scope = %{ "name" => "messages", @@ -1129,7 +1130,11 @@ defmodule ElixirLS.DebugAdapter.Server do scopes = [vars_scope, versioned_vars_scope, process_info_scope] - |> Kernel.++(if frame.args != :undefined and Enum.count(frame.args) > 0, do: [args_scope], else: []) + |> Kernel.++( + if frame.args != :undefined and Enum.count(frame.args) > 0, + do: [args_scope], + else: [] + ) |> Kernel.++(if Enum.count(frame.messages) > 0, do: [messages_scope], else: []) {state, scopes} diff --git a/apps/debug_adapter/test/debugger_test.exs b/apps/debug_adapter/test/debugger_test.exs index 67c885877..f693d42fc 100644 --- a/apps/debug_adapter/test/debugger_test.exs +++ b/apps/debug_adapter/test/debugger_test.exs @@ -2322,8 +2322,7 @@ defmodule ElixirLS.DebugAdapter.ServerTest do assert Proto in :int.interpreted() assert [ - {{Proto, 2}, - [:active, :enable, :null, {BreakpointCondition, :check_0}]} + {{Proto, 2}, [:active, :enable, :null, {BreakpointCondition, :check_0}]} ] = :int.all_breaks(Proto) assert %{{Proto, :go, 1} => [2]} = :sys.get_state(server).function_breakpoints @@ -2434,8 +2433,7 @@ defmodule ElixirLS.DebugAdapter.ServerTest do assert Proto.List in :int.interpreted() assert [ - {{Proto.List, 7}, - [:active, :enable, :null, {BreakpointCondition, :check_0}]} + {{Proto.List, 7}, [:active, :enable, :null, {BreakpointCondition, :check_0}]} ] = :int.all_breaks(Proto.List) assert %{{Proto.List, :go, 1} => [7]} = :sys.get_state(server).function_breakpoints diff --git a/apps/language_server/lib/language_server/build.ex b/apps/language_server/lib/language_server/build.ex index 77c8124e0..c93e5ca68 100644 --- a/apps/language_server/lib/language_server/build.ex +++ b/apps/language_server/lib/language_server/build.ex @@ -62,7 +62,7 @@ defmodule ElixirLS.LanguageServer.Build do deps_diagnostics = deps_raw_diagnostics - |> Enum.map(&Diagnostics.code_diagnostic/1) + |> Enum.map(&Diagnostics.from_code_diagnostic(&1, mixfile, root_path)) case deps_result do :ok -> @@ -71,7 +71,10 @@ defmodule ElixirLS.LanguageServer.Build do run_mix_compile(Keyword.get(opts, :force?, false)) compile_diagnostics = - Diagnostics.normalize(compile_diagnostics, root_path, mixfile) + compile_diagnostics + |> Enum.map( + &Diagnostics.from_mix_task_compiler_diagnostic(&1, mixfile, root_path) + ) Server.build_finished( parent, @@ -95,7 +98,7 @@ defmodule ElixirLS.LanguageServer.Build do mixfile_diagnostics ++ deps_diagnostics ++ [ - Diagnostics.error_to_diagnostic( + Diagnostics.from_error( kind, err, stacktrace, @@ -270,16 +273,54 @@ defmodule ElixirLS.LanguageServer.Build do # We can get diagnostics if Mixfile fails to load {mixfile_status, mixfile_diagnostics} = - case Kernel.ParallelCompiler.compile([mixfile]) do - {:ok, _, warnings} -> - {:ok, Enum.map(warnings, &Diagnostics.mixfile_diagnostic(&1, :warning))} - - {:error, errors, warnings} -> - { - :error, - Enum.map(warnings, &Diagnostics.mixfile_diagnostic(&1, :warning)) ++ - Enum.map(errors, &Diagnostics.mixfile_diagnostic(&1, :error)) - } + if Version.match?(System.version(), ">= 1.15.3") do + {result, raw_diagnostics} = + with_diagnostics([log: true], fn -> + try do + Code.compile_file(mixfile) + :ok + catch + kind, err -> + {payload, stacktrace} = Exception.blame(kind, err, __STACKTRACE__) + {:error, kind, payload, stacktrace} + end + end) + + diagnostics = + raw_diagnostics + |> Enum.map(&Diagnostics.from_code_diagnostic(&1, mixfile, root_path)) + + case result do + :ok -> + {:ok, diagnostics} + + {:error, kind, err, stacktrace} -> + {:error, + diagnostics ++ + [Diagnostics.from_error(kind, err, stacktrace, mixfile, root_path)]} + end + else + case Kernel.ParallelCompiler.compile([mixfile]) do + {:ok, _, warnings} -> + {:ok, + Enum.map( + warnings, + &Diagnostics.from_kernel_parallel_compiler_tuple(&1, :warning, mixfile) + )} + + {:error, errors, warnings} -> + { + :error, + Enum.map( + warnings, + &Diagnostics.from_kernel_parallel_compiler_tuple(&1, :warning, mixfile) + ) ++ + Enum.map( + errors, + &Diagnostics.from_kernel_parallel_compiler_tuple(&1, :error, mixfile) + ) + } + end end # restore warnings @@ -325,18 +366,18 @@ defmodule ElixirLS.LanguageServer.Build do end end) + config_path = SourceFile.Path.absname(Mix.Project.config()[:config_path], root_path) + config_diagnostics = config_raw_diagnostics - |> Enum.map(&Diagnostics.code_diagnostic/1) + |> Enum.map(&Diagnostics.from_code_diagnostic(&1, config_path, root_path)) case config_result do {:error, kind, err, stacktrace} -> - config_path = Mix.Project.config()[:config_path] - {:error, mixfile_diagnostics ++ config_diagnostics ++ - [Diagnostics.error_to_diagnostic(kind, err, stacktrace, config_path, root_path)]} + [Diagnostics.from_error(kind, err, stacktrace, config_path, root_path)]} :ok -> {:ok, mixfile_diagnostics ++ config_diagnostics} diff --git a/apps/language_server/lib/language_server/diagnostics.ex b/apps/language_server/lib/language_server/diagnostics.ex index 03789b370..db1ff7570 100644 --- a/apps/language_server/lib/language_server/diagnostics.ex +++ b/apps/language_server/lib/language_server/diagnostics.ex @@ -5,23 +5,53 @@ defmodule ElixirLS.LanguageServer.Diagnostics do """ alias ElixirLS.LanguageServer.{SourceFile, JsonRpc} - def normalize(diagnostics, root_path, mixfile) do - for %Mix.Task.Compiler.Diagnostic{} = diagnostic <- diagnostics do - case diagnostic |> dbg do - %Mix.Task.Compiler.Diagnostic{details: payload = %_{line: _}, compiler_name: compiler_name} -> - # remove stacktrace - message = Exception.format_banner(:error, payload) - compiler_name = if compiler_name == "Elixir", do: "ElixirLS", else: compiler_name - %Mix.Task.Compiler.Diagnostic{diagnostic | message: message, compiler_name: compiler_name} + @enforce_keys [:file, :severity, :message, :position, :compiler_name] + defstruct [ + :file, + :severity, + :message, + :position, + :compiler_name, + span: nil, + details: nil, + stacktrace: [] + ] + + def from_mix_task_compiler_diagnostic( + %Mix.Task.Compiler.Diagnostic{} = diagnostic, + mixfile, + root_path + ) do + diagnostic_fields = diagnostic |> Map.from_struct() |> Map.delete(:__struct__) + normalized = struct(__MODULE__, diagnostic_fields) + + if Version.match?(System.version(), ">= 1.16.0-dev") do + # don't include stacktrace in exceptions with position + message = + if diagnostic.file not in [nil, "nofile"] and diagnostic.position != 0 and + is_tuple(diagnostic.details) and tuple_size(diagnostic.details) == 2 do + {kind, reason} = diagnostic.details + Exception.format_banner(kind, reason) + else + diagnostic.message + end - _ -> - {type, file, position, stacktrace} = - extract_message_info(diagnostic.message, root_path) + {file, position} = + get_file_and_position_with_stacktrace_fallback( + {diagnostic.file, diagnostic.position}, + Map.fetch!(diagnostic, :stacktrace), + root_path, + mixfile + ) - diagnostic - |> maybe_update_file(file, mixfile) - |> maybe_update_position(type, position, stacktrace) - end + %__MODULE__{normalized | message: message, file: file, position: position} + else + {type, file, position, stacktrace} = + extract_message_info(diagnostic.message, root_path) + + normalized + |> maybe_update_file(file, mixfile) + |> maybe_update_position(type, position, stacktrace) end end @@ -94,7 +124,7 @@ defmodule ElixirLS.LanguageServer.Diagnostics do end defp get_file_and_position(message, root_path) do - # this regex won't match filenames with spaces but in elixir 1.16 errors we can't be sure where + # this regex won't match filenames with spaces # the file name starts e.g. # invalid syntax found on lib/template.eex:2:5: file_position = @@ -184,100 +214,107 @@ defmodule ElixirLS.LanguageServer.Diagnostics do end) end - def mixfile_diagnostic({file, position, message}, severity) when not is_nil(file) do - %Mix.Task.Compiler.Diagnostic{ - compiler_name: "ElixirLS", - file: file, + def from_kernel_parallel_compiler_tuple({file, position, message}, severity, fallback_file) do + %__MODULE__{ + compiler_name: "Elixir", + file: file || fallback_file, position: position, message: message, severity: severity } end - def code_diagnostic(%{ - file: file, - severity: severity, - message: message, - position: position - }) - when not is_nil(file) do - %Mix.Task.Compiler.Diagnostic{ - compiler_name: "ElixirLS", + def from_code_diagnostic( + %{ + file: file, + severity: severity, + message: message, + position: position, + stacktrace: stacktrace + } = diagnostic, + fallback_file, + root_path + ) do + {file, position} = + get_file_and_position_with_stacktrace_fallback( + {file, position}, + stacktrace, + root_path, + fallback_file + ) + + %__MODULE__{ + compiler_name: "Elixir", file: file, position: position, + # elixir >= 1.16 + span: diagnostic[:span], + # elixir >= 1.16 + details: diagnostic[:details], + stacktrace: stacktrace, message: message, severity: severity } end - def error_to_diagnostic(:error, %_{line: _} = payload, _stacktrace, path, project_dir) do - path = SourceFile.Path.absname(path, project_dir) - message = Exception.format_banner(:error, payload) + def from_error(kind, payload, stacktrace, file, project_dir) do + # assume file is absolute + {position, span} = get_line_span(file, payload, stacktrace, project_dir) - position = case payload do - %{line: line, column: column} -> {line, column} - %{line: line} -> line - end + message = + if position do + # NOTICE get_line_span returns nil position on failure + # known and expected errors have defined position + Exception.format_banner(kind, payload) + else + Exception.format(kind, payload, stacktrace) + end - %Mix.Task.Compiler.Diagnostic{ - compiler_name: "ElixirLS", - file: path, + # try to get position from first matching stacktrace for that file + position = position || get_position_from_stacktrace(stacktrace, file, project_dir) + + %__MODULE__{ + compiler_name: "Elixir", + stacktrace: stacktrace, + file: file, position: position, + span: span, message: message, severity: :error, - details: payload + details: {kind, payload} } end - def error_to_diagnostic(kind, payload, stacktrace, path, project_dir) when not is_nil(path) do - path = SourceFile.Path.absname(path, project_dir) - message = Exception.format(kind, payload, stacktrace) - - line = - stacktrace - |> Enum.find_value(fn {_m, _f, _a, opts} -> - file = opts |> Keyword.get(:file) - - if file != nil and SourceFile.Path.absname(file, project_dir) == path do - opts |> Keyword.get(:line) - end - end) - - %Mix.Task.Compiler.Diagnostic{ - compiler_name: "ElixirLS", - file: path, - # 0 means unknown - position: line || 0, - message: message, - severity: :error, - details: payload - } - end + def from_shutdown_reason(error, fallback_file, root_path) when not is_nil(fallback_file) do + msg = Exception.format_exit(error) - def exception_to_diagnostic(error, path) when not is_nil(path) do - msg = + {{file, position}, stacktrace} = case error do - {:shutdown, 1} -> - "Build failed for unknown reason. See output log." + {_payload, [{_, _, _, info} | _] = stacktrace} when is_list(info) -> + if candidate = get_file_position_from_stacktrace(stacktrace, root_path) do + {candidate, stacktrace} + else + {{fallback_file, 0}, stacktrace} + end _ -> - Exception.format_exit(error) + {{fallback_file, 0}, []} end - %Mix.Task.Compiler.Diagnostic{ - compiler_name: "ElixirLS", - file: path, - # 0 means unknown - position: 0, + %__MODULE__{ + compiler_name: "Elixir", + file: file, + position: position, message: msg, severity: :error, - details: error + stacktrace: stacktrace, + details: {:exit, error} } end def publish_file_diagnostics(uri, uri_diagnostics, source_file, version) do diagnostics_json = - for diagnostic <- uri_diagnostics do + for %__MODULE__{} = diagnostic <- uri_diagnostics do severity = case diagnostic.severity do :error -> 1 @@ -295,13 +332,14 @@ defmodule ElixirLS.LanguageServer.Diagnostics do %{ "message" => message, "severity" => severity, - "range" => range(diagnostic.position, source_file), + "range" => range(normalize_position(diagnostic), source_file), "source" => diagnostic.compiler_name, "relatedInformation" => build_related_information(diagnostic, uri, source_file), "tags" => get_tags(diagnostic) } end |> Enum.sort_by(& &1["range"]["start"]) + |> Enum.dedup() message = %{ "uri" => uri, @@ -319,124 +357,180 @@ defmodule ElixirLS.LanguageServer.Diagnostics do end defp get_tags(diagnostic) do - unused = if Regex.match?(~r/unused|no effect/u, diagnostic.message) do - [1] - else - [] - end - deprecated = if Regex.match?(~r/deprecated/u, diagnostic.message) do - [2] - else - [] - end + unused = + if Regex.match?(~r/unused|no effect/u, diagnostic.message) do + [1] + else + [] + end + + deprecated = + if Regex.match?(~r/deprecated/u, diagnostic.message) do + [2] + else + [] + end unused ++ deprecated end defp get_related_information_description(description, uri, source_file) do - line = case Regex.run( - ~r/line (\d+)/u, - description - ) do - [_, line] -> String.to_integer(line) - _ -> nil - end + line = + case Regex.run( + ~r/line (\d+)/u, + description + ) do + [_, line] -> String.to_integer(line) + _ -> nil + end - message = case String.split(description, "hint: ") do - [_, hint] -> hint - _ -> description - end + message = + case String.split(description, "hint: ") do + [_, hint] -> + hint - if line do - [ - %{ - "location" => %{ - "uri" => uri, - "range" => range(line, source_file) - }, - "message" => message - } - ] - else - [] - end + _ -> + case String.split(description, "HINT: ") do + [_, hint] -> hint + _ -> description + end + end + + if line do + [ + %{ + "location" => %{ + "uri" => uri, + "range" => range(line, source_file) + }, + "message" => message + } + ] + else + [] + end end defp get_related_information_message(message, uri, source_file) do - line = case Regex.run( - ~r/line (\d+)/u, - message - ) do - [_, line] -> String.to_integer(line) - _ -> nil - end + line = + case Regex.run( + ~r/line (\d+)/u, + message + ) do + [_, line] -> + String.to_integer(line) - if line do - [ - %{ - "location" => %{ - "uri" => uri, - "range" => range(line, source_file) - }, - "message" => "related" - } - ] - else - [] - end + _ -> + case Regex.run( + ~r/\.ex\:(\d+)\)/u, + message + ) do + [_, line] -> String.to_integer(line) + _ -> nil + end + end + + if line do + [ + %{ + "location" => %{ + "uri" => uri, + "range" => range(line, source_file) + }, + "message" => "related" + } + ] + else + [] + end end defp build_related_information(diagnostic, uri, source_file) do case diagnostic.details do # for backwards compatibility with elixir < 1.16 - %kind{} = payload when kind == MismatchedDelimiterError -> + {:error, %kind{} = payload} when kind == MismatchedDelimiterError -> [ %{ "location" => %{ "uri" => uri, - "range" => range({payload.line, payload.column - 1, payload.line, payload.column - 1 + String.length(to_string(payload.opening_delimiter))}, source_file) + "range" => + range( + {payload.line, payload.column, payload.line, + payload.column + String.length(to_string(payload.opening_delimiter))}, + source_file + ) }, "message" => "opening delimiter: #{payload.opening_delimiter}" }, %{ "location" => %{ "uri" => uri, - "range" => range({payload.end_line, payload.end_column - 1, payload.end_line, payload.end_column - 1 + String.length(to_string(payload.closing_delimiter))}, source_file) + "range" => + range( + {payload.end_line, payload.end_column, payload.end_line, + payload.end_column + String.length(to_string(payload.closing_delimiter))}, + source_file + ) }, "message" => "closing delimiter: #{payload.closing_delimiter}" } ] - %kind{end_line: end_line, opening_delimiter: opening_delimiter} = payload when kind == TokenMissingError and not is_nil(opening_delimiter) -> - message = String.split(payload.description, "hint: ") |> hd + + {:error, %kind{opening_delimiter: opening_delimiter} = payload} + when kind == TokenMissingError and not is_nil(opening_delimiter) -> [ %{ "location" => %{ "uri" => uri, - "range" => range({payload.line, payload.column - 1, payload.line, payload.column - 1 + String.length(to_string(payload.opening_delimiter))}, source_file) + "range" => + range( + {payload.line, payload.column, payload.line, + payload.column + String.length(to_string(payload.opening_delimiter))}, + source_file + ) }, "message" => "opening delimiter: #{payload.opening_delimiter}" }, %{ "location" => %{ "uri" => uri, - "range" => range(end_line, source_file) + "range" => range({payload.end_line, payload.end_column}, source_file) }, - "message" => message + "message" => "expected delimiter: #{payload.expected_delimiter}" } - ] ++ get_related_information_description(payload.description, uri, source_file) - %{description: description} -> - get_related_information_description(description, uri, source_file) - _ -> [] - end ++ get_related_information_message(diagnostic.message, uri, source_file) + ] + + {:error, %{description: description}} -> + get_related_information_description(description, uri, source_file) ++ + get_related_information_message(diagnostic.message, uri, source_file) + + _ -> + # elixir < 1.16 and other errors on 1.16 + get_related_information_message(diagnostic.message, uri, source_file) + end end - # for details see - # https://hexdocs.pm/mix/1.13.4/Mix.Task.Compiler.Diagnostic.html#t:position/0 - # https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#diagnostic + defp normalize_position(%{ + position: {start_line, start_column}, + span: {end_line, end_column}, + details: %kind{closing_delimiter: closing_delimiter} + }) + when kind == MismatchedDelimiterError do + # convert to pre 1.16 4-tuple + # include mismatched delimiter in range + {start_line, start_column, end_line, end_column + String.length(to_string(closing_delimiter))} + end - # position is a 1 based line number - # 0 means unknown + defp normalize_position(%{position: {start_line, start_column}, span: {end_line, end_column}}) do + # convert to pre 1.16 4-tuple + {start_line, start_column, end_line, end_column} + end + + defp normalize_position(%{position: position}), do: position + + # position is a 1 based line number or 0 if line is unknown # we return a 0 length range at first non whitespace character in line + # or first position in file if line is unknown defp range(line_start, source_file) when is_integer(line_start) and not is_nil(source_file) do # line is 1 based @@ -472,17 +566,17 @@ defmodule ElixirLS.LanguageServer.Diagnostics do } end - # position is a 1 based line number and 0 based character cursor (UTF8) + # position is a 1 based line number and 1 based character cursor (UTF8) # we return a 0 length range exactly at that location defp range({line_start, char_start}, source_file) when not is_nil(source_file) do # some diagnostics are broken - line_start = line_start || 1 - char_start = char_start || 0 + line_start = max(line_start, 1) + char_start = max(char_start, 1) lines = SourceFile.lines(source_file) # elixir_position_to_lsp will handle positions outside file range {line_start_lsp, char_start_lsp} = - SourceFile.elixir_position_to_lsp(lines, {line_start, char_start + 1}) + SourceFile.elixir_position_to_lsp(lines, {line_start, char_start}) %{ "start" => %{ @@ -496,24 +590,24 @@ defmodule ElixirLS.LanguageServer.Diagnostics do } end - # position is a range defined by 1 based line numbers and 0 based character cursors (UTF8) + # position is a range defined by 1 based line numbers and 1 based character cursors (UTF8) # we return exactly that range defp range({line_start, char_start, line_end, char_end}, source_file) when not is_nil(source_file) do # some diagnostics are broken - line_start = line_start || 1 - char_start = char_start || 0 + line_start = max(line_start, 1) + char_start = max(char_start, 1) - line_end = line_end || 1 - char_end = char_end || 0 + line_end = max(line_end, 1) + char_end = max(char_end, 1) lines = SourceFile.lines(source_file) # elixir_position_to_lsp will handle positions outside file range {line_start_lsp, char_start_lsp} = - SourceFile.elixir_position_to_lsp(lines, {line_start, char_start + 1}) + SourceFile.elixir_position_to_lsp(lines, {line_start, char_start}) {line_end_lsp, char_end_lsp} = - SourceFile.elixir_position_to_lsp(lines, {line_end, char_end + 1}) + SourceFile.elixir_position_to_lsp(lines, {line_end, char_end}) %{ "start" => %{ @@ -534,4 +628,132 @@ defmodule ElixirLS.LanguageServer.Diagnostics do # we don't care about utf16 positions here as we send 0 %{"start" => %{"line" => 0, "character" => 0}, "end" => %{"line" => 0, "character" => 0}} end + + # this utility function is copied from elixir source + # TODO colum >= 0 PR + def get_line_span( + _file, + %{line: line, column: column, end_line: end_line, end_column: end_column}, + _stack, + _project_dir + ) + when is_integer(line) and line > 0 and is_integer(column) and column > 0 and + is_integer(end_line) and end_line > 0 and is_integer(end_column) and end_column > 0 do + {{line, column}, {end_line, end_column}} + end + + def get_line_span(_file, %{line: line, column: column}, _stack, _project_dir) + when is_integer(line) and line > 0 and is_integer(column) and column > 0 do + {{line, column}, nil} + end + + def get_line_span(_file, %{line: line}, _stack, _project_dir) + when is_integer(line) and line > 0 do + {line, nil} + end + + def get_line_span(file, :undef, [{_, _, _, []}, {_, _, _, info} | _], project_dir) do + get_line_span_from_stacktrace_info(info, file, project_dir) + end + + # we need that case as exception is normalized + def get_line_span( + file, + %UndefinedFunctionError{}, + [{_, _, _, []}, {_, _, _, info} | _], + project_dir + ) do + get_line_span_from_stacktrace_info(info, file, project_dir) + end + + def get_line_span( + file, + _reason, + [{_, _, _, [file: expanding]}, {_, _, _, info} | _], + project_dir + ) + when expanding in [~c"expanding macro", ~c"expanding struct"] do + get_line_span_from_stacktrace_info(info, file, project_dir) + end + + def get_line_span(file, _reason, [{_, _, _, info} | _], project_dir) do + get_line_span_from_stacktrace_info(info, file, project_dir) + end + + def get_line_span(_, _, _, _) do + {nil, nil} + end + + defp get_line_span_from_stacktrace_info(_info, _file, :no_stacktrace), do: {nil, nil} + + defp get_line_span_from_stacktrace_info(info, file, project_dir) do + info_file = Keyword.get(info, :file) + + if info_file != nil and SourceFile.Path.absname(info_file, project_dir) == file do + {Keyword.get(info, :line), nil} + else + {nil, nil} + end + end + + defp get_file_position_from_stacktrace(_stacktrace, :no_stacktrace), do: nil + + defp get_file_position_from_stacktrace(stacktrace, project_dir) do + Enum.find_value(stacktrace, fn {_, _, _, info} -> + if info_file = Keyword.get(info, :file) do + info_file = SourceFile.Path.absname(info_file, project_dir) + + if SourceFile.Path.path_in_dir?(info_file, project_dir) and + File.exists?(info_file, [:raw]) do + {info_file, Keyword.get(info, :line, 0)} + end + end + end) + end + + defp get_position_from_stacktrace(_stacktrace, _file, :no_stacktrace), do: 0 + + defp get_position_from_stacktrace(stacktrace, file, project_dir) do + Enum.find_value(stacktrace, 0, fn {_, _, _, info} -> + info_file = Keyword.get(info, :file) + + if info_file != nil and SourceFile.Path.absname(info_file, project_dir) == file do + Keyword.get(info, :line) + end + end) + end + + defp get_file_and_position_with_stacktrace_fallback( + {nil, _}, + stacktrace, + root_path, + fallback_file + ) do + # file unknown, try to get first matching project file from stacktrace + if candidate = get_file_position_from_stacktrace(stacktrace, root_path) do + candidate + else + # we have to return something + {fallback_file, 0} + end + end + + defp get_file_and_position_with_stacktrace_fallback( + {file, 0}, + stacktrace, + root_path, + _fallback_file + ) do + # file known but position unknown - try first matching stacktrace entry from that file + {file, get_position_from_stacktrace(stacktrace, file, root_path)} + end + + defp get_file_and_position_with_stacktrace_fallback( + {file, position}, + _stacktrace, + _root_path, + _fallback_file + ) do + {file, position} + end end diff --git a/apps/language_server/lib/language_server/dialyzer.ex b/apps/language_server/lib/language_server/dialyzer.ex index 3c222a328..db2ed5e11 100644 --- a/apps/language_server/lib/language_server/dialyzer.ex +++ b/apps/language_server/lib/language_server/dialyzer.ex @@ -1,5 +1,5 @@ defmodule ElixirLS.LanguageServer.Dialyzer do - alias ElixirLS.LanguageServer.{JsonRpc, Server, SourceFile} + alias ElixirLS.LanguageServer.{JsonRpc, Server, SourceFile, Diagnostics} alias ElixirLS.LanguageServer.Dialyzer.{Manifest, Analyzer, Utils, SuccessTypings} import Utils use GenServer @@ -604,7 +604,7 @@ defmodule ElixirLS.LanguageServer.Dialyzer do source_file = SourceFile.Path.absname(to_string(source_file), project_dir), in_project?(source_file, project_dir), not SourceFile.Path.path_in_dir?(source_file, deps_path) do - %Mix.Task.Compiler.Diagnostic{ + %Diagnostics{ compiler_name: "ElixirLS Dialyzer", file: source_file, position: normalize_postion(position), diff --git a/apps/language_server/lib/language_server/parser.ex b/apps/language_server/lib/language_server/parser.ex index ce3be6799..33fefef74 100644 --- a/apps/language_server/lib/language_server/parser.ex +++ b/apps/language_server/lib/language_server/parser.ex @@ -428,43 +428,22 @@ defmodule ElixirLS.LanguageServer.Parser do {:ok, ast} rescue e in [EEx.SyntaxError, SyntaxError, TokenMissingError, MismatchedDelimiterError] -> - message = Exception.format_banner(:error, e) - - diagnostic = %Mix.Task.Compiler.Diagnostic{ - compiler_name: "ElixirLS", - file: file, - position: {e.line, e.column}, - message: message, - severity: :error, - details: e - } + diagnostic = Diagnostics.from_error(:error, e, __STACKTRACE__, file, :no_stacktrace) {:error, diagnostic} catch kind, err -> - {payload, stacktrace} = Exception.blame(kind, err, __STACKTRACE__) - - message = Exception.format(kind, payload, stacktrace) - - diagnostic = %Mix.Task.Compiler.Diagnostic{ - compiler_name: "ElixirLS", - file: file, - # 0 means unknown - position: 0, - message: message, - severity: :error, - details: payload - } + diagnostic = Diagnostics.from_error(kind, err, __STACKTRACE__, file, :no_stacktrace) # e.g. https://github.com/elixir-lang/elixir/issues/12926 Logger.warning( "Unexpected parser error, please report it to elixir project https://github.com/elixir-lang/elixir/issues\n" <> - message + diagnostic.message ) JsonRpc.telemetry( "parser_error", - %{"elixir_ls.parser_error" => message}, + %{"elixir_ls.parser_error" => diagnostic.message}, %{} ) @@ -475,7 +454,7 @@ defmodule ElixirLS.LanguageServer.Parser do warning_diagnostics = raw_diagnostics |> Enum.map(fn raw -> - Diagnostics.code_diagnostic(raw) + Diagnostics.from_code_diagnostic(raw, file, :no_stacktrace) end) case result do diff --git a/apps/language_server/lib/language_server/server.ex b/apps/language_server/lib/language_server/server.ex index cc2428189..484fe3e2d 100644 --- a/apps/language_server/lib/language_server/server.ex +++ b/apps/language_server/lib/language_server/server.ex @@ -368,13 +368,13 @@ defmodule ElixirLS.LanguageServer.Server do WorkspaceSymbols.notify_build_complete() state - :shutdown -> - WorkspaceSymbols.notify_build_complete() - state + # :shutdown -> + # WorkspaceSymbols.notify_build_complete() + # state - {:shutdown, _} -> - WorkspaceSymbols.notify_build_complete() - state + # {:shutdown, _} -> + # WorkspaceSymbols.notify_build_complete() + # state _ -> message = Exception.format_exit(reason) @@ -386,7 +386,12 @@ defmodule ElixirLS.LanguageServer.Server do ) path = Path.join(state.project_dir, MixfileHelpers.mix_exs()) - handle_build_result(:error, [Diagnostics.exception_to_diagnostic(reason, path)], state) + + handle_build_result( + :error, + [Diagnostics.from_shutdown_reason(reason, path, state.project_dir)], + state + ) end state = @@ -1544,7 +1549,7 @@ defmodule ElixirLS.LanguageServer.Server do uris_from_parser_diagnostics = Map.keys(state.parser_diagnostics) filter_diagnostics_with_known_location = fn - %Mix.Task.Compiler.Diagnostic{file: file} when is_binary(file) -> + %Diagnostics{file: file} when is_binary(file) -> file != "nofile" _ -> @@ -1555,7 +1560,7 @@ defmodule ElixirLS.LanguageServer.Server do (state.build_diagnostics ++ state.dialyzer_diagnostics) |> Enum.filter(filter_diagnostics_with_known_location) |> Enum.group_by(fn - %Mix.Task.Compiler.Diagnostic{file: file} -> SourceFile.Path.to_uri(file, project_dir) + %Diagnostics{file: file} -> SourceFile.Path.to_uri(file, project_dir) end) uris_from_build_and_dialyzer_diagnostics = @@ -1621,9 +1626,10 @@ defmodule ElixirLS.LanguageServer.Server do # document open but was closed when build was triggered # document could have been edited # combine diagnostics - # they can be duplicated but it is not trivial to deduplicate + # they can be duplicated but it is not trivial to deduplicate here + # instead we deduplicate on publish with rendered messages {parser_diagnostics_document_version, - Enum.uniq(parser_diagnostics ++ build_and_dialyzer_diagnostics)} + parser_diagnostics ++ build_and_dialyzer_diagnostics} true -> # document closed diff --git a/apps/language_server/test/diagnostics_test.exs b/apps/language_server/test/diagnostics_test.exs index 8b9295055..0a4d7a528 100644 --- a/apps/language_server/test/diagnostics_test.exs +++ b/apps/language_server/test/diagnostics_test.exs @@ -2,199 +2,680 @@ 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, Path.join(root_path, "mix.exs")) - 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, Path.join(root_path, "mix.exs")) - - assert diagnostic.position == 3 + if Version.match?(System.version(), "< 1.16.0-dev") do + describe "pre 1.16 Mix.Task.Compiler.Diagnostic normalization" 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.from_mix_task_compiler_diagnostic( + Path.join(root_path, "mix.exs"), + root_path + ) + + assert diagnostic.position == 2 + 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.from_mix_task_compiler_diagnostic( + Path.join(root_path, "mix.exs"), + root_path + ) + + assert diagnostic.position == 3 + end + + test "update file and position with column 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:5: some message + lib/my_app/my_module.ex:10: MyApp.MyModule.render/1 + """ + + diagnostic = + build_diagnostic(message, file, position) + |> Diagnostics.from_mix_task_compiler_diagnostic( + Path.join(root_path, "mix.exs"), + root_path + ) + + assert diagnostic.position == {3, 5} + 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.from_mix_task_compiler_diagnostic( + Path.join(root_path, "mix.exs"), + root_path + ) + + 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.from_mix_task_compiler_diagnostic( + Path.join(root_path, "mix.exs"), + root_path + ) + + 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.from_mix_task_compiler_diagnostic( + Path.join(root_path, "mix.exs"), + root_path + ) + + assert diagnostic.position == 13 + end + + test "if position is nil and error is TokenMissingError, try to retrieve from the hint" do + root_path = Path.join(__DIR__, "fixtures/token_missing_error") + file = Path.join(root_path, "lib/has_error.ex") + position = nil + + message = """ + ** (TokenMissingError) lib/has_error.ex:16:1: missing terminator: end (for "do" starting at line 1) + + HINT: it looks like the "do" on line 6 does not have a matching "end" + + (elixir 1.12.1) lib/kernel/parallel_compiler.ex:319: anonymous fn/4 in Kernel.ParallelCompiler.spawn_workers/7 + """ + + diagnostic = + build_diagnostic(message, file, position) + |> Diagnostics.from_mix_task_compiler_diagnostic( + Path.join(root_path, "mix.exs"), + root_path + ) + + assert diagnostic.position == 1 + 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 - test "update file and position if file is present in the message - 1.16 format" do - root_path = Path.join(__DIR__, "fixtures/build_errors_on_external_resource") - file = Path.join(root_path, "lib/has_error.ex") - position = 2 - - message = """ - ** (SyntaxError) invalid syntax found on lib/template.eex:2:5: - error: syntax error before: ',' - │ - 2 │ , - │ ^ - │ - └─ lib/template.eex:2:5 - (eex 1.16.0-rc.0) lib/eex/compiler.ex:332: EEx.Compiler.generate_buffer/4 - lib/has_error.ex:2: (module) - (elixir 1.16.0-rc.0) lib/kernel/parallel_compiler.ex:428: anonymous fn/5 in Kernel.ParallelCompiler.spawn_workers/8 - """ - - [diagnostic | _] = - [build_diagnostic(message, file, position)] - |> Diagnostics.normalize(root_path, Path.join(root_path, "mix.exs")) - - assert diagnostic.position == {2, 5} - assert diagnostic.file == Path.join(root_path, "lib/template.eex") + describe "normalization" do + if Version.match?(System.version(), ">= 1.16.0-dev") do + test "Mix.Task.Compiler.Diagnostic" do + diagnostic = %Mix.Task.Compiler.Diagnostic{ + file: "/somedir/lib/b.ex", + severity: :warning, + message: + "module attribute @bar in code block has no effect as it is never returned (remove the attribute or assign it to _ to avoid warnings)", + position: {20, 5}, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Sample, :bar, 0, [file: "lib/b.ex", column: 5, line: 20]}] + } + + normalized = + Diagnostics.from_mix_task_compiler_diagnostic( + diagnostic, + "/somedir/mix.exs", + "/somedir" + ) + + assert normalized == %Diagnostics{ + file: "/somedir/lib/b.ex", + severity: :warning, + message: + "module attribute @bar in code block has no effect as it is never returned (remove the attribute or assign it to _ to avoid warnings)", + position: {20, 5}, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Sample, :bar, 0, [file: "lib/b.ex", column: 5, line: 20]}] + } + end + + test "Mix.Task.Compiler.Diagnostic without position - get from stacktrace" do + diagnostic = %Mix.Task.Compiler.Diagnostic{ + file: Path.join(File.cwd!(), "temp.ex"), + severity: :warning, + message: + "module attribute @bar in code block has no effect as it is never returned (remove the attribute or assign it to _ to avoid warnings)", + position: 0, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Sample, :bar, 0, [file: "temp.ex", column: 5, line: 20]}] + } + + File.touch("temp.ex") + + normalized = + Diagnostics.from_mix_task_compiler_diagnostic( + diagnostic, + "/somedir/mix.exs", + File.cwd!() + ) + + assert normalized == %Diagnostics{ + file: Path.join(File.cwd!(), "temp.ex"), + severity: :warning, + message: + "module attribute @bar in code block has no effect as it is never returned (remove the attribute or assign it to _ to avoid warnings)", + position: 20, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Sample, :bar, 0, [file: "temp.ex", column: 5, line: 20]}] + } + after + File.rm_rf!("temp.ex") + end + + test "Mix.Task.Compiler.Diagnostic without file and position - get from stacktrace" do + diagnostic = %Mix.Task.Compiler.Diagnostic{ + file: nil, + severity: :warning, + message: + "module attribute @bar in code block has no effect as it is never returned (remove the attribute or assign it to _ to avoid warnings)", + position: 0, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Sample, :bar, 0, [file: "temp.ex", column: 5, line: 20]}] + } + + File.touch("temp.ex") + + normalized = + Diagnostics.from_mix_task_compiler_diagnostic( + diagnostic, + "/somedir/mix.exs", + File.cwd!() + ) + + assert normalized == %Diagnostics{ + file: Path.join(File.cwd!(), "temp.ex"), + severity: :warning, + message: + "module attribute @bar in code block has no effect as it is never returned (remove the attribute or assign it to _ to avoid warnings)", + position: 20, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Sample, :bar, 0, [file: "temp.ex", column: 5, line: 20]}] + } + after + File.rm_rf!("temp.ex") + end + + test "Mix.Task.Compiler.Diagnostic without file and position - when attempt to get it from stacktrace fails fall back to mix.exs" do + diagnostic = %Mix.Task.Compiler.Diagnostic{ + file: nil, + severity: :warning, + message: + "module attribute @bar in code block has no effect as it is never returned (remove the attribute or assign it to _ to avoid warnings)", + position: 0, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Sample, :bar, 0, [file: "lib/b.ex", column: 5, line: 20]}] + } + + normalized = + Diagnostics.from_mix_task_compiler_diagnostic( + diagnostic, + "/somedir/mix.exs", + "/somedir" + ) + + assert normalized == %Diagnostics{ + file: "/somedir/mix.exs", + severity: :warning, + message: + "module attribute @bar in code block has no effect as it is never returned (remove the attribute or assign it to _ to avoid warnings)", + position: 0, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Sample, :bar, 0, [file: "lib/b.ex", column: 5, line: 20]}] + } + end end - test "update file and position with column 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:5: some message - lib/my_app/my_module.ex:10: MyApp.MyModule.render/1 - """ - - [diagnostic | _] = - [build_diagnostic(message, file, position)] - |> Diagnostics.normalize(root_path, Path.join(root_path, "mix.exs")) + test "Code.diagnostic/1" do + diagnostic = %{ + file: "/somedir/mix.exs", + message: + "redefining module Foo (current version loaded from .elixir_ls/build/test/lib/somedir/ebin/Elixir.Foo.beam)", + position: 102, + severity: :warning, + span: nil, + stacktrace: [{Foo, :__MODULE__, 0, [file: "mix.exs", line: 102]}] + } - assert diagnostic.position == {3, 5} + normalized = Diagnostics.from_code_diagnostic(diagnostic, "/somedir/mix.exs", "/somedir") + + assert normalized == %Diagnostics{ + compiler_name: "Elixir", + details: nil, + file: "/somedir/mix.exs", + message: + "redefining module Foo (current version loaded from .elixir_ls/build/test/lib/somedir/ebin/Elixir.Foo.beam)", + position: 102, + severity: :warning, + span: nil, + stacktrace: [{Foo, :__MODULE__, 0, [file: "mix.exs", line: 102]}] + } 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, Path.join(root_path, "mix.exs")) + test "Code.diagnostic/1 without position - get from stacktrace" do + diagnostic = %{ + file: Path.join(File.cwd!(), "temp.ex"), + message: + "redefining module Foo (current version loaded from .elixir_ls/build/test/lib/somedir/ebin/Elixir.Foo.beam)", + position: 0, + severity: :warning, + span: nil, + stacktrace: [{Foo, :__MODULE__, 0, [file: "temp.ex", line: 102]}] + } - assert diagnostic.file =~ "umbrella/apps/app2/lib/app2.ex" - assert diagnostic.position == 5 + File.touch("temp.ex") + normalized = Diagnostics.from_code_diagnostic(diagnostic, "/somedir/mix.exs", File.cwd!()) + + assert normalized == %Diagnostics{ + file: Path.join(File.cwd!(), "temp.ex"), + severity: :warning, + message: + "redefining module Foo (current version loaded from .elixir_ls/build/test/lib/somedir/ebin/Elixir.Foo.beam)", + position: 102, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Foo, :__MODULE__, 0, [file: "temp.ex", line: 102]}] + } + after + File.rm_rf!("temp.ex") 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, Path.join(root_path, "mix.exs")) + test "Code.diagnostic/1 without file and position - get from stacktrace" do + diagnostic = %{ + file: nil, + message: + "redefining module Foo (current version loaded from .elixir_ls/build/test/lib/somedir/ebin/Elixir.Foo.beam)", + position: 0, + severity: :warning, + span: nil, + stacktrace: [{Foo, :__MODULE__, 0, [file: "temp.ex", line: 102]}] + } - assert diagnostic.position == 2 + File.touch("temp.ex") + normalized = Diagnostics.from_code_diagnostic(diagnostic, "/somedir/mix.exs", File.cwd!()) + + assert normalized == %Diagnostics{ + file: Path.join(File.cwd!(), "temp.ex"), + severity: :warning, + message: + "redefining module Foo (current version loaded from .elixir_ls/build/test/lib/somedir/ebin/Elixir.Foo.beam)", + position: 102, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [{Foo, :__MODULE__, 0, [file: "temp.ex", line: 102]}] + } + after + File.rm_rf!("temp.ex") 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, Path.join(root_path, "mix.exs")) + test "Code.diagnostic/1 without file and position - when attempt to get it from stacktrace fails fall back to provided file" do + diagnostic = %{ + file: nil, + message: + "redefining module Foo (current version loaded from .elixir_ls/build/test/lib/somedir/ebin/Elixir.Foo.beam)", + position: 0, + severity: :warning, + span: nil, + stacktrace: [{Foo, :__MODULE__, 0, [file: "mix.exs", line: 102]}] + } - assert diagnostic.position == 13 + normalized = Diagnostics.from_code_diagnostic(diagnostic, "/somedir/mix.exs", "/somedir") + + assert normalized == %Diagnostics{ + compiler_name: "Elixir", + details: nil, + file: "/somedir/mix.exs", + message: + "redefining module Foo (current version loaded from .elixir_ls/build/test/lib/somedir/ebin/Elixir.Foo.beam)", + position: 0, + severity: :warning, + span: nil, + stacktrace: [{Foo, :__MODULE__, 0, [file: "mix.exs", line: 102]}] + } end - test "if position is nil and error is TokenMissingError, try to retrieve from the hint" do - root_path = Path.join(__DIR__, "fixtures/token_missing_error") - file = Path.join(root_path, "lib/has_error.ex") - position = nil - - message = """ - ** (TokenMissingError) lib/has_error.ex:16:1: missing terminator: end (for "do" starting at line 1) - - HINT: it looks like the "do" on line 6 does not have a matching "end" - - (elixir 1.12.1) lib/kernel/parallel_compiler.ex:319: anonymous fn/4 in Kernel.ParallelCompiler.spawn_workers/7 - """ - - [diagnostic | _] = - [build_diagnostic(message, file, position)] - |> Diagnostics.normalize(root_path, Path.join(root_path, "mix.exs")) + if Version.match?(System.version(), ">= 1.16.0-dev") do + test "exception" do + payload = %TokenMissingError{ + file: "/somedir/lib/b.ex", + line: 3, + column: 13, + end_line: 39, + end_column: 1, + line_offset: 0, + snippet: + "# throw :foo\n# exit(:bar)\ndefmodule A do\n def a() do\n raise \"asd\"\n # end\nend\n# A.a()\n# IO.warn(\"dfg\", __ENV__)\n# \"oops\"\n# :ok\n# asdfsf = 2\n# :ok\n# defmodule Sample do\n# @foo 1\n# @bar 1\n# @foo\n\n# def bar do\n# @bar\n# :ok\n# end\n# end\n# exit({:shutdown, 1})\n# throw :asd\n# exit(:dupa)\n# IO.warn(\"asd\")\ndefmodule Foo do\n @after_verify __MODULE__\n\n def __after_verify__(_) do\n # raise \"what\"\n # throw :asd\n # exit(:qw)\n # exit({:shutdown, 1})\n # IO.warn(\"asd\")\n end\nend\n", + opening_delimiter: :do, + expected_delimiter: :end, + description: + "missing terminator: end\nhint: it looks like the \"do\" on line 3 does not have a matching \"end\"" + } + + normalized = Diagnostics.from_error(:error, payload, [], "/somedir/mix.exs", "/somedir") + + assert %ElixirLS.LanguageServer.Diagnostics{ + file: "/somedir/mix.exs", + severity: :error, + position: {3, 13}, + compiler_name: "Elixir", + span: {39, 1}, + details: {:error, %TokenMissingError{}}, + stacktrace: [] + } = normalized + end + + test "exception without position - fall back to stacktrace" do + payload = %CompileError{ + file: "/somedir/lib/b.ex", + line: nil + } + + normalized = + Diagnostics.from_error( + :error, + payload, + [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"mix.exs", line: 99]}], + "/somedir/mix.exs", + "/somedir" + ) + + assert %ElixirLS.LanguageServer.Diagnostics{ + file: "/somedir/mix.exs", + severity: :error, + position: 99, + compiler_name: "Elixir", + span: nil, + details: {:error, %CompileError{}}, + stacktrace: [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"mix.exs", line: 99]}] + } = normalized + end + + test "exception without file and position - fall back to stacktrace" do + payload = %RuntimeError{message: "foo"} + + normalized = + Diagnostics.from_error( + :error, + payload, + [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"mix.exs", line: 99]}], + "/somedir/mix.exs", + "/somedir" + ) + + assert %ElixirLS.LanguageServer.Diagnostics{ + file: "/somedir/mix.exs", + severity: :error, + position: 99, + compiler_name: "Elixir", + span: nil, + details: {:error, %RuntimeError{__exception__: true, message: "foo"}}, + stacktrace: [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"mix.exs", line: 99]}] + } = normalized + end + + test "exception without file and position - when attempt to get it from stacktrace fails fall back to provided file" do + payload = %RuntimeError{message: "foo"} + + normalized = Diagnostics.from_error(:error, payload, [], "/somedir/mix.exs", "/somedir") + + assert %ElixirLS.LanguageServer.Diagnostics{ + file: "/somedir/mix.exs", + severity: :error, + position: 0, + compiler_name: "Elixir", + span: nil, + details: {:error, %RuntimeError{__exception__: true, message: "foo"}}, + stacktrace: [] + } = normalized + end + end - assert diagnostic.position == 1 + test "throw" do + payload = :asd + + normalized = + Diagnostics.from_error( + :throw, + payload, + [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"mix.exs", line: 99]}], + "/somedir/mix.exs", + "/somedir" + ) + + assert %ElixirLS.LanguageServer.Diagnostics{ + compiler_name: "Elixir", + details: {:throw, :asd}, + file: "/somedir/mix.exs", + position: 99, + severity: :error, + span: nil, + stacktrace: [ + {:elixir_compiler_2, :__FILE__, 1, [file: ~c"mix.exs", line: 99]} + ], + message: "** (throw) :asd" + } = normalized end - test "if position is nil and error is TokenMissingError, try to retrieve from the hint - 1.16 format" do - root_path = Path.join(__DIR__, "fixtures/token_missing_error") - file = Path.join(root_path, "lib/has_error.ex") - position = nil + test "exit" do + payload = :asd + + normalized = + Diagnostics.from_error( + :exit, + payload, + [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"mix.exs", line: 99]}], + "/somedir/mix.exs", + "/somedir" + ) + + assert %ElixirLS.LanguageServer.Diagnostics{ + compiler_name: "Elixir", + details: {:exit, :asd}, + file: "/somedir/mix.exs", + position: 99, + severity: :error, + span: nil, + stacktrace: [ + {:elixir_compiler_2, :__FILE__, 1, [file: ~c"mix.exs", line: 99]} + ], + message: "** (exit) :asd" + } = normalized + end - message = """ - ** (TokenMissingError) token missing on lib/has_error.ex:16:1: - error: missing terminator: end (for "fn" starting at line 6) - └─ lib/has_error.ex:16:1 - """ + test "shutdown reason" do + payload = :asd + + normalized = Diagnostics.from_shutdown_reason(payload, "/somedir/mix.exs", "/somedir") + + assert %ElixirLS.LanguageServer.Diagnostics{ + compiler_name: "Elixir", + details: {:exit, :asd}, + file: "/somedir/mix.exs", + position: 0, + severity: :error, + span: nil, + stacktrace: [], + message: ":asd" + } = normalized + end - [diagnostic | _] = - [build_diagnostic(message, file, position)] - |> Diagnostics.normalize(root_path, Path.join(root_path, "mix.exs")) + test "shutdown reason with exception stacktrace" do + payload = + {%RuntimeError{message: "foo"}, + [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"temp.ex", line: 99]}]} + + File.touch("temp.ex") + normalized = Diagnostics.from_shutdown_reason(payload, "/somedir/mix.exs", File.cwd!()) + + assert %ElixirLS.LanguageServer.Diagnostics{ + compiler_name: "Elixir", + details: + {:exit, + {%RuntimeError{message: "foo"}, + [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"temp.ex", line: 99]}]}}, + file: file, + position: 99, + severity: :error, + span: nil, + stacktrace: [ + {:elixir_compiler_2, :__FILE__, 1, [file: ~c"temp.ex", line: 99]} + ], + message: + "an exception was raised:\n ** (RuntimeError) foo\n temp.ex:99: (file)" + } = normalized + + assert file == Path.join(File.cwd!(), "temp.ex") + after + File.rm_rf!("temp.ex") + end - assert diagnostic.position == 6 + test "shutdown reason with exception stacktrace - when attempt to get it from stacktrace fails fall back to provided file" do + payload = + {%RuntimeError{message: "foo"}, + [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"/abc/temp.ex", line: 99]}]} + + File.touch("temp.ex") + normalized = Diagnostics.from_shutdown_reason(payload, "/somedir/mix.exs", File.cwd!()) + + assert %ElixirLS.LanguageServer.Diagnostics{ + compiler_name: "Elixir", + details: + {:exit, + {%RuntimeError{message: "foo"}, + [{:elixir_compiler_2, :__FILE__, 1, [file: ~c"/abc/temp.ex", line: 99]}]}}, + file: file, + position: 0, + severity: :error, + span: nil, + stacktrace: [ + {:elixir_compiler_2, :__FILE__, 1, [file: ~c"/abc/temp.ex", line: 99]} + ], + message: + "an exception was raised:\n ** (RuntimeError) foo\n /abc/temp.ex:99: (file)" + } = normalized + + assert file == "/somedir/mix.exs" + after + File.rm_rf!("temp.ex") 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 - } + test "Kernel.ParallelCompiler.compile/1 tuple" do + payload = + {"/somedir/mix.exs", 74, + "** (RuntimeError) asd\n mix.exs:74: (file)\n (elixir 1.16.0-rc.1) lib/kernel/parallel_compiler.ex:429: anonymous fn/5 in Kernel.ParallelCompiler.spawn_workers/8\n"} + + normalized = + Diagnostics.from_kernel_parallel_compiler_tuple(payload, :error, "/somedir/mix.exs") + + assert normalized == %Diagnostics{ + file: "/somedir/mix.exs", + severity: :error, + message: + "** (RuntimeError) asd\n mix.exs:74: (file)\n (elixir 1.16.0-rc.1) lib/kernel/parallel_compiler.ex:429: anonymous fn/5 in Kernel.ParallelCompiler.spawn_workers/8\n", + position: 74, + compiler_name: "Elixir", + span: nil, + details: nil, + stacktrace: [] + } end end end diff --git a/apps/language_server/test/dialyzer_test.exs b/apps/language_server/test/dialyzer_test.exs index 461dff627..45194f295 100644 --- a/apps/language_server/test/dialyzer_test.exs +++ b/apps/language_server/test/dialyzer_test.exs @@ -13,7 +13,6 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do Parser } - import ExUnit.CaptureLog import ElixirLS.LanguageServer.Test.ServerTestHelpers use ElixirLS.Utils.MixTest.Case, async: false use Protocol