From 70d52dcc16a2532b181da60723f9d0d18384505b Mon Sep 17 00:00:00 2001 From: Mitchell Hanberg Date: Wed, 16 Aug 2023 23:16:18 -0400 Subject: [PATCH] feat(extension): credo (#163) Proves Credo diagnostics for projects that include Credo. Closes #16 TODO: Code actions --- lib/next_ls.ex | 28 ++- lib/next_ls/db.ex | 6 + lib/next_ls/extensions/credo_extension.ex | 160 ++++++++++++++++++ lib/next_ls/extensions/elixir_extension.ex | 4 + lib/next_ls/runtime.ex | 4 +- priv/monkey/_next_ls_private_credo.ex | 9 + test/next_ls/dependency_test.exs | 5 - .../extensions/credo_extension_test.exs | 109 ++++++++++++ test/next_ls/references_test.exs | 4 +- test/next_ls/workspaces_test.exs | 4 +- test/support/utils.ex | 2 +- 11 files changed, 318 insertions(+), 17 deletions(-) create mode 100644 lib/next_ls/extensions/credo_extension.ex create mode 100644 priv/monkey/_next_ls_private_credo.ex create mode 100644 test/next_ls/extensions/credo_extension_test.exs diff --git a/lib/next_ls.ex b/lib/next_ls.ex index 13448355..4b9f1cf0 100644 --- a/lib/next_ls.ex +++ b/lib/next_ls.ex @@ -63,7 +63,8 @@ defmodule NextLS do dynamic_supervisor = Keyword.fetch!(args, :dynamic_supervisor) registry = Keyword.fetch!(args, :registry) - extensions = Keyword.get(args, :extensions, [NextLS.ElixirExtension]) + + extensions = Keyword.get(args, :extensions, [NextLS.ElixirExtension, NextLS.CredoExtension]) cache = Keyword.fetch!(args, :cache) {:ok, logger} = DynamicSupervisor.start_child(dynamic_supervisor, {NextLS.Logger, lsp: lsp}) @@ -363,7 +364,11 @@ defmodule NextLS do {:ok, _} = DynamicSupervisor.start_child( lsp.assigns.dynamic_supervisor, - {extension, cache: lsp.assigns.cache, registry: lsp.assigns.registry, publisher: self()} + {extension, + cache: lsp.assigns.cache, + registry: lsp.assigns.registry, + publisher: self(), + task_supervisor: lsp.assigns.runtime_task_supervisor} ) end @@ -411,7 +416,14 @@ defmodule NextLS do if status == :ready do Progress.stop(lsp, token, "NextLS runtime for folder #{name} has initialized!") GenLSP.log(lsp, "[NextLS] Runtime for folder #{name} is ready...") - send(parent, {:runtime_ready, name, self()}) + + msg = {:runtime_ready, name, self()} + + dispatch(lsp.assigns.registry, :extensions, fn entries -> + for {pid, _} <- entries, do: send(pid, msg) + end) + + send(parent, msg) else Progress.stop(lsp, token) GenLSP.error(lsp, "[NextLS] Runtime for folder #{name} failed to initialize") @@ -526,7 +538,13 @@ defmodule NextLS do if status == :ready do Progress.stop(lsp, token, "NextLS runtime for folder #{name} has initialized!") GenLSP.log(lsp, "[NextLS] Runtime for folder #{name} is ready...") - send(parent, {:runtime_ready, name, self()}) + msg = {:runtime_ready, name, self()} + + dispatch(lsp.assigns.registry, :extensions, fn entries -> + for {pid, _} <- entries, do: send(pid, msg) + end) + + send(parent, msg) else Progress.stop(lsp, token) GenLSP.error(lsp, "[NextLS] Runtime for folder #{name} failed to initialize") @@ -596,8 +614,6 @@ defmodule NextLS do end def handle_info(:publish, lsp) do - GenLSP.log(lsp, "[NextLS] Compiled!") - all = for {_namespace, cache} <- DiagnosticCache.get(lsp.assigns.cache), {file, diagnostics} <- cache, reduce: %{} do d -> Map.update(d, file, diagnostics, fn value -> value ++ diagnostics end) diff --git a/lib/next_ls/db.ex b/lib/next_ls/db.ex index 6d4664c1..749069b6 100644 --- a/lib/next_ls/db.ex +++ b/lib/next_ls/db.ex @@ -162,6 +162,12 @@ defmodule NextLS.DB do is_atom(arg) and String.starts_with?(to_string(arg), "Elixir.") -> Macro.to_string(arg) + arg in [nil, :undefined] -> + arg + + is_atom(arg) -> + to_string(arg) + true -> arg end diff --git a/lib/next_ls/extensions/credo_extension.ex b/lib/next_ls/extensions/credo_extension.ex new file mode 100644 index 00000000..b0662eb9 --- /dev/null +++ b/lib/next_ls/extensions/credo_extension.ex @@ -0,0 +1,160 @@ +defmodule NextLS.CredoExtension do + @moduledoc false + use GenServer + + alias GenLSP.Enumerations.DiagnosticSeverity + alias GenLSP.Structures.CodeDescription + alias GenLSP.Structures.Diagnostic + alias GenLSP.Structures.Position + alias GenLSP.Structures.Range + alias NextLS.DiagnosticCache + alias NextLS.Runtime + + def start_link(args) do + GenServer.start_link( + __MODULE__, + Keyword.take(args, [:cache, :registry, :publisher, :task_supervisor]), + Keyword.take(args, [:name]) + ) + end + + @impl GenServer + def init(args) do + cache = Keyword.fetch!(args, :cache) + registry = Keyword.fetch!(args, :registry) + publisher = Keyword.fetch!(args, :publisher) + task_supervisor = Keyword.fetch!(args, :task_supervisor) + + Registry.register(registry, :extensions, :credo) + + {:ok, + %{ + runtimes: Map.new(), + cache: cache, + registry: registry, + task_supervisor: task_supervisor, + publisher: publisher, + refresh_refs: Map.new() + }} + end + + @impl GenServer + + def handle_info({:runtime_ready, _, _}, state), do: {:noreply, state} + + def handle_info({:compiler, _diagnostics}, state) do + {state, refresh_refs} = + dispatch(state.registry, :runtimes, fn entries -> + # loop over runtimes + for {runtime, %{path: path}} <- entries, reduce: {state, %{}} do + {state, refs} -> + # determine the existence of Credo and memoize the result + state = + if not Map.has_key?(state.runtimes, runtime) do + case Runtime.call(runtime, {Code, :ensure_loaded?, [Credo]}) do + {:ok, true} -> + :next_ls + |> :code.priv_dir() + |> Path.join("monkey/_next_ls_private_credo.ex") + |> then(&Runtime.call(runtime, {Code, :compile_file, [&1]})) + + Runtime.call(runtime, {Application, :ensure_all_started, [:credo]}) + Runtime.call(runtime, {GenServer, :call, [Credo.CLI.Output.Shell, {:suppress_output, true}]}) + + put_in(state.runtimes[runtime], true) + + _ -> + state + end + else + state + end + + # if runtime has Credo + if state.runtimes[runtime] do + namespace = {:credo, path} + DiagnosticCache.clear(state.cache, namespace) + + task = + Task.Supervisor.async_nolink(state.task_supervisor, fn -> + case Runtime.call(runtime, {:_next_ls_private_credo, :issues, [path]}) do + {:ok, issues} -> issues + _error -> [] + end + end) + + {state, Map.put(refs, task.ref, namespace)} + else + {state, refs} + end + end + end) + + send(state.publisher, :publish) + + {:noreply, put_in(state.refresh_refs, refresh_refs)} + end + + def handle_info({ref, issues}, %{refresh_refs: refs} = state) when is_map_key(refs, ref) do + Process.demonitor(ref, [:flush]) + {{:credo, path} = namespace, refs} = Map.pop(refs, ref) + + for issue <- issues do + diagnostic = %Diagnostic{ + range: %Range{ + start: %Position{ + line: issue.line_no - 1, + character: (issue.column || 1) - 1 + }, + end: %Position{ + line: issue.line_no - 1, + character: 999 + } + }, + severity: category_to_severity(issue.category), + data: %{check: issue.check, file: issue.filename}, + source: "credo", + code: Macro.to_string(issue.check), + code_description: %CodeDescription{ + href: "https://hexdocs.pm/credo/#{Macro.to_string(issue.check)}.html" + }, + message: issue.message + } + + DiagnosticCache.put(state.cache, namespace, Path.join(path, issue.filename), diagnostic) + end + + send(state.publisher, :publish) + + {:noreply, put_in(state.refresh_refs, refs)} + end + + def handle_info({:DOWN, ref, :process, _pid, _reason}, %{refresh_refs: refs} = state) when is_map_key(refs, ref) do + {_, refs} = Map.pop(refs, ref) + + {:noreply, put_in(state.refresh_refs, refs)} + end + + defp dispatch(registry, key, callback) do + ref = make_ref() + me = self() + + Registry.dispatch(registry, key, fn entries -> + result = callback.(entries) + + send(me, {ref, result}) + end) + + receive do + {^ref, result} -> result + end + end + + defp category_to_severity(:refactor), do: DiagnosticSeverity.error() + defp category_to_severity(:warning), do: DiagnosticSeverity.warning() + defp category_to_severity(:design), do: DiagnosticSeverity.information() + + defp category_to_severity(:consistency), do: DiagnosticSeverity.information() + + defp category_to_severity(:readability), do: DiagnosticSeverity.information() +end diff --git a/lib/next_ls/extensions/elixir_extension.ex b/lib/next_ls/extensions/elixir_extension.ex index 76a9ed35..24531ab5 100644 --- a/lib/next_ls/extensions/elixir_extension.ex +++ b/lib/next_ls/extensions/elixir_extension.ex @@ -24,6 +24,10 @@ defmodule NextLS.ElixirExtension do end @impl GenServer + def handle_info({:runtime_ready, _path, _pid}, state) do + {:noreply, state} + end + def handle_info({:compiler, diagnostics}, state) when is_list(diagnostics) do DiagnosticCache.clear(state.cache, :elixir) diff --git a/lib/next_ls/runtime.ex b/lib/next_ls/runtime.ex index 88f8b8a1..161c0d44 100644 --- a/lib/next_ls/runtime.ex +++ b/lib/next_ls/runtime.ex @@ -50,7 +50,7 @@ defmodule NextLS.Runtime do on_initialized = Keyword.fetch!(opts, :on_initialized) db = Keyword.fetch!(opts, :db) - Registry.register(registry, :runtimes, %{name: name, uri: uri, db: db}) + Registry.register(registry, :runtimes, %{name: name, uri: uri, path: working_dir, db: db}) pid = cond do @@ -214,6 +214,8 @@ defmodule NextLS.Runtime do for {pid, _} <- entries, do: send(pid, {:compiler, diagnostics}) end) + NextLS.Logger.log(state.logger, "Compiled #{state.name}!") + diagnostics unknown -> diff --git a/priv/monkey/_next_ls_private_credo.ex b/priv/monkey/_next_ls_private_credo.ex new file mode 100644 index 00000000..19958c32 --- /dev/null +++ b/priv/monkey/_next_ls_private_credo.ex @@ -0,0 +1,9 @@ +defmodule :_next_ls_private_credo do + @moduledoc false + + def issues(dir) do + ["--strict", "--all", "--working-dir", dir] + |> Credo.run() + |> Credo.Execution.get_issues() + end +end diff --git a/test/next_ls/dependency_test.exs b/test/next_ls/dependency_test.exs index 74ac1204..b0b85282 100644 --- a/test/next_ls/dependency_test.exs +++ b/test/next_ls/dependency_test.exs @@ -59,10 +59,6 @@ defmodule NextLS.DependencyTest do def bar() do 42 end - - def call_baz() do - Baz.baz() - end end """) @@ -264,7 +260,6 @@ defmodule NextLS.DependencyTest do elixir: "~> 1.10", deps: [ {:bar, path: "../bar"}, - {:baz, path: "../baz"} ] ] end diff --git a/test/next_ls/extensions/credo_extension_test.exs b/test/next_ls/extensions/credo_extension_test.exs new file mode 100644 index 00000000..6eb99461 --- /dev/null +++ b/test/next_ls/extensions/credo_extension_test.exs @@ -0,0 +1,109 @@ +defmodule NextLS.CredoExtensionTest do + # this test installs and compiles credo from scratch everytime it runs + # we need to determine a way to cache this without losing the utility of + # the test. + use ExUnit.Case, async: true + + import GenLSP.Test + import NextLS.Support.Utils + + @moduletag :tmp_dir + @moduletag root_paths: ["my_proj"] + + setup %{tmp_dir: tmp_dir} do + File.mkdir_p!(Path.join(tmp_dir, "my_proj/lib")) + File.write!(Path.join(tmp_dir, "my_proj/mix.exs"), proj_mix_exs()) + + [cwd: tmp_dir] + end + + setup %{cwd: cwd} do + foo = Path.join(cwd, "my_proj/lib/foo.ex") + + File.write!(foo, """ + defmodule Foo do + def run() do + :ok + end + end + """) + + [foo: foo] + end + + setup %{cwd: cwd} do + assert {_, 0} = System.cmd("mix", ["deps.get"], cd: Path.join(cwd, "my_proj")) + :ok + end + + setup :with_lsp + + test "publishes credo diagnostics", %{client: client, foo: foo} = context do + assert :ok == notify(client, %{method: "initialized", jsonrpc: "2.0", params: %{}}) + assert_request(client, "client/registerCapability", fn _params -> nil end) + assert_is_ready(context, "my_proj") + assert_compiled(context, "my_proj") + assert_notification "$/progress", %{"value" => %{"kind" => "end", "message" => "Finished indexing!"}} + + uri = uri(foo) + + assert_notification "textDocument/publishDiagnostics", %{ + "uri" => ^uri, + "diagnostics" => [ + %{ + "code" => "Credo.Check.Readability.ParenthesesOnZeroArityDefs", + "codeDescription" => %{ + "href" => "https://hexdocs.pm/credo/Credo.Check.Readability.ParenthesesOnZeroArityDefs.html" + }, + "data" => %{ + "check" => "Elixir.Credo.Check.Readability.ParenthesesOnZeroArityDefs", + "file" => "lib/foo.ex" + }, + "message" => "Do not use parentheses when defining a function which has no arguments.", + "range" => %{ + "end" => %{"character" => 999, "line" => 1}, + "start" => %{"character" => 0, "line" => 1} + }, + "severity" => 3, + "source" => "credo" + }, + %{ + "code" => "Credo.Check.Readability.ModuleDoc", + "codeDescription" => %{ + "href" => "https://hexdocs.pm/credo/Credo.Check.Readability.ModuleDoc.html" + }, + "data" => %{ + "check" => "Elixir.Credo.Check.Readability.ModuleDoc", + "file" => "lib/foo.ex" + }, + "message" => "Modules should have a @moduledoc tag.", + "range" => %{ + "end" => %{"character" => 999, "line" => 0}, + "start" => %{"character" => 10, "line" => 0} + }, + "severity" => 3, + "source" => "credo" + } + ] + } + end + + defp proj_mix_exs do + """ + defmodule MyProj.MixProject do + use Mix.Project + + def project do + [ + app: :my_proj, + version: "0.1.0", + elixir: "~> 1.10", + deps: [ + {:credo, ">= 0.0.0"} + ] + ] + end + end + """ + end +end diff --git a/test/next_ls/references_test.exs b/test/next_ls/references_test.exs index cf58cf08..be8ef570 100644 --- a/test/next_ls/references_test.exs +++ b/test/next_ls/references_test.exs @@ -44,7 +44,7 @@ defmodule NextLS.ReferencesTest do assert :ok == notify(client, %{method: "initialized", jsonrpc: "2.0", params: %{}}) assert_request(client, "client/registerCapability", fn _params -> nil end) assert_is_ready(context, "my_proj") - assert_notification "window/logMessage", %{"message" => "[NextLS] Compiled!"} + assert_compiled(context, "my_proj") assert_notification "$/progress", %{"value" => %{"kind" => "end", "message" => "Finished indexing!"}} request(client, %{ @@ -78,7 +78,7 @@ defmodule NextLS.ReferencesTest do assert :ok == notify(client, %{method: "initialized", jsonrpc: "2.0", params: %{}}) assert_request(client, "client/registerCapability", fn _params -> nil end) assert_is_ready(context, "my_proj") - assert_notification "window/logMessage", %{"message" => "[NextLS] Compiled!"} + assert_compiled(context, "my_proj") assert_notification "$/progress", %{"value" => %{"kind" => "end", "message" => "Finished indexing!"}} request(client, %{ diff --git a/test/next_ls/workspaces_test.exs b/test/next_ls/workspaces_test.exs index 7401f59c..0adf5227 100644 --- a/test/next_ls/workspaces_test.exs +++ b/test/next_ls/workspaces_test.exs @@ -125,7 +125,7 @@ defmodule NextLS.WorkspacesTest do end) assert_is_ready(context, "proj_one") - assert_notification "window/logMessage", %{"message" => "[NextLS] Compiled!"} + assert_compiled(context, "proj_one") end @tag root_paths: ["proj_one"] @@ -135,7 +135,7 @@ defmodule NextLS.WorkspacesTest do assert_request(client, "client/registerCapability", fn _params -> nil end) assert_is_ready(context, "proj_one") - assert_notification "window/logMessage", %{"message" => "[NextLS] Compiled!"} + assert_compiled(context, "proj_one") notify(client, %{ method: "workspace/didChangeWatchedFiles", diff --git a/test/support/utils.ex b/test/support/utils.ex index bdc76520..8c38a384 100644 --- a/test/support/utils.ex +++ b/test/support/utils.ex @@ -44,7 +44,7 @@ defmodule NextLS.Support.Utils do r_tvisor = start_supervised!(Supervisor.child_spec(Task.Supervisor, id: :two)) rvisor = start_supervised!({DynamicSupervisor, [strategy: :one_for_one]}) start_supervised!({Registry, [keys: :duplicate, name: context.module]}) - extensions = [NextLS.ElixirExtension] + extensions = [NextLS.ElixirExtension, NextLS.CredoExtension] cache = start_supervised!(NextLS.DiagnosticCache) server =