From 2ea7be2f2fa19d7595826820af024125c7b8a7c2 Mon Sep 17 00:00:00 2001 From: Steve Cohen Date: Thu, 14 Sep 2023 15:42:42 -0700 Subject: [PATCH] Better typespecs (#367) * Added more stringent typing to protocol Our protocol types were rather weak, relying on most of the structs defining their type as `@type t :: %__MODULE__{}`. We can do _much_ better than that, since we have detailed infomration about the entire type hierarchy. This commit does that providing detailed type information for the created structs and their constructors. I also found a couple of cases where the generators would miss type aliases that were just aliases to "base" types, like strings, integers, etc. --- apps/proto/lib/lexical/proto/alias.ex | 3 + apps/proto/lib/lexical/proto/enum.ex | 32 +++ .../proto/lib/lexical/proto/macros/message.ex | 7 +- apps/proto/lib/lexical/proto/macros/struct.ex | 8 +- .../lib/lexical/proto/macros/typespec.ex | 199 +++++++++++++++++- apps/proto/lib/lexical/proto/notification.ex | 4 +- apps/proto/lib/lexical/proto/request.ex | 8 +- apps/proto/lib/lexical/proto/response.ex | 6 +- apps/proto/lib/lexical/proto/type.ex | 6 +- .../lib/mix/tasks/lsp/data_model/type.ex | 13 ++ .../mix/tasks/lsp/data_model/type_alias.ex | 15 +- .../types/change_annotation/identifier.ex | 4 + .../lexical/protocol/types/document/filter.ex | 7 + .../lexical/protocol/types/pattern.ex | 4 + .../lexical.plugin.diagnostic.result.ex | 2 +- .../lib/lexical/server/configuration.ex | 33 +-- apps/server/lib/lexical/server/iex/helpers.ex | 2 +- .../lexical/server/project/progress/value.ex | 6 +- .../lexical/server/project/progress_test.exs | 2 +- 19 files changed, 322 insertions(+), 39 deletions(-) create mode 100644 apps/protocol/lib/generated/lexical/protocol/types/change_annotation/identifier.ex create mode 100644 apps/protocol/lib/generated/lexical/protocol/types/document/filter.ex create mode 100644 apps/protocol/lib/generated/lexical/protocol/types/pattern.ex diff --git a/apps/proto/lib/lexical/proto/alias.ex b/apps/proto/lib/lexical/proto/alias.ex index 5108efe73..854f9adab 100644 --- a/apps/proto/lib/lexical/proto/alias.ex +++ b/apps/proto/lib/lexical/proto/alias.ex @@ -1,12 +1,15 @@ defmodule Lexical.Proto.Alias do alias Lexical.Proto.CompileMetadata alias Lexical.Proto.Field + alias Lexical.Proto.Macros.Typespec defmacro defalias(alias_definition) do caller_module = __CALLER__.module CompileMetadata.add_type_alias_module(caller_module) quote location: :keep do + @type t :: unquote(Typespec.typespec(alias_definition, __CALLER__)) + def parse(lsp_map) do Field.extract(unquote(alias_definition), :alias, lsp_map) end diff --git a/apps/proto/lib/lexical/proto/enum.ex b/apps/proto/lib/lexical/proto/enum.ex index fe7ab47b9..85df34fd5 100644 --- a/apps/proto/lib/lexical/proto/enum.ex +++ b/apps/proto/lib/lexical/proto/enum.ex @@ -1,6 +1,26 @@ defmodule Lexical.Proto.Enum do + alias Lexical.Proto.Macros.Typespec + defmacro defenum(opts) do + names = + opts + |> Keyword.keys() + |> Enum.map(&{:literal, [], &1}) + + value_type = + opts + |> Keyword.values() + |> List.first() + |> Macro.expand(__CALLER__) + |> determine_type() + + name_type = Typespec.choice(names, __CALLER__) + quote location: :keep do + @type name :: unquote(name_type) + @type value :: unquote(value_type) + @type t :: name() | value() + unquote(parse_functions(opts)) def parse(unknown) do @@ -25,6 +45,18 @@ defmodule Lexical.Proto.Enum do end end + defp determine_type(i) when is_integer(i) do + quote do + integer() + end + end + + defp determine_type(s) when is_binary(s) do + quote do + String.t() + end + end + defp parse_functions(opts) do for {name, value} <- opts do quote location: :keep do diff --git a/apps/proto/lib/lexical/proto/macros/message.ex b/apps/proto/lib/lexical/proto/macros/message.ex index 6c77f0a10..6221ccfa3 100644 --- a/apps/proto/lib/lexical/proto/macros/message.ex +++ b/apps/proto/lib/lexical/proto/macros/message.ex @@ -9,19 +9,20 @@ defmodule Lexical.Proto.Macros.Message do Typespec } - def build(meta_type, method, types, param_names, opts \\ []) do + def build(meta_type, method, types, param_names, env, opts \\ []) do parse_fn = if Keyword.get(opts, :include_parse?, true) do Parse.build(types) end quote do - unquote(Struct.build(types)) - unquote(Typespec.build()) + unquote(Struct.build(types, env)) unquote(Access.build()) unquote(parse_fn) unquote(Meta.build(types)) + @type t :: unquote(Typespec.typespec()) + def method do unquote(method) end diff --git a/apps/proto/lib/lexical/proto/macros/struct.ex b/apps/proto/lib/lexical/proto/macros/struct.ex index 2f2802a67..af10cda7e 100644 --- a/apps/proto/lib/lexical/proto/macros/struct.ex +++ b/apps/proto/lib/lexical/proto/macros/struct.ex @@ -1,5 +1,7 @@ defmodule Lexical.Proto.Macros.Struct do - def build(opts) do + alias Lexical.Proto.Macros.Typespec + + def build(opts, env) do keys = Keyword.keys(opts) required_keys = required_keys(opts) @@ -23,7 +25,11 @@ defmodule Lexical.Proto.Macros.Struct do quote location: :keep do @enforce_keys unquote(required_keys) defstruct unquote(keys) + @type option :: unquote(Typespec.keyword_constructor_options(opts, env)) + @type options :: [option] + @spec new() :: t() + @spec new(options()) :: t() def new(opts \\ []) do struct!(__MODULE__, opts) end diff --git a/apps/proto/lib/lexical/proto/macros/typespec.ex b/apps/proto/lib/lexical/proto/macros/typespec.ex index d7f78e730..80ce0b4e8 100644 --- a/apps/proto/lib/lexical/proto/macros/typespec.ex +++ b/apps/proto/lib/lexical/proto/macros/typespec.ex @@ -1,7 +1,202 @@ defmodule Lexical.Proto.Macros.Typespec do - def build(_opts \\ []) do + def typespec(opts \\ [], env \\ nil) + + def typespec([], _) do quote do - @type t :: %__MODULE__{} + %__MODULE__{} + end + end + + def typespec(opts, env) when is_list(opts) do + typespecs = + for {name, type} <- opts, + name != :.. do + {name, do_typespec(type, env)} + end + + quote do + %__MODULE__{unquote_splicing(typespecs)} + end + end + + def typespec(typespec, env) do + do_typespec(typespec, env) + end + + def choice(options, env) do + do_typespec({:one_of, [], [options]}, env) + end + + def keyword_constructor_options(opts, env) do + for {name, type} <- opts, + name != :.. do + {name, do_typespec(type, env)} + end + |> or_types() + end + + defp do_typespec([], _env) do + # This is what's presented to typespec when a response has no results, as in the Shutdown response + nil + end + + defp do_typespec(nil, _env) do + quote(do: nil) + end + + defp do_typespec({:boolean, _, _}, _env) do + quote(do: boolean()) + end + + defp do_typespec({:string, _, _}, _env) do + quote(do: String.t()) + end + + defp do_typespec({:integer, _, _}, _env) do + quote(do: integer()) + end + + defp do_typespec({:float, _, _}, _env) do + quote(do: float()) + end + + defp do_typespec({:__MODULE__, _, nil}, env) do + env.module + end + + defp do_typespec({:optional, _, [optional_type]}, env) do + quote do + unquote(do_typespec(optional_type, env)) | nil + end + end + + defp do_typespec({:__aliases__, _, raw_alias} = aliased_module, env) do + expanded_alias = Macro.expand(aliased_module, env) + + case List.last(raw_alias) do + :Position -> + other_alias = + case expanded_alias do + Lexical.Document.Range -> + Lexical.Protocol.Types.Range + + _ -> + Lexical.Document.Range + end + + quote do + unquote(expanded_alias).t() | unquote(other_alias).t() + end + + :Range -> + other_alias = + case expanded_alias do + Lexical.Document.Range -> + Lexical.Protocol.Types.Range + + _ -> + Lexical.Document.Range + end + + quote do + unquote(expanded_alias).t() | unquote(other_alias).t() + end + + _ -> + quote do + unquote(expanded_alias).t() + end + end + end + + defp do_typespec({:literal, _, value}, _env) when is_atom(value) do + value + end + + defp do_typespec({:literal, _, [value]}, _env) do + literal_type(value) + end + + defp do_typespec({:type_alias, _, [alias_dest]}, env) do + do_typespec(alias_dest, env) + end + + defp do_typespec({:one_of, _, [type_list]}, env) do + type_list + |> Enum.map(&do_typespec(&1, env)) + |> or_types() + end + + defp do_typespec({:list_of, _, items}, env) do + refined = + items + |> Enum.map(&do_typespec(&1, env)) + |> or_types() + + quote do + [unquote(refined)] + end + end + + defp do_typespec({:tuple_of, _, [items]}, env) do + refined = Enum.map(items, &do_typespec(&1, env)) + + quote do + {unquote_splicing(refined)} + end + end + + defp do_typespec({:map_of, _, items}, env) do + value_types = + items + |> Enum.map(&do_typespec(&1, env)) + |> or_types() + + quote do + %{String.t() => unquote(value_types)} + end + end + + defp do_typespec({:any, _, _}, _env) do + quote do + any() + end + end + + defp or_types(list_of_types) do + Enum.reduce(list_of_types, nil, fn + type, nil -> + type + + type, acc -> + quote do + unquote(type) | unquote(acc) + end + end) + end + + defp literal_type(thing) do + case thing do + string when is_binary(string) -> + quote(do: String.t()) + + integer when is_integer(integer) -> + quote(do: integer()) + + float when is_binary(float) -> + quote(do: float()) + + boolean when is_boolean(boolean) -> + quote(do: boolean()) + + atom when is_atom(atom) -> + atom + + [] -> + quote(do: []) + + [elem | _] -> + quote(do: [unquote(literal_type(elem))]) end end end diff --git a/apps/proto/lib/lexical/proto/notification.ex b/apps/proto/lib/lexical/proto/notification.ex index d148f69e1..1cfefc594 100644 --- a/apps/proto/lib/lexical/proto/notification.ex +++ b/apps/proto/lib/lexical/proto/notification.ex @@ -31,7 +31,7 @@ defmodule Lexical.Proto.Notification do quote location: :keep do defmodule LSP do - unquote(Message.build({:notification, :lsp}, method, lsp_types, param_names)) + unquote(Message.build({:notification, :lsp}, method, lsp_types, param_names, caller)) def new(opts \\ []) do opts @@ -41,7 +41,7 @@ defmodule Lexical.Proto.Notification do end unquote( - Message.build({:notification, :elixir}, method, elixir_types, param_names, + Message.build({:notification, :elixir}, method, elixir_types, param_names, caller, include_parse?: false ) ) diff --git a/apps/proto/lib/lexical/proto/request.ex b/apps/proto/lib/lexical/proto/request.ex index 0d919cd2a..ae5a5bbf6 100644 --- a/apps/proto/lib/lexical/proto/request.ex +++ b/apps/proto/lib/lexical/proto/request.ex @@ -35,9 +35,13 @@ defmodule Lexical.Proto.Request do param_names = Keyword.keys(types) lsp_module_name = Module.concat(caller.module, LSP) + Message.build({:request, :elixir}, method, elixir_types, param_names, caller, + include_parse?: false + ) + quote location: :keep do defmodule LSP do - unquote(Message.build({:request, :lsp}, method, lsp_types, param_names)) + unquote(Message.build({:request, :lsp}, method, lsp_types, param_names, caller)) def new(opts \\ []) do opts @@ -50,7 +54,7 @@ defmodule Lexical.Proto.Request do alias Lexical.Protocol.Types unquote( - Message.build({:request, :elixir}, method, elixir_types, param_names, + Message.build({:request, :elixir}, method, elixir_types, param_names, caller, include_parse?: false ) ) diff --git a/apps/proto/lib/lexical/proto/response.ex b/apps/proto/lib/lexical/proto/response.ex index a5040e306..d3e8a519e 100644 --- a/apps/proto/lib/lexical/proto/response.ex +++ b/apps/proto/lib/lexical/proto/response.ex @@ -13,15 +13,15 @@ defmodule Lexical.Proto.Response do jsonrpc_types = [ id: quote(do: optional(one_of([integer(), string()]))), - error: quote(do: optional(LspTypes.ResponseError)), + error: quote(do: optional(Lexical.Proto.LspTypes.ResponseError)), result: quote(do: optional(unquote(response_type))) ] quote location: :keep do alias Lexical.Proto.LspTypes unquote(Access.build()) - unquote(Struct.build(jsonrpc_types)) - unquote(Typespec.build()) + unquote(Struct.build(jsonrpc_types, __CALLER__)) + @type t :: unquote(Typespec.typespec()) unquote(Meta.build(jsonrpc_types)) unquote(constructors()) diff --git a/apps/proto/lib/lexical/proto/type.ex b/apps/proto/lib/lexical/proto/type.ex index 05309bbe7..385cad849 100644 --- a/apps/proto/lib/lexical/proto/type.ex +++ b/apps/proto/lib/lexical/proto/type.ex @@ -20,8 +20,10 @@ defmodule Lexical.Proto.Type do unquote(Json.build(caller_module)) unquote(Inspect.build(caller_module)) unquote(Access.build()) - unquote(Struct.build(types)) - unquote(Typespec.build(types)) + unquote(Struct.build(types, __CALLER__)) + + @type t :: unquote(Typespec.typespec(types, __CALLER__)) + unquote(Parse.build(types)) unquote(Match.build(types, caller_module)) unquote(Meta.build(types)) diff --git a/apps/proto/lib/mix/tasks/lsp/data_model/type.ex b/apps/proto/lib/mix/tasks/lsp/data_model/type.ex index efe3fdca9..a6f6e3594 100644 --- a/apps/proto/lib/mix/tasks/lsp/data_model/type.ex +++ b/apps/proto/lib/mix/tasks/lsp/data_model/type.ex @@ -29,6 +29,19 @@ defmodule Mix.Tasks.Lsp.DataModel.Type do end end + def to_typespec(%__MODULE__{} = type) do + case type.type_name do + "string" -> quote(do: String.t()) + "integer" -> quote(do: integer()) + "uinteger" -> quote(do: integer()) + "boolean" -> quote(do: boolean()) + "null" -> quote(do: nil) + "DocumentUri" -> quote(do: String.t()) + "decimal" -> quote(do: float()) + "URI" -> quote(do: Lexical.uri()) + end + end + def references(%__MODULE__{}) do [] end diff --git a/apps/proto/lib/mix/tasks/lsp/data_model/type_alias.ex b/apps/proto/lib/mix/tasks/lsp/data_model/type_alias.ex index 19ea29a46..9bd5e8768 100644 --- a/apps/proto/lib/mix/tasks/lsp/data_model/type_alias.ex +++ b/apps/proto/lib/mix/tasks/lsp/data_model/type_alias.ex @@ -28,8 +28,19 @@ defmodule Mix.Tasks.Lsp.DataModel.TypeAlias do :skip end - def build_definition(%__MODULE__{type: %Base{}}, _, _, _) do - :skip + def build_definition(%__MODULE__{type: %Base{}} = type_alias, %Mappings{} = mappings, _, _) do + with {:ok, destination_module} <- Mappings.fetch_destination_module(mappings, type_alias.name) do + type = Base.to_typespec(type_alias.type) + + ast = + quote do + defmodule unquote(destination_module) do + @type t :: unquote(type) + end + end + + {:ok, ast} + end end def build_definition( diff --git a/apps/protocol/lib/generated/lexical/protocol/types/change_annotation/identifier.ex b/apps/protocol/lib/generated/lexical/protocol/types/change_annotation/identifier.ex new file mode 100644 index 000000000..e97f4ef31 --- /dev/null +++ b/apps/protocol/lib/generated/lexical/protocol/types/change_annotation/identifier.ex @@ -0,0 +1,4 @@ +# This file's contents are auto-generated. Do not edit. +defmodule Lexical.Protocol.Types.ChangeAnnotation.Identifier do + @type t :: String.t() +end diff --git a/apps/protocol/lib/generated/lexical/protocol/types/document/filter.ex b/apps/protocol/lib/generated/lexical/protocol/types/document/filter.ex new file mode 100644 index 000000000..08f917eb0 --- /dev/null +++ b/apps/protocol/lib/generated/lexical/protocol/types/document/filter.ex @@ -0,0 +1,7 @@ +# This file's contents are auto-generated. Do not edit. +defmodule Lexical.Protocol.Types.Document.Filter do + alias Lexical.Proto + alias Lexical.Protocol.Types + use Proto + defalias one_of([Types.TextDocument.Filter, Types.Notebook.Cell.TextDocument.Filter]) +end diff --git a/apps/protocol/lib/generated/lexical/protocol/types/pattern.ex b/apps/protocol/lib/generated/lexical/protocol/types/pattern.ex new file mode 100644 index 000000000..126e359d6 --- /dev/null +++ b/apps/protocol/lib/generated/lexical/protocol/types/pattern.ex @@ -0,0 +1,4 @@ +# This file's contents are auto-generated. Do not edit. +defmodule Lexical.Protocol.Types.Pattern do + @type t :: String.t() +end diff --git a/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex b/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex index 22ec28e0c..c3a7ffafa 100644 --- a/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex +++ b/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex @@ -50,7 +50,7 @@ defimpl Lexical.Convertible, for: Lexical.Plugin.V1.Diagnostic.Result do defp position_to_range(%Document{} = document, {start_line, start_column, end_line, end_column}) do with {:ok, start_pos} <- position_to_range(document, {start_line, start_column}), {:ok, end_pos} <- position_to_range(document, {end_line, end_column}) do - {:ok, Document.Range.new(start_pos, end_pos)} + {:ok, Types.Range.new(start: start_pos, end: end_pos)} end end diff --git a/apps/server/lib/lexical/server/configuration.ex b/apps/server/lib/lexical/server/configuration.ex index 055bc34da..3a437504d 100644 --- a/apps/server/lib/lexical/server/configuration.ex +++ b/apps/server/lib/lexical/server/configuration.ex @@ -1,11 +1,11 @@ defmodule Lexical.Server.Configuration do alias Lexical.Project - alias Lexical.Proto.LspTypes.Registration alias Lexical.Protocol.Id alias Lexical.Protocol.Notifications.DidChangeConfiguration alias Lexical.Protocol.Requests alias Lexical.Protocol.Requests.RegisterCapability alias Lexical.Protocol.Types.ClientCapabilities + alias Lexical.Protocol.Types.Registration alias Lexical.Server.Configuration.Support alias Lexical.Server.Dialyzer @@ -14,8 +14,13 @@ defmodule Lexical.Server.Configuration do additional_watched_extensions: nil, dialyzer_enabled?: false - @type t :: %__MODULE__{} - @dialyzer {:nowarn_function, maybe_enable_dialyzer: 2} + @type t :: %__MODULE__{ + project: Project.t() | nil, + additional_watched_extensions: [String.t()] | nil, + dialyzer_enabled?: boolean() + } + + @dialyzer {:nowarn_function, set_dialyzer_enabled: 2} @spec new(Lexical.uri(), map()) :: t def new(root_uri, %ClientCapabilities{} = client_capabilities) do @@ -27,7 +32,6 @@ defmodule Lexical.Server.Configuration do @spec default(t | nil) :: {:ok, t} | {:ok, t, Requests.RegisterCapability.t()} - | {:restart, Logger.level(), String.t()} def default(nil) do {:ok, default_config()} end @@ -39,7 +43,6 @@ defmodule Lexical.Server.Configuration do @spec on_change(t, DidChangeConfiguration.t()) :: {:ok, t} | {:ok, t, Requests.RegisterCapability.t()} - | {:restart, Logger.level(), String.t()} def on_change(%__MODULE__{} = old_config, :defaults) do apply_config_change(old_config, default_config()) end @@ -53,22 +56,20 @@ defmodule Lexical.Server.Configuration do end defp apply_config_change(%__MODULE__{} = old_config, %{} = settings) do - with {:ok, new_config} <- maybe_enable_dialyzer(old_config, settings) do - maybe_add_watched_extensions(new_config, settings) - end + old_config + |> set_dialyzer_enabled(settings) + |> maybe_add_watched_extensions(settings) end - defp maybe_enable_dialyzer(%__MODULE__{} = old_config, settings) do + defp set_dialyzer_enabled(%__MODULE__{} = old_config, settings) do enabled? = - case Dialyzer.check_support() do - :ok -> - Map.get(settings, "dialyzerEnabled", true) - - _ -> - false + if Dialyzer.check_support() == :ok do + Map.get(settings, "dialyzerEnabled", true) + else + false end - {:ok, %__MODULE__{old_config | dialyzer_enabled?: enabled?}} + %__MODULE__{old_config | dialyzer_enabled?: enabled?} end defp maybe_add_watched_extensions(%__MODULE__{} = old_config, %{ diff --git a/apps/server/lib/lexical/server/iex/helpers.ex b/apps/server/lib/lexical/server/iex/helpers.ex index 69077ff8b..ff14900cc 100644 --- a/apps/server/lib/lexical/server/iex/helpers.ex +++ b/apps/server/lib/lexical/server/iex/helpers.ex @@ -80,7 +80,7 @@ defmodule Lexical.Server.IEx.Helpers do def complete(project, %Document{} = source, line, character, context) do context = if is_nil(context) do - Completion.Context.new(trigger_kind: nil) + Completion.Context.new(trigger_kind: :trigger_character) else context end diff --git a/apps/server/lib/lexical/server/project/progress/value.ex b/apps/server/lib/lexical/server/project/progress/value.ex index 1e61940ec..9d505e71d 100644 --- a/apps/server/lib/lexical/server/project/progress/value.ex +++ b/apps/server/lib/lexical/server/project/progress/value.ex @@ -25,21 +25,21 @@ defmodule Lexical.Server.Project.Progress.Value do def to_protocol(%__MODULE__{kind: :begin} = value) do Notifications.Progress.new( token: value.token, - value: WorkDone.Progress.Begin.new(kind: value.kind, title: value.title) + value: WorkDone.Progress.Begin.new(kind: "begin", title: value.title) ) end def to_protocol(%__MODULE__{kind: :report} = value) do Notifications.Progress.new( token: value.token, - value: WorkDone.Progress.Report.new(kind: value.kind, message: value.message) + value: WorkDone.Progress.Report.new(kind: "report", message: value.message) ) end def to_protocol(%__MODULE__{kind: :end} = value) do Notifications.Progress.new( token: value.token, - value: WorkDone.Progress.End.new(kind: value.kind, message: value.message) + value: WorkDone.Progress.End.new(kind: "end", message: value.message) ) end end diff --git a/apps/server/test/lexical/server/project/progress_test.exs b/apps/server/test/lexical/server/project/progress_test.exs index 9bb618909..a0f0a98ae 100644 --- a/apps/server/test/lexical/server/project/progress_test.exs +++ b/apps/server/test/lexical/server/project/progress_test.exs @@ -55,7 +55,7 @@ defmodule Lexical.Server.Project.ProgressTest do RemoteControl.Api.broadcast(project, report_message) assert_receive {:transport, %Notifications.Progress{lsp: %{token: ^token, value: value}}} - assert value.kind == :report + assert value.kind == "report" assert value.message == "lib/file.ex" assert value.percentage == nil assert value.cancellable == nil