From 6e775f372eaef603d1e41ea5bc608b9fa1f4cadd Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Fri, 29 Dec 2023 10:07:50 +0100 Subject: [PATCH] fix crash when apply spec fallback fails to return a valid line do not emit telemetry in that case --- .../dialyzer/success_typings.ex | 2 +- .../language_server/providers/completion.ex | 2 +- .../providers/execute_command/apply_spec.ex | 101 ++++++++++-------- .../providers/workspace_symbols.ex | 14 ++- 4 files changed, 66 insertions(+), 53 deletions(-) diff --git a/apps/language_server/lib/language_server/dialyzer/success_typings.ex b/apps/language_server/lib/language_server/dialyzer/success_typings.ex index 81e4825c2..a040b8313 100644 --- a/apps/language_server/lib/language_server/dialyzer/success_typings.ex +++ b/apps/language_server/lib/language_server/dialyzer/success_typings.ex @@ -14,7 +14,7 @@ defmodule ElixirLS.LanguageServer.Dialyzer.SuccessTypings do :dialyzer_plt.lookup_contract(plt, mfa) == :none, {stripped_fun, stripped_arity} = SourceFile.strip_macro_prefix({fun, arity}), line = SourceFile.function_line(mod, stripped_fun, stripped_arity), - is_integer(line), + is_integer(line) and line > 0, do: {file, line, {mod, stripped_fun, stripped_arity}, success_typing, stripped_fun != fun} end diff --git a/apps/language_server/lib/language_server/providers/completion.ex b/apps/language_server/lib/language_server/providers/completion.ex index 6df2e7bef..58485b515 100644 --- a/apps/language_server/lib/language_server/providers/completion.ex +++ b/apps/language_server/lib/language_server/providers/completion.ex @@ -1380,7 +1380,7 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do defp sanitize_function_name(name, origin) when origin in ["Kernel", "Kernel.SpecialForms"], do: name - defp sanitize_function_name(name, origin) do + defp sanitize_function_name(name, _origin) do if not Regex.match?(~r/^([_\p{Ll}\p{Lo}][\p{L}\p{N}_]*[?!]?)$/u, name) do # not an allowed identifier - quote escaped = diff --git a/apps/language_server/lib/language_server/providers/execute_command/apply_spec.ex b/apps/language_server/lib/language_server/providers/execute_command/apply_spec.ex index 1ef581f99..bd42753ab 100644 --- a/apps/language_server/lib/language_server/providers/execute_command/apply_spec.ex +++ b/apps/language_server/lib/language_server/providers/execute_command/apply_spec.ex @@ -7,6 +7,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.ApplySpec do alias ElixirLS.LanguageServer.{JsonRpc, SourceFile} import ElixirLS.LanguageServer.Protocol alias ElixirLS.LanguageServer.Server + require Logger @behaviour ElixirLS.LanguageServer.Providers.ExecuteCommand @@ -40,60 +41,68 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.ApplySpec do else new_line = SourceFile.function_line(mod, fun, arity) - if SourceFile.function_def_on_line?(cur_text, new_line, fun) do + if new_line != nil and new_line > 0 and + SourceFile.function_def_on_line?(cur_text, new_line, fun) do new_line - else - raise "Function definition has moved since suggestion was generated. " <> - "Try again after file has been recompiled." end end - cur_line = Enum.at(SourceFile.lines(cur_text), line - 1) - [indentation] = Regex.run(~r/^\s*/u, cur_line) - - # Attempt to format to fit within the preferred line length, fallback to having it all on one - # line if anything fails - formatted = - try do - target_line_length = - case SourceFile.formatter_for(uri, state.project_dir, true) do - {:ok, {_, opts, _formatter_exs_dir}} -> - Keyword.get(opts, :line_length, @default_target_line_length) - - {:error, _} -> - @default_target_line_length - end - - target_line_length = target_line_length - String.length(indentation) - - Code.format_string!("@spec #{spec}", line_length: target_line_length) - |> IO.iodata_to_binary() - |> SourceFile.lines() - |> Enum.map_join("\n", &(indentation <> &1)) - |> Kernel.<>("\n") - rescue - _ -> - "#{indentation}@spec #{spec}\n" - end + if line do + cur_line = Enum.at(SourceFile.lines(cur_text), line - 1) + [indentation] = Regex.run(~r/^\s*/u, cur_line) + + # Attempt to format to fit within the preferred line length, fallback to having it all on one + # line if anything fails + formatted = + try do + target_line_length = + case SourceFile.formatter_for(uri, state.project_dir, true) do + {:ok, {_, opts, _formatter_exs_dir}} -> + Keyword.get(opts, :line_length, @default_target_line_length) + + {:error, reason} -> + Logger.warning( + "Unable to get formatter for #{uri} due to #{inspect(reason)}, using default line length" + ) + + @default_target_line_length + end + + target_line_length = target_line_length - String.length(indentation) + + Code.format_string!("@spec #{spec}", line_length: target_line_length) + |> IO.iodata_to_binary() + |> SourceFile.lines() + |> Enum.map_join("\n", &(indentation <> &1)) + |> Kernel.<>("\n") + rescue + _ -> + "#{indentation}@spec #{spec}\n" + end - edit_result = - JsonRpc.send_request("workspace/applyEdit", %{ - "label" => "Add @spec to #{mod}.#{fun}/#{arity}", - "edit" => %{ - "changes" => %{ - # we don't care about utf16 positions here as we send 0 - uri => [%{"range" => range(line - 1, 0, line - 1, 0), "newText" => formatted}] + edit_result = + JsonRpc.send_request("workspace/applyEdit", %{ + "label" => "Add @spec to #{mod}.#{fun}/#{arity}", + "edit" => %{ + "changes" => %{ + # we don't care about utf16 positions here as we send 0 + uri => [%{"range" => range(line - 1, 0, line - 1, 0), "newText" => formatted}] + } } - } - }) + }) - case edit_result do - {:ok, %{"applied" => true}} -> - {:ok, nil} + case edit_result do + {:ok, %{"applied" => true}} -> + {:ok, nil} - other -> - {:error, :server_error, - "cannot insert spec, workspace/applyEdit returned #{inspect(other)}", true} + other -> + {:error, :server_error, + "cannot insert spec, workspace/applyEdit returned #{inspect(other)}", true} + end + else + {:error, :server_error, + "cannot insert spec, function definition has moved since suggestion was generated. " <> + "Try again after file has been recompiled.", false} end end end diff --git a/apps/language_server/lib/language_server/providers/workspace_symbols.ex b/apps/language_server/lib/language_server/providers/workspace_symbols.ex index a7a1e7d65..c5b45cfb4 100644 --- a/apps/language_server/lib/language_server/providers/workspace_symbols.ex +++ b/apps/language_server/lib/language_server/providers/workspace_symbols.ex @@ -276,11 +276,15 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do end defp find_function_location(module, function, arity, path) do - if String.ends_with?(path, ".erl") do - ErlangSourceFile.function_line(path, function) - else - SourceFile.function_line(module, function, arity) - end + line = + if String.ends_with?(path, ".erl") do + ErlangSourceFile.function_line(path, function) + else + SourceFile.function_line(module, function, arity) + end + + # both functions can return nil + max(line || 1, 1) end defp find_module_path(module, beam_file) do