From 84b646a1203407597241932a9484372cfdf07813 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Mon, 3 Aug 2020 07:49:09 -1000 Subject: [PATCH 1/5] WIP: Configuration file support Add a user configuration file that is read on startup. Change the server settings to always be fully fleshed out. So the ElixirLS.LanguageServer.Server state `settings` always has all the settings (and as atoms) instead of being an empty map by default. This consolidates the default setting logic into the new `ConfigParser` module instead of being strewn about the Server. TODO: - [ ] Fix tests (I'm getting a strange error with the tests that I cannot figure out.) - [ ] Read a configuratioon file from the repository Note: Editor configuration always overrides the configuration file, and the VSCode extension always sends in the full configuration. This means that the configuration file is not currently useable with the VSCode extension. This will probably have to be modified in the VSCode extension because I think it is important for the extension to have the final control of the server settings. --- README.md | 77 +++++++- .../elixir_ls_utils/lib/config/setting_def.ex | 8 + apps/elixir_ls_utils/lib/config_parser.ex | 171 ++++++++++++++++++ apps/elixir_ls_utils/lib/xdg.ex | 32 ++++ .../test/config_parser_test.exs | 113 ++++++++++++ .../test/support/example_config_file.jsonc | 7 + apps/elixir_ls_utils/test/support/mock_xdg.ex | 5 + .../lib/language_server/config_loader.ex | 68 +++++++ .../lib/language_server/server.ex | 154 +++++++++------- apps/language_server/test/dialyzer_test.exs | 1 + .../language_server/config_loader_test.exs | 18 ++ apps/language_server/test/server_test.exs | 57 +++++- .../test/support/server_test_helpers.ex | 24 ++- config/config.exs | 8 + 14 files changed, 669 insertions(+), 74 deletions(-) create mode 100644 apps/elixir_ls_utils/lib/config/setting_def.ex create mode 100644 apps/elixir_ls_utils/lib/config_parser.ex create mode 100644 apps/elixir_ls_utils/lib/xdg.ex create mode 100644 apps/elixir_ls_utils/test/config_parser_test.exs create mode 100644 apps/elixir_ls_utils/test/support/example_config_file.jsonc create mode 100644 apps/elixir_ls_utils/test/support/mock_xdg.ex create mode 100644 apps/language_server/lib/language_server/config_loader.ex create mode 100644 apps/language_server/test/language_server/config_loader_test.exs diff --git a/README.md b/README.md index dd5877742..9f44a3e66 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,73 @@ Erlang: You may want to install Elixir and Erlang from source, using the [kiex](https://github.com/taylor/kiex) and [kerl](https://github.com/kerl/kerl) tools. This will let you go-to-definition for core Elixir and Erlang modules. +## Configuration + +ElixirLS can be configured through configuration/settings files + +Configuration file location and precedence: + +- $XDG_CONFIG_HOME/elixir_ls/config.json (e.g. `~/.config/elixir_ls/config.json`) +- repo/.elixir_ls_config.json + - Or should this be the projectDir/.elixir_ls_config.json (which is a little tricky because the project directory can be set in `workspace/didChangeConfiguration`) + - I think this should be added in the future +- Editor config (e.g. `workspace/didChangeConfiguration`) + - Note: Mainly supported via vscode-elixir-ls + +### Configuration File Contents + +The configuration file can include comments (e.g. lines starting with `//` are ignored) + +Configuration keys: +- `dialyzerEnabled`: Run ElixirLS's rapid Dialyzer when code is saved + - Allowed values: `true`, `false` + - default: `true` +- `dialyzerWarnOpts`: Dialyzer options to enable or disable warnings. See + Dialyzer's documentation for options. Note that the `race_conditions` option + is unsupported + - Allowed values: "error_handling", "no_behaviours", "no_contracts", + "no_fail_call", "no_fun_app", "no_improper_lists", "no_match", + "no_missing_calls", "no_opaque", "no_return", "no_undefined_callbacks", + "no_unused", "underspecs", "unknown", "unmatched_returns", "overspecs", + "specdiffs" + - default: `[]` +- `dialyzerFormat`: Formatter to use for Dialyzer warnings + - Allowed values: ["dialyzer", "dialyxir_short", "dialyxir_long"] + - default: `"dialyxir_long"` +- `mixEnv`: Mix environment to use for compilation + - default: `"test"` +- `projectDir`: Subdirectory containing Mix project if not in the project root + - default: `"."` +- `fetchDeps`: Automatically fetch project dependencies when compiling + - Allowed values: `true`, `false` + - default: `true` +- `suggestSpecs`: Suggest @spec annotations inline using Dialyzer's inferred + success typings (Requires Dialyzer) + - Allowed values: `true`, `false` + - default: `true` + +Example config: +```jsonc +{ + // You may want to disable dialyzer if you find it unhelpful or your machine is underpowered + "dialyzerEnabled": false, + + // You may want to disable fetching dependencies since it sometimes gets stuck/has race conditions + "fetchDeps": false +} +``` + +### Local setup + +Because ElixirLS may get launched from an IDE that itself got launched from a +graphical shell, the environment may not be complete enough to run or even find +the correct Elixir/OTP version. The wrapper scripts try to configure `asdf-vm` +if available, but that may not be what you want or need. Therefore, prior to +executing Elixir, the script will source `$XDG_CONFIG_HOME/elixir_ls/setup.sh` +(e.g. `~/.config/elixir_ls/setup.sh`), if available. The environment variable +`ELS_MODE` is set to either `debugger` or `language_server` to help you decide +what to do inside the script, if needed. + ## Debugger support ElixirLS includes debugger support adhering to the [VS Code debugger protocol](https://code.visualstudio.com/docs/extensionAPI/api-debugging) which is closely related to the Language Server Protocol. At the moment, only line breakpoints are supported. @@ -191,7 +258,7 @@ If you get an error like the following immediately on startup: ** (EXIT) no process: the process is not alive or there's no process currently associated with the given name, possibly because its application isn't started ``` -and you installed Elixir and Erlang from the Erlang Solutions repository, you may not have a full installation of erlang. This can be solved with `sudo apt-get install esl-erlang`. Originally reported in [#208](https://github.com/elixir-lsp/elixir-ls/issues/208). +and you installed Elixir and Erlang from the Erlang Solutions repository, you may not have a full installation of erlang. This can be solved with `sudo apt-get install esl-erlang`. Originally reported in [#208](https://github.com/elixir-lsp/elixir-ls/issues/208). If you're running Fedora [you may need to run](https://github.com/JakeBecker/vscode-elixir-ls/issues/104#issuecomment-622414197) `sudo dnf install erlang` ## Known Issues/Limitations @@ -210,14 +277,6 @@ Run `mix compile`, then `mix elixir_ls.release -o `. This builds th If you're packaging these archives in an IDE plugin, make sure to build using the minimum supported OTP version for the best backwards-compatibility. Alternatively, you can use a [precompiled release](https://github.com/elixir-lsp/elixir-ls/releases). -### Local setup - -Because ElixirLS may get launched from an IDE that itself got launched from a graphical shell, the environment may not -be complete enough to run or even find the correct Elixir/OTP version. The wrapper scripts try to configure `asdf-vm` -if available, but that may not be what you want or need. Therefore, prior to executing Elixir, the script will source -`$XDG_CONFIG_HOME/elixir_ls/setup.sh` (e.g. `~/.config/elixir_ls/setup.sh`), if available. The environment variable -`ELS_MODE` is set to either `debugger` or `language_server` to help you decide what to do inside the script, if needed. - ## Acknowledgements and related projects ElixirLS isn't the first frontend-independent server for Elixir language support. The original was [Alchemist Server](https://github.com/tonini/alchemist-server/), which powers the [Alchemist](https://github.com/tonini/alchemist.el) plugin for Emacs. Another project, [Elixir Sense](https://github.com/msaraiva/elixir_sense), builds upon Alchemist and powers the [Elixir plugin for Atom](https://github.com/msaraiva/atom-elixir) as well as another VS Code plugin, [VSCode Elixir](https://github.com/fr1zle/vscode-elixir). ElixirLS uses Elixir Sense for several code insight features. Credit for those projects goes to their respective authors. diff --git a/apps/elixir_ls_utils/lib/config/setting_def.ex b/apps/elixir_ls_utils/lib/config/setting_def.ex new file mode 100644 index 000000000..45831bdd7 --- /dev/null +++ b/apps/elixir_ls_utils/lib/config/setting_def.ex @@ -0,0 +1,8 @@ +defmodule ElixirLS.Utils.Config.SettingDef do + @moduledoc """ + Defines attributes for an individual setting supported by ElixirLS. + """ + + @enforce_keys [:key, :json_key, :type, :default, :doc] + defstruct [:key, :json_key, :type, :default, :doc] +end diff --git a/apps/elixir_ls_utils/lib/config_parser.ex b/apps/elixir_ls_utils/lib/config_parser.ex new file mode 100644 index 000000000..63d1faef1 --- /dev/null +++ b/apps/elixir_ls_utils/lib/config_parser.ex @@ -0,0 +1,171 @@ +defmodule ElixirLS.Utils.ConfigParser do + @moduledoc """ + Parses and loads an ElixirLS configuration file + """ + + alias ElixirLS.Utils.Config.SettingDef + + @settings [ + %SettingDef{ + key: :dialyzer_enabled, + json_key: "dialyzerEnabled", + type: :boolean, + default: true, + doc: "Run ElixirLS's rapid Dialyzer when code is saved" + }, + %SettingDef{ + key: :dialyzer_format, + json_key: "dialyzerFormat", + type: {:one_of, ["dialyzer", "dialyxir_short", "dialyxir_long"]}, + default: "dialyxir_long", + doc: "Formatter to use for Dialyzer warnings" + }, + %SettingDef{ + key: :dialyzer_warn_opts, + json_key: "dialyzerWarnOpts", + type: + {:custom, ElixirLS.Utils.NimbleListChecker, :list, + [ + "error_handling", + "no_behaviours", + "no_contracts", + "no_fail_call", + "no_fun_app", + "no_improper_lists", + "no_match", + "no_missing_calls", + "no_opaque", + "no_return", + "no_undefined_callbacks", + "no_unused", + "underspecs", + "unknown", + "unmatched_returns", + "overspecs", + "specdiffs" + ]}, + default: [], + doc: + "Dialyzer options to enable or disable warnings. See Dialyzer's documentation for options. Note that the `race_conditions` option is unsupported" + }, + %SettingDef{ + key: :fetch_deps, + json_key: "fetchDeps", + type: :boolean, + default: true, + doc: "Automatically fetch project dependencies when compiling" + }, + %SettingDef{ + key: :mix_env, + json_key: "mixEnv", + type: :string, + default: "test", + doc: "Mix environment to use for compilation" + }, + %SettingDef{ + key: :mix_target, + json_key: "mixTarget", + type: :string, + default: "host", + doc: "Mix target (`MIX_TARGET`) to use for compilation (requires Elixir >= 1.8)" + }, + %SettingDef{ + key: :project_dir, + json_key: "projectDir", + type: :string, + default: "", + doc: + "Subdirectory containing Mix project if not in the project root. " <> + "If value is \"\" then defaults to the workspace rootUri." + }, + %SettingDef{ + key: :suggest_specs, + json_key: "suggestSpecs", + type: :boolean, + default: true, + doc: + "Suggest @spec annotations inline using Dialyzer's inferred success typings " <> + "(Requires Dialyzer)" + }, + %SettingDef{ + key: :trace, + json_key: "trace", + type: :map, + default: %{}, + doc: "Ignored" + } + ] + + def load_config_file(path) do + with {:ok, contents} <- File.read(path) do + load_config(contents) + end + end + + def load_config(contents) do + with {:ok, settings_map} <- json_decode(contents), + {:ok, validated_options} <- parse_config(settings_map) do + {:ok, Map.new(validated_options), []} + end + end + + def default_config do + @settings + |> Map.new(fn %SettingDef{} = setting_def -> + %SettingDef{key: key, default: default} = setting_def + {key, default} + end) + end + + @doc """ + Parse the raw decoded JSON to the settings map (including translation from + camelCase to snake_case) + """ + def parse_config(settings_map) do + # Because we use a configuration layering approach, this configuration + # parsing should be based on the settings_map and not the possible settings. + # The return value should be *only* the settings that were passed in, don't + # return the defaults here. + values = + settings_map + |> Enum.map(fn {json_key, value} -> + case translate_key(json_key) do + {:ok, key} -> {:ok, {key, value}} + {:error, "unknown key"} -> {:error, {:unrecognized_configuration_key, json_key, value}} + end + end) + + {good, errors} = Enum.split_with(values, &match?({:ok, _}, &1)) + config = Map.new(good, fn {:ok, {key, val}} -> {key, val} end) + + {:ok, config, errors} + end + + for %SettingDef{key: key, json_key: json_key} <- @settings do + defp translate_key(unquote(json_key)) do + {:ok, unquote(key)} + end + end + + defp translate_key(_), do: {:error, "unknown key"} + + for setting <- @settings do + def valid_key?(unquote(setting.json_key)), do: true + end + + def valid_key?(_), do: false + + def json_decode(contents) when is_binary(contents) do + contents + |> String.split(["\n", "\r", "\r\n"], trim: true) + |> Enum.map(&String.trim/1) + # Ignore json comments + |> Enum.reject(&String.starts_with?(&1, "#")) + |> Enum.join() + |> JasonVendored.decode() + |> case do + {:ok, _} = ok -> ok + {:error, %JasonVendored.DecodeError{} = err} -> {:error, {:invalid_json, err}} + end + end +end diff --git a/apps/elixir_ls_utils/lib/xdg.ex b/apps/elixir_ls_utils/lib/xdg.ex new file mode 100644 index 000000000..f395fa488 --- /dev/null +++ b/apps/elixir_ls_utils/lib/xdg.ex @@ -0,0 +1,32 @@ +defmodule ElixirLS.Utils.XDG do + @moduledoc """ + Utilities for reading files within ElixirLS's XDG configuration directory + """ + + @default_xdg_directory "$HOME/.config" + + def read_elixir_ls_config_file(path) do + xdg_directory() + |> Path.join("elixir_ls") + |> Path.join(path) + |> File.read() + |> case do + {:ok, file_contents} -> {:ok, file_contents} + err -> err + end + end + + defp xdg_directory do + case System.get_env("XDG_CONFIG_HOME") do + nil -> + @default_xdg_directory + + xdg_directory -> + if File.dir?(xdg_directory) do + xdg_directory + else + raise "$XDG_CONFIG_HOME environment variable set, but directory does not exist" + end + end + end +end diff --git a/apps/elixir_ls_utils/test/config_parser_test.exs b/apps/elixir_ls_utils/test/config_parser_test.exs new file mode 100644 index 000000000..664836658 --- /dev/null +++ b/apps/elixir_ls_utils/test/config_parser_test.exs @@ -0,0 +1,113 @@ +defmodule ElixirLS.Utils.ConfigParserTest do + use ExUnit.Case, async: true + + alias ElixirLS.Utils.ConfigParser + + @default_mix_env "test" + @default_dialyzer_enabled true + @default_project_dir "" + + test "default_config returns the defaults" do + config = ConfigParser.default_config() + + assert config.mix_env == @default_mix_env + assert config.dialyzer_enabled == @default_dialyzer_enabled + assert config.project_dir == @default_project_dir + end + + test "load_config new defaults" do + config_contents = "{\"dialyzerFormat\": \"dialyxir_short\"}" + + assert {:ok, %{dialyzer_format: "dialyxir_short"}, []} = + ConfigParser.load_config(config_contents) + end + + test "load_config with an empty configuration file returns an empty config" do + config_contents = "{}" + + assert {:ok, config, errors} = ConfigParser.load_config(config_contents) + + assert config == %{} + assert errors == [] + end + + test "load_config with an invalid setting key" do + config_contents = JasonVendored.encode!(%{"badKey" => "invalid"}) + + assert {:ok, config, errors} = ConfigParser.load_config(config_contents) + + assert [error: {:unrecognized_configuration_key, "badKey", "invalid"}] = errors + end + + test "load_config with dialyzer disabled and false" do + config_contents = "{\n\t\"dialyzerEnabled\": false,\n\t\"dialyzerFormat\": \"dialyzer\"\n}\n" + assert {:ok, config, errors} = ConfigParser.load_config(config_contents) + + assert config == %{ + dialyzer_enabled: false, + dialyzer_format: "dialyzer" + } + end + + @tag :pending + test "load_config with a setting with an invalid value" do + config_contents = JasonVendored.encode!(%{"dialyzerFormat" => "other_format"}) + + assert {:ok, _config, errors} = ConfigParser.load_config(config_contents) + + assert [ + error: + {:value_not_allowed, "other_format", "dialyzerFormat", + ["dialyzer", "dialyxir_short", "dialyxir_long"]} + ] = errors + end + + test "load_config can set the default mix env" do + config_contents = JasonVendored.encode!(%{"mixEnv" => "dev"}) + + assert {:ok, config, errors} = ConfigParser.load_config(config_contents) + + assert config.mix_env == "dev" + assert errors == [] + end + + test "load_config with an invalid json file" do + config_contents = "this is not json" + + assert {:error, {:invalid_json, decode_error}} = ConfigParser.load_config(config_contents) + + assert decode_error.data == config_contents + end + + test "load_config/1 ignores lines with comments" do + config_contents = """ + # This is the configuration file for ElixirLS + { + # Disable dialyzer + "dialyzerEnabled": false + } + """ + + assert {:ok, config, _} = ConfigParser.load_config(config_contents) + + assert config.dialyzer_enabled == false + end + + test "load_config_file/1 loads a valid configuration file" do + path = + Path.join([__DIR__, "support/example_config_file.jsonc"]) + |> Path.expand() + + assert {:ok, config, errors} = ConfigParser.load_config_file(path) + assert config.dialyzer_enabled == false + assert config.fetch_deps == false + assert config.mix_env == "dev" + assert errors == [] + end + + test "load_config_file/1 with a missing configuration file" do + path = "non-existant-file" + + assert {:error, :enoent} = ConfigParser.load_config_file(path) + end +end diff --git a/apps/elixir_ls_utils/test/support/example_config_file.jsonc b/apps/elixir_ls_utils/test/support/example_config_file.jsonc new file mode 100644 index 000000000..a6e5e01cb --- /dev/null +++ b/apps/elixir_ls_utils/test/support/example_config_file.jsonc @@ -0,0 +1,7 @@ +# This is the configuration file for ElixirLS +{ + # Disable dialyzer + "dialyzerEnabled": false, + "fetchDeps": false, + "mixEnv": "dev" +} diff --git a/apps/elixir_ls_utils/test/support/mock_xdg.ex b/apps/elixir_ls_utils/test/support/mock_xdg.ex new file mode 100644 index 000000000..9b5fbef47 --- /dev/null +++ b/apps/elixir_ls_utils/test/support/mock_xdg.ex @@ -0,0 +1,5 @@ +defmodule ElixirLS.Utils.Test.MockXDG do + def read_elixir_ls_config_file(_) do + {:error, :enoent} + end +end diff --git a/apps/language_server/lib/language_server/config_loader.ex b/apps/language_server/lib/language_server/config_loader.ex new file mode 100644 index 000000000..4efc40769 --- /dev/null +++ b/apps/language_server/lib/language_server/config_loader.ex @@ -0,0 +1,68 @@ +defmodule ElixirLS.LanguageServer.ConfigLoader do + @moduledoc """ + Responsible for loading the configuration. Applies configuration in this + order: defaults, previously loaded configuration, user home dir configuration, + editor configuration + """ + + # Configuration Loading Order + # + # - Server starts up (it knows the root directory) + # - User provides did_change_configuration (or we fall back to defaults) + # - NOTE: This is where all the configuration is loaded + # - Load and apply all the configuration in this order: + # - Existing Server Config + # - User Home Config + # - Workspace Config + # - Editor Config + + alias ElixirLS.Utils.ConfigParser + alias ElixirLS.LanguageServer.JsonRpc + + # Can we change this to load also from the workspace root? Note: might need multiple passes for that to work + + def load(prev_config, editor_config, opts \\ []) do + user_home_config = + Keyword.get_lazy(opts, :load_user_home_config, fn -> + load_user_home_config() + end) + + default_config = ConfigParser.default_config() + editor_config = ConfigParser.parse_config(editor_config) + + config = + [default_config, prev_config, user_home_config, editor_config] + |> Enum.reduce(%{}, fn + {:ok, :skip}, acc -> + acc + + {:ok, config, errors}, acc when is_map(config) -> + Enum.each(errors, fn {:error, {:unrecognized_configuration_key, key, value}} -> + JsonRpc.log_message(:warning, "Invalid configuration key: #{key} with value #{inspect value}") + end) + + Map.merge(acc, config) + + config, acc when is_map(config) -> + Map.merge(acc, config) + end) + + errors = [] + {:ok, config, errors} + end + + defp load_user_home_config do + case xdg_module().read_elixir_ls_config_file("config.json") do + {:ok, file_contents} -> + ConfigParser.load_config(file_contents) + + {:error, :enoent} -> + {:ok, :skip} + + {:error, err} -> + {:error, err} + end + end + + defp xdg_module, do: Application.fetch_env!(:language_server, :xdg_module) +end diff --git a/apps/language_server/lib/language_server/server.ex b/apps/language_server/lib/language_server/server.ex index a0085a649..30ba90446 100644 --- a/apps/language_server/lib/language_server/server.ex +++ b/apps/language_server/lib/language_server/server.ex @@ -17,6 +17,7 @@ defmodule ElixirLS.LanguageServer.Server do use GenServer alias ElixirLS.LanguageServer.{SourceFile, Build, Protocol, JsonRpc, Dialyzer} + alias ElixirLS.LanguageServer.ConfigLoader alias ElixirLS.LanguageServer.Providers.{ Completion, @@ -34,31 +35,37 @@ defmodule ElixirLS.LanguageServer.Server do use Protocol - defstruct [ - :server_instance_id, - :build_ref, - :dialyzer_sup, - :client_capabilities, - :root_uri, - :project_dir, - :settings, - build_diagnostics: [], - dialyzer_diagnostics: [], - needs_build?: false, - load_all_modules?: false, - build_running?: false, - analysis_ready?: false, - received_shutdown?: false, - requests: %{}, - # Tracks source files that are currently open in the editor - source_files: %{}, - awaiting_contracts: [] - ] + @default_config_timeout_ms 5_000 + + defmodule State do + defstruct [ + :server_instance_id, + :build_ref, + :dialyzer_sup, + :client_capabilities, + :root_uri, + :project_dir, + settings: %{}, + build_diagnostics: [], + dialyzer_diagnostics: [], + needs_build?: false, + load_all_modules?: false, + build_running?: false, + analysis_ready?: false, + received_shutdown?: false, + requests: %{}, + # Tracks source files that are currently open in the editor + source_files: %{}, + awaiting_contracts: [], + default_config_timeout_ms: nil + ] + end ## Client API - def start_link(name \\ nil) do - GenServer.start_link(__MODULE__, :ok, name: name) + def start_link(opts \\ []) do + name = Keyword.get(opts, :name, nil) + GenServer.start_link(__MODULE__, opts, name: name) end def receive_packet(server \\ __MODULE__, packet) do @@ -84,8 +91,11 @@ defmodule ElixirLS.LanguageServer.Server do ## Server Callbacks @impl GenServer - def init(:ok) do - {:ok, %__MODULE__{}} + def init(opts) do + default_config_timeout_ms = + Keyword.get(opts, :default_config_timeout_ms, @default_config_timeout_ms) + + {:ok, %State{default_config_timeout_ms: default_config_timeout_ms}} end @impl GenServer @@ -158,14 +168,14 @@ defmodule ElixirLS.LanguageServer.Server do def handle_info(:default_config, state) do state = case state do - %{settings: nil} -> + %{settings: settings} when settings == %{} -> JsonRpc.show_message( :info, "Did not receive workspace/didChangeConfiguration notification after 5 seconds. " <> - "Using default settings." + "Using default settings. (This is expected if you have no editor configuration)" ) - set_settings(state, %{}) + set_settings(state, %{}, %{}) _ -> state @@ -235,18 +245,18 @@ defmodule ElixirLS.LanguageServer.Server do # the `projectDir` or `mixEnv` settings. If the settings don't match the format expected, leave # settings unchanged or set default settings if this is the first request. defp handle_notification(did_change_configuration(changed_settings), state) do - prev_settings = state.settings || %{} - - new_settings = - case changed_settings do - %{"elixirLS" => changed_settings} when is_map(changed_settings) -> - Map.merge(prev_settings, changed_settings) - - _ -> - prev_settings - end + case changed_settings do + %{"elixirLS" => changed_settings} when is_map(changed_settings) -> + prev_settings = state.settings + set_settings(state, prev_settings, changed_settings) - set_settings(state, new_settings) + _ -> + if state.settings == %{} do + set_settings(state, %{}, %{}) + else + state + end + end end defp handle_notification(notification("exit"), state) do @@ -367,8 +377,8 @@ defmodule ElixirLS.LanguageServer.Server do server_instance_id: server_instance_id } - # If we don't receive workspace/didChangeConfiguration for 5 seconds, use default settings - Process.send_after(self(), :default_config, 5000) + # If we don't receive workspace/didChangeConfiguration before timeout, use default settings + Process.send_after(self(), :default_config, state.default_config_timeout_ms) # Explicitly request file watchers from the client if supported supports_dynamic = @@ -530,7 +540,7 @@ defmodule ElixirLS.LanguageServer.Server do end defp handle_request(code_lens_req(_id, uri), state) do - if dialyzer_enabled?(state) and state.settings["suggestSpecs"] != false do + if dialyzer_enabled?(state) and state.settings.suggest_specs != false do {:async, fn -> CodeLens.code_lens(state.server_instance_id, uri, state.source_files[uri].text) end, state} @@ -599,7 +609,7 @@ defmodule ElixirLS.LanguageServer.Server do defp trigger_build(state) do if build_enabled?(state) and not state.build_running? do - fetch_deps? = Map.get(state.settings || %{}, "fetchDeps", true) + fetch_deps? = state.settings.fetch_deps {_pid, build_ref} = Build.build(self(), state.project_dir, @@ -607,7 +617,7 @@ defmodule ElixirLS.LanguageServer.Server do load_all_modules?: state.load_all_modules? ) - %__MODULE__{ + %State{ state | build_ref: build_ref, needs_build?: false, @@ -616,25 +626,26 @@ defmodule ElixirLS.LanguageServer.Server do load_all_modules?: false } else - %__MODULE__{state | needs_build?: true, analysis_ready?: false} + %State{state | needs_build?: true, analysis_ready?: false} end end defp dialyze(state) do warn_opts = - (state.settings["dialyzerWarnOpts"] || []) + state.settings.dialyzer_warn_opts |> Enum.map(&String.to_atom/1) - if dialyzer_enabled?(state), - do: Dialyzer.analyze(state.build_ref, warn_opts, dialyzer_default_format(state)) + if dialyzer_enabled?(state) do + Dialyzer.analyze( + state.build_ref, + warn_opts, + state.settings.dialyzer_format + ) + end state end - defp dialyzer_default_format(state) do - state.settings["dialyzerFormat"] || "dialyxir_long" - end - defp handle_build_result(status, diagnostics, state) do old_diagnostics = state.build_diagnostics ++ state.dialyzer_diagnostics state = put_in(state.build_diagnostics, diagnostics) @@ -747,23 +758,44 @@ defmodule ElixirLS.LanguageServer.Server do :ok end - defp set_settings(state, settings) do - enable_dialyzer = - Dialyzer.check_support() == :ok && Map.get(settings, "dialyzerEnabled", true) + defp notify_configuration_errors(errors) do + Enum.each(errors, fn + {:error, {:unrecognized_configuration_key, key, value}} -> + JsonRpc.show_message( + :warning, + "Invalid configuration setting found. Unrecognized key `#{key}` with value #{ + inspect(value) + }" + ) + end) + end + + defp set_settings(state, prev_settings, settings_map) do + {:ok, config, errors} = ConfigLoader.load(prev_settings, settings_map) + notify_configuration_errors(errors) + + mix_target = config.mix_target + + %{ + dialyzer_enabled: dialyzer_enabled, + mix_env: mix_env, + project_dir: project_dir + } = config + + dialyzer_supported = Dialyzer.check_support() - mix_env = Map.get(settings, "mixEnv", "test") - mix_target = Map.get(settings, "mixTarget") - project_dir = Map.get(settings, "projectDir") + config = %{config | dialyzer_enabled: dialyzer_supported && config.dialyzer_enabled} + config = Map.merge(state.settings, config) state = state |> set_mix_env(mix_env) |> maybe_set_mix_target(mix_target) |> set_project_dir(project_dir) - |> set_dialyzer_enabled(enable_dialyzer) + |> set_dialyzer_enabled(dialyzer_enabled) state = create_gitignore(state) - trigger_build(%{state | settings: settings}) + trigger_build(%{state | settings: config}) end defp set_dialyzer_enabled(state, enable_dialyzer) do @@ -782,7 +814,7 @@ defmodule ElixirLS.LanguageServer.Server do end defp set_mix_env(state, env) do - prev_env = state.settings["mixEnv"] + prev_env = state.settings[:mix_env] if is_nil(prev_env) or env == prev_env do Mix.env(String.to_atom(env)) @@ -811,7 +843,7 @@ defmodule ElixirLS.LanguageServer.Server do defp set_mix_target(state, target) do target = target || "host" - prev_target = state.settings["mixTarget"] + prev_target = get_in(state.settings, [:mix_target]) if is_nil(prev_target) or target == prev_target do # We've already checked for Elixir >= 1.8.0 by this point diff --git a/apps/language_server/test/dialyzer_test.exs b/apps/language_server/test/dialyzer_test.exs index a11d9a3f8..afc14ffcb 100644 --- a/apps/language_server/test/dialyzer_test.exs +++ b/apps/language_server/test/dialyzer_test.exs @@ -15,6 +15,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do setup do server = ElixirLS.LanguageServer.Test.ServerTestHelpers.start_server() + ElixirLS.LanguageServer.Test.ServerTestHelpers.assert_server_does_not_crash(server) {:ok, %{server: server}} end diff --git a/apps/language_server/test/language_server/config_loader_test.exs b/apps/language_server/test/language_server/config_loader_test.exs new file mode 100644 index 000000000..79c426dca --- /dev/null +++ b/apps/language_server/test/language_server/config_loader_test.exs @@ -0,0 +1,18 @@ +defmodule ElixirLS.LanguageServer.ConfigLoaderTest do + use ExUnit.Case + + alias ElixirLS.LanguageServer.ConfigLoader + + describe "load/2" do + test "dialyzer enabled is respected" do + changed_settings = %{ + "dialyzerEnabled" => false + } + + prev_settings = %{} + + assert {:ok, %{dialyzer_enabled: false}, _} = + ConfigLoader.load(prev_settings, changed_settings) + end + end +end diff --git a/apps/language_server/test/server_test.exs b/apps/language_server/test/server_test.exs index c928ff0c5..15dfeb16b 100644 --- a/apps/language_server/test/server_test.exs +++ b/apps/language_server/test/server_test.exs @@ -22,7 +22,9 @@ defmodule ElixirLS.LanguageServer.ServerTest do setup context do unless context[:skip_server] do - server = ElixirLS.LanguageServer.Test.ServerTestHelpers.start_server() + server_opts = Map.get(context, :server_opts, []) + server = ElixirLS.LanguageServer.Test.ServerTestHelpers.start_server(server_opts) + ElixirLS.LanguageServer.Test.ServerTestHelpers.assert_server_does_not_crash(server) {:ok, %{server: server}} else @@ -30,6 +32,57 @@ defmodule ElixirLS.LanguageServer.ServerTest do end end + test "textDocument/didChangeConfiguration with valid configuration", %{server: server} do + json_settings = %{"elixirLS" => %{"dialyzerEnabled" => false}} + Server.receive_packet(server, did_change_configuration(json_settings)) + + state = :sys.get_state(server) + assert state.settings.dialyzer_enabled == false + + json_settings = %{"elixirLS" => %{"dialyzerEnabled" => true}} + Server.receive_packet(server, did_change_configuration(json_settings)) + + state = :sys.get_state(server) + assert state.settings.dialyzer_enabled == true + end + + test "textDocument/didChangeConfiguration uses the defaults", %{server: server} do + json_settings = %{} + Server.receive_packet(server, did_change_configuration(json_settings)) + + state = :sys.get_state(server) + assert state.settings.dialyzer_enabled == true + assert state.settings.dialyzer_format == "dialyxir_long" + end + + test "textDocument/didChangeConfiguration with invalid configuration", %{server: server} do + json_settings = %{"elixirLS" => %{"invalidConfig" => "abc"}} + Server.receive_packet(server, did_change_configuration(json_settings)) + + assert_receive %{ + "method" => "window/logMessage", + "params" => %{ + "message" => "Invalid configuration key: " <> error_details, + "type" => 2 + } + } + + assert error_details == "invalidConfig with value \"abc\"" + + _ = :sys.get_state(server) + end + + @tag server_opts: [default_config_timeout_ms: 10] + test "uses the default settings if textDocument/didChangeConfiguration with invalid configuration not called", + %{server: server} do + Server.receive_packet(server, initialize_req(1, root_uri(), %{})) + Process.sleep(100) + + state = :sys.get_state(server) + assert state.settings.dialyzer_enabled == true + assert state.settings.dialyzer_format == "dialyxir_long" + end + test "textDocument/didChange when the client hasn't claimed ownership with textDocument/didOpen", %{server: server} do uri = "file:///file.ex" @@ -447,7 +500,7 @@ defmodule ElixirLS.LanguageServer.ServerTest do end defp with_new_server(func) do - server = start_supervised!({Server, nil}) + server = start_supervised!({Server, []}) packet_capture = start_supervised!({PacketCapture, self()}) Process.group_leader(server, packet_capture) diff --git a/apps/language_server/test/support/server_test_helpers.ex b/apps/language_server/test/support/server_test_helpers.ex index 04a3a4f31..d37562790 100644 --- a/apps/language_server/test/support/server_test_helpers.ex +++ b/apps/language_server/test/support/server_test_helpers.ex @@ -6,10 +6,10 @@ defmodule ElixirLS.LanguageServer.Test.ServerTestHelpers do alias ElixirLS.LanguageServer.Providers.WorkspaceSymbols alias ElixirLS.Utils.PacketCapture - def start_server do + def start_server(opts \\ []) do packet_capture = start_supervised!({PacketCapture, self()}) - server = start_supervised!({Server, nil}) + server = start_supervised!({Server, opts}) Process.group_leader(server, packet_capture) json_rpc = start_supervised!({JsonRpc, name: JsonRpc}) @@ -20,4 +20,24 @@ defmodule ElixirLS.LanguageServer.Test.ServerTestHelpers do server end + + def assert_server_does_not_crash(server) do + Task.start_link(fn -> + ref = Process.monitor(server) + + receive do + {:DOWN, ^ref, :process, _object, :normal} -> + nil + + {:DOWN, ^ref, :process, _object, reason} -> + ExUnit.Assertions.flunk(""" + Server should not crash. + + Crashed with: + #{inspect(reason, pretty: true)} + """) + end + end) + + end end diff --git a/config/config.exs b/config/config.exs index f507a8725..5fdc77aa9 100644 --- a/config/config.exs +++ b/config/config.exs @@ -15,3 +15,11 @@ use Mix.Config # level: :info, # format: "$date $time [$level] $metadata$message\n", # metadata: [:user_id] + +case Mix.env() do + :test -> + config :language_server, xdg_module: ElixirLS.Utils.Test.MockXDG + + _ -> + config :language_server, xdg_module: ElixirLS.Utils.XDG +end From 82d42bdc46024aa7f4c7c8335e0e028d87d0dd16 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Mon, 3 Aug 2020 21:39:03 -1000 Subject: [PATCH 2/5] Fix test --- apps/language_server/test/server_test.exs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/apps/language_server/test/server_test.exs b/apps/language_server/test/server_test.exs index 15dfeb16b..ec266431f 100644 --- a/apps/language_server/test/server_test.exs +++ b/apps/language_server/test/server_test.exs @@ -72,15 +72,18 @@ defmodule ElixirLS.LanguageServer.ServerTest do _ = :sys.get_state(server) end - @tag server_opts: [default_config_timeout_ms: 10] + @tag server_opts: [default_config_timeout_ms: 100] test "uses the default settings if textDocument/didChangeConfiguration with invalid configuration not called", %{server: server} do - Server.receive_packet(server, initialize_req(1, root_uri(), %{})) - Process.sleep(100) + in_fixture(__DIR__, "references", fn -> + Server.receive_packet(server, initialize_req(1, root_uri(), %{})) + Server.receive_packet(server, notification("initialized")) + Process.sleep(1000) - state = :sys.get_state(server) - assert state.settings.dialyzer_enabled == true - assert state.settings.dialyzer_format == "dialyxir_long" + state = :sys.get_state(server) + assert state.settings.dialyzer_enabled == true + assert state.settings.dialyzer_format == "dialyxir_long" + end) end test "textDocument/didChange when the client hasn't claimed ownership with textDocument/didOpen", From 1acf1270b4a16740a7b3b860db3055e9ed7fec0f Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Tue, 4 Aug 2020 07:13:59 -1000 Subject: [PATCH 3/5] Fix comment-style for configuration files --- apps/elixir_ls_utils/lib/config_parser.ex | 2 +- apps/elixir_ls_utils/test/support/example_config_file.jsonc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/elixir_ls_utils/lib/config_parser.ex b/apps/elixir_ls_utils/lib/config_parser.ex index 63d1faef1..5362ec1ce 100644 --- a/apps/elixir_ls_utils/lib/config_parser.ex +++ b/apps/elixir_ls_utils/lib/config_parser.ex @@ -160,7 +160,7 @@ defmodule ElixirLS.Utils.ConfigParser do |> String.split(["\n", "\r", "\r\n"], trim: true) |> Enum.map(&String.trim/1) # Ignore json comments - |> Enum.reject(&String.starts_with?(&1, "#")) + |> Enum.reject(&String.starts_with?(&1, "//")) |> Enum.join() |> JasonVendored.decode() |> case do diff --git a/apps/elixir_ls_utils/test/support/example_config_file.jsonc b/apps/elixir_ls_utils/test/support/example_config_file.jsonc index a6e5e01cb..f7e6c2f41 100644 --- a/apps/elixir_ls_utils/test/support/example_config_file.jsonc +++ b/apps/elixir_ls_utils/test/support/example_config_file.jsonc @@ -1,6 +1,6 @@ -# This is the configuration file for ElixirLS +// This is the configuration file for ElixirLS { - # Disable dialyzer + // Disable dialyzer "dialyzerEnabled": false, "fetchDeps": false, "mixEnv": "dev" From b4ae33ee10ad70522bb8da512bbdfed96722f13e Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Tue, 4 Aug 2020 07:24:36 -1000 Subject: [PATCH 4/5] Fix comments in test also --- apps/elixir_ls_utils/test/config_parser_test.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/elixir_ls_utils/test/config_parser_test.exs b/apps/elixir_ls_utils/test/config_parser_test.exs index 664836658..eeaabd52c 100644 --- a/apps/elixir_ls_utils/test/config_parser_test.exs +++ b/apps/elixir_ls_utils/test/config_parser_test.exs @@ -81,9 +81,9 @@ defmodule ElixirLS.Utils.ConfigParserTest do test "load_config/1 ignores lines with comments" do config_contents = """ - # This is the configuration file for ElixirLS + // This is the configuration file for ElixirLS { - # Disable dialyzer + // Disable dialyzer "dialyzerEnabled": false } """ From f74d0577da6881f436f7f5980fee36adc8cdfacc Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Tue, 4 Aug 2020 07:47:50 -1000 Subject: [PATCH 5/5] Support project config file --- README.md | 2 +- apps/language_server/lib/language_server.ex | 3 ++- .../lib/language_server/config_loader.ex | 26 ++++++++++++++++--- .../lib/language_server/server.ex | 16 +++++++++++- 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 9f44a3e66..c58e47a68 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ Configuration file location and precedence: ### Configuration File Contents -The configuration file can include comments (e.g. lines starting with `//` are ignored) +The configuration files are json but can include line-based comments (e.g. lines starting with `//` are ignored, although multi-line comments `/**/` are not supported) Configuration keys: - `dialyzerEnabled`: Run ElixirLS's rapid Dialyzer when code is saved diff --git a/apps/language_server/lib/language_server.ex b/apps/language_server/lib/language_server.ex index f913823ef..14b3e0d45 100644 --- a/apps/language_server/lib/language_server.ex +++ b/apps/language_server/lib/language_server.ex @@ -7,7 +7,8 @@ defmodule ElixirLS.LanguageServer do @impl Application def start(_type, _args) do children = [ - {ElixirLS.LanguageServer.Server, ElixirLS.LanguageServer.Server}, + {ElixirLS.LanguageServer.Server, + name: ElixirLS.LanguageServer.Server}, {ElixirLS.LanguageServer.JsonRpc, name: ElixirLS.LanguageServer.JsonRpc}, {ElixirLS.LanguageServer.Providers.WorkspaceSymbols, []} ] diff --git a/apps/language_server/lib/language_server/config_loader.ex b/apps/language_server/lib/language_server/config_loader.ex index 4efc40769..6e9e07a5e 100644 --- a/apps/language_server/lib/language_server/config_loader.ex +++ b/apps/language_server/lib/language_server/config_loader.ex @@ -21,24 +21,32 @@ defmodule ElixirLS.LanguageServer.ConfigLoader do # Can we change this to load also from the workspace root? Note: might need multiple passes for that to work - def load(prev_config, editor_config, opts \\ []) do + def load(project_dir, prev_config, editor_config, opts \\ []) do user_home_config = Keyword.get_lazy(opts, :load_user_home_config, fn -> load_user_home_config() end) + project_config = + Keyword.get_lazy(opts, :project_config, fn -> + load_project_config(project_dir) + end) + default_config = ConfigParser.default_config() editor_config = ConfigParser.parse_config(editor_config) config = - [default_config, prev_config, user_home_config, editor_config] + [default_config, prev_config, user_home_config, project_config, editor_config] |> Enum.reduce(%{}, fn {:ok, :skip}, acc -> acc {:ok, config, errors}, acc when is_map(config) -> Enum.each(errors, fn {:error, {:unrecognized_configuration_key, key, value}} -> - JsonRpc.log_message(:warning, "Invalid configuration key: #{key} with value #{inspect value}") + JsonRpc.log_message( + :warning, + "Invalid configuration key: #{key} with value #{inspect(value)}" + ) end) Map.merge(acc, config) @@ -64,5 +72,15 @@ defmodule ElixirLS.LanguageServer.ConfigLoader do end end - defp xdg_module, do: Application.fetch_env!(:language_server, :xdg_module) + defp load_project_config(nil), do: {:ok, :skip} + + defp load_project_config(project_dir) do + path = Path.join(project_dir, "/.elixir_ls_config.json") + case File.read(path) do + {:ok, file_contents} -> ConfigParser.load_config(file_contents) + {:error, _} -> {:ok, :skip} + end + end + + defp xdg_module, do: Application.get_env(:language_server, :xdg_module, ElixirLS.Utils.XDG) end diff --git a/apps/language_server/lib/language_server/server.ex b/apps/language_server/lib/language_server/server.ex index 30ba90446..6b978a664 100644 --- a/apps/language_server/lib/language_server/server.ex +++ b/apps/language_server/lib/language_server/server.ex @@ -771,7 +771,21 @@ defmodule ElixirLS.LanguageServer.Server do end defp set_settings(state, prev_settings, settings_map) do - {:ok, config, errors} = ConfigLoader.load(prev_settings, settings_map) + root_dir = SourceFile.path_from_uri(state.root_uri) + + project_dir = + case state.project_dir || settings_map["projectDir"] do + nil -> root_dir + project_dir -> Path.join(root_dir, project_dir) + end + + {:ok, config, errors} = + ConfigLoader.load( + project_dir, + prev_settings, + settings_map + ) + notify_configuration_errors(errors) mix_target = config.mix_target