Run Dialyzer on CI (#285)

lukaszsamson · web-flow · commit 2fbb023e75c1 · 2020-06-07T07:59:53.000-10:00
* remove dead code

* add explicit dependancy on Jason

* match on success in functions that may fail

* warn if unable to remove temp file

* warn if unable to read/write protocol messages

* only build test/support in test env

* annotate match against opaque type

* add missing applications

otherwise elixir 1.11 will emit a warning

* add empty ignore file

* add dialyzer config

* run formatter

* run dialyzer on CI
diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs
@@ -0,0 +1 @@
+[]
diff --git a/.formatter.exs b/.formatter.exs
@@ -1,6 +1,6 @@
 [
   inputs: [
-    "mix.exs",
+    "*.exs",
     "config/**/*.exs",
     "apps/*/{config,lib,test}/**/*.{ex,exs}",
     "apps/*/mix.exs"
diff --git a/.travis.yml b/.travis.yml
@@ -9,6 +9,13 @@ script:
       echo "Not checking formatting"
     fi
   - mix test
+  - |
+    if [[ "$RUN_DIALYZER" -eq 1 ]]
+    then
+      mix dialyzer
+    else
+      echo "Not running Dialyzer"
+    fi
 env:
   global:
     - MIX_HOME=$HOME/.mix
@@ -27,4 +34,6 @@ matrix:
       elixir: 1.9.4
     - otp_release: 22.2
       elixir: 1.10.2
-      env: CHECK_FORMATTED=1
+      env:
+        - CHECK_FORMATTED=1
+        - RUN_DIALYZER=1
diff --git a/apps/elixir_ls_debugger/lib/debugger.ex b/apps/elixir_ls_debugger/lib/debugger.ex
@@ -9,7 +9,7 @@ defmodule ElixirLS.Debugger do
   def start(_type, _args) do
     # We don't start this as a worker because if the debugger crashes, we want
     # this process to remain alive to print errors
-    ElixirLS.Debugger.Output.start(ElixirLS.Debugger.Output)
+    {:ok, _pid} = ElixirLS.Debugger.Output.start(ElixirLS.Debugger.Output)
 
     children = [
       {ElixirLS.Debugger.Server, name: ElixirLS.Debugger.Server}
diff --git a/apps/elixir_ls_debugger/lib/debugger/cli.ex b/apps/elixir_ls_debugger/lib/debugger/cli.ex
@@ -5,7 +5,7 @@ defmodule ElixirLS.Debugger.CLI do
   def main do
     WireProtocol.intercept_output(&Output.print/1, &Output.print_err/1)
     Launch.start_mix()
-    Application.ensure_all_started(:elixir_ls_debugger, :permanent)
+    {:ok, _} = Application.ensure_all_started(:elixir_ls_debugger, :permanent)
     IO.puts("Started ElixirLS debugger v#{Launch.debugger_version()}")
     Launch.print_versions()
     warn_if_unsupported_version()
diff --git a/apps/elixir_ls_debugger/lib/debugger/server.ex b/apps/elixir_ls_debugger/lib/debugger/server.ex
@@ -54,7 +54,7 @@ defmodule ElixirLS.Debugger.Server do
 
   @impl GenServer
   def init(opts) do
-    :int.start()
+    {:ok, _pid} = :int.start()
     state = if opts[:output], do: %__MODULE__{output: opts[:output]}, else: %__MODULE__{}
     {:ok, state}
   end
@@ -595,9 +595,9 @@ defmodule ElixirLS.Debugger.Server do
   # test helpers before tests). We save the .beam files to a temporary folder which we add to the
   # code path.
   defp require_files(required_files) do
-    File.rm_rf(@temp_beam_dir)
-    File.mkdir_p(@temp_beam_dir)
-    Code.append_path(Path.expand(@temp_beam_dir))
+    {:ok, _} = File.rm_rf(@temp_beam_dir)
+    :ok = File.mkdir_p(@temp_beam_dir)
+    true = Code.append_path(Path.expand(@temp_beam_dir))
 
     for path <- required_files,
         file <- Path.wildcard(path),
@@ -608,7 +608,7 @@ defmodule ElixirLS.Debugger.Server do
   end
 
   defp save_and_reload(module, beam_bin) do
-    File.write(Path.join(@temp_beam_dir, to_string(module) <> ".beam"), beam_bin)
+    :ok = File.write(Path.join(@temp_beam_dir, to_string(module) <> ".beam"), beam_bin)
     :code.delete(module)
     :int.ni(module)
   end
diff --git a/apps/elixir_ls_debugger/mix.exs b/apps/elixir_ls_debugger/mix.exs
@@ -19,7 +19,7 @@ defmodule ElixirLS.Debugger.Mixfile do
   end
 
   def application do
-    [mod: {ElixirLS.Debugger, []}, extra_applications: [:mix, :logger]]
+    [mod: {ElixirLS.Debugger, []}, extra_applications: [:mix, :logger, :debugger]]
   end
 
   defp deps do
diff --git a/apps/elixir_ls_utils/lib/mix.tasks.elixir_ls.release.ex b/apps/elixir_ls_utils/lib/mix.tasks.elixir_ls.release.ex
@@ -33,7 +33,7 @@ defmodule Mix.Tasks.ElixirLs.Release do
     if opts[:zip] do
       zip_file = to_charlist(Path.expand(opts[:zip]))
       files = Enum.map(File.ls!(destination), &to_charlist/1)
-      :zip.create(zip_file, files, cwd: to_charlist(destination))
+      {:ok, _} = :zip.create(zip_file, files, cwd: to_charlist(destination))
     end
 
     :ok
diff --git a/apps/elixir_ls_utils/lib/packet_stream.ex b/apps/elixir_ls_utils/lib/packet_stream.ex
@@ -4,55 +4,69 @@ defmodule ElixirLS.Utils.PacketStream do
   """
 
   def stream(pid \\ Process.group_leader()) do
-    if is_pid(pid), do: :io.setopts(pid, binary: true, encoding: :latin1)
+    if is_pid(pid) do
+      :ok = :io.setopts(pid, binary: true, encoding: :latin1)
+    end
 
     Stream.resource(
       fn -> :ok end,
       fn _acc ->
         case read_packet(pid) do
           :eof -> {:halt, :ok}
+          {:error, reason} -> {:halt, {:error, reason}}
           packet -> {[packet], :ok}
         end
       end,
-      fn _acc -> :ok end
+      fn
+        :ok ->
+          :ok
+
+        {:error, reason} ->
+          IO.warn("Unable to read from device: #{inspect(reason)}")
+      end
     )
   end
 
   defp read_packet(pid) do
     header = read_header(pid)
 
-    if header == :eof do
-      :eof
-    else
-      read_body(pid, header)
+    case header do
+      :eof -> :eof
+      {:error, reason} -> {:error, reason}
+      header -> read_body(pid, header)
     end
   end
 
   defp read_header(pid, header \\ %{}) do
     line = IO.binread(pid, :line)
 
-    if line == :eof do
-      :eof
-    else
-      line = String.trim(line)
+    case line do
+      :eof ->
+        :eof
 
-      if line == "" do
-        header
-      else
-        [key, value] = String.split(line, ": ")
-        read_header(pid, Map.put(header, key, value))
-      end
+      {:error, reason} ->
+        {:error, reason}
+
+      line ->
+        line = String.trim(line)
+
+        if line == "" do
+          header
+        else
+          [key, value] = String.split(line, ": ")
+          read_header(pid, Map.put(header, key, value))
+        end
     end
   end
 
   defp read_body(pid, header) do
     %{"Content-Length" => content_length_str} = header
     body = IO.binread(pid, String.to_integer(content_length_str))
 
-    if body == :eof do
-      :eof
-    else
-      JasonVendored.decode!(body)
+    case body do
+      :eof -> :eof
+      {:error, reason} -> {:error, reason}
+      body -> JasonVendored.decode!(body)
     end
   end
 end
diff --git a/apps/elixir_ls_utils/lib/wire_protocol.ex b/apps/elixir_ls_utils/lib/wire_protocol.ex
@@ -7,7 +7,14 @@ defmodule ElixirLS.Utils.WireProtocol do
   def send(packet) do
     pid = io_dest()
     body = JasonVendored.encode!(packet) <> "\r\n\r\n"
-    IO.binwrite(pid, "Content-Length: #{byte_size(body)}\r\n\r\n" <> body)
+
+    case IO.binwrite(pid, "Content-Length: #{byte_size(body)}\r\n\r\n" <> body) do
+      :ok ->
+        :ok
+
+      {:error, reason} ->
+        IO.warn("Unable to write to the device: #{inspect(reason)}")
+    end
   end
 
   defp io_dest do
@@ -21,7 +28,7 @@ defmodule ElixirLS.Utils.WireProtocol do
   def intercept_output(print_fn, print_err_fn) do
     raw_user = Process.whereis(:user)
     raw_standard_error = Process.whereis(:standard_error)
-    :io.setopts(raw_user, binary: true, encoding: :latin1)
+    :ok = :io.setopts(raw_user, binary: true, encoding: :latin1)
 
     {:ok, user} = OutputDevice.start_link(raw_user, print_fn)
     {:ok, standard_error} = OutputDevice.start_link(raw_standard_error, print_err_fn)
diff --git a/apps/elixir_ls_utils/mix.exs b/apps/elixir_ls_utils/mix.exs
@@ -8,7 +8,7 @@ defmodule ElixirLS.Utils.Mixfile do
       build_path: "../../_build",
       config_path: "../../config/config.exs",
       deps_path: "../../deps",
-      elixirc_paths: ["lib", "test/support"],
+      elixirc_paths: elixirc_paths(Mix.env()),
       lockfile: "../../mix.lock",
       elixir: ">= 1.7.0",
       build_embedded: false,
@@ -30,4 +30,7 @@ defmodule ElixirLS.Utils.Mixfile do
       {:mix_task_archive_deps, github: "JakeBecker/mix_task_archive_deps"}
     ]
   end
+
+  defp elixirc_paths(:test), do: ["lib", "test/support"]
+  defp elixirc_paths(_), do: ["lib"]
 end
diff --git a/apps/language_server/lib/language_server/cli.ex b/apps/language_server/lib/language_server/cli.ex
@@ -6,7 +6,7 @@ defmodule ElixirLS.LanguageServer.CLI do
     WireProtocol.intercept_output(&JsonRpc.print/1, &JsonRpc.print_err/1)
     Launch.start_mix()
 
-    Application.ensure_all_started(:language_server, :temporary)
+    {:ok, _} = Application.ensure_all_started(:language_server, :temporary)
     IO.puts("Started ElixirLS v#{Launch.language_server_version()}")
     Launch.print_versions()
 
diff --git a/apps/language_server/lib/language_server/diagnostics.ex b/apps/language_server/lib/language_server/diagnostics.ex
@@ -100,10 +100,6 @@ defmodule ElixirLS.LanguageServer.Diagnostics do
     end
   end
 
-  defp file_path(nil, _root_path) do
-    {:error, :file_not_found}
-  end
-
   defp file_path(file, root_path) do
     path = Path.join([root_path, file])
 
diff --git a/apps/language_server/lib/language_server/dialyzer.ex b/apps/language_server/lib/language_server/dialyzer.ex
@@ -330,7 +330,18 @@ defmodule ElixirLS.LanguageServer.Dialyzer do
         |> Stream.run()
 
         for file <- removed_files do
-          File.rm(temp_file_path(root_path, file))
+          path = temp_file_path(root_path, file)
+
+          case File.rm(path) do
+            :ok ->
+              :ok
+
+            {:error, :enoent} ->
+              :ok
+
+            {:error, reason} ->
+              IO.warn("Unable to remove temporary file #{path}: #{inspect(reason)}")
+          end
         end
 
         temp_modules =
diff --git a/apps/language_server/lib/language_server/dialyzer/manifest.ex b/apps/language_server/lib/language_server/dialyzer/manifest.ex
@@ -54,7 +54,7 @@ defmodule ElixirLS.LanguageServer.Dialyzer.Manifest do
           File.mkdir_p!(Path.dirname(manifest_path))
           tmp_manifest_path = manifest_path <> ".new"
           File.write!(tmp_manifest_path, :erlang.term_to_binary(manifest_data, compressed: 1))
-          File.rename(tmp_manifest_path, manifest_path)
+          :ok = File.rename(tmp_manifest_path, manifest_path)
           File.touch!(manifest_path, timestamp)
         end)
 
@@ -82,13 +82,14 @@ defmodule ElixirLS.LanguageServer.Dialyzer.Manifest do
       exported_types_list
     } = File.read!(manifest_path) |> :erlang.binary_to_term()
 
+    # FIXME: matching against opaque type
     plt(
       info: info,
       types: types,
       contracts: contracts,
       callbacks: callbacks,
       exported_types: exported_types
-    ) = active_plt = :dialyzer_plt.new()
+    ) = active_plt = apply(:dialyzer_plt, :new, [])
 
     for item <- info_list, do: :ets.insert(info, item)
     for item <- types_list, do: :ets.insert(types, item)
@@ -102,7 +103,7 @@ defmodule ElixirLS.LanguageServer.Dialyzer.Manifest do
   end
 
   def load_elixir_plt() do
-    :dialyzer_plt.from_file(to_charlist(elixir_plt_path()))
+    apply(:dialyzer_plt, :from_file, [to_charlist(elixir_plt_path())])
   rescue
     _ -> build_elixir_plt()
   catch
@@ -172,6 +173,7 @@ defmodule ElixirLS.LanguageServer.Dialyzer.Manifest do
   end
 
   defp transfer_plt(active_plt, pid) do
+    # FIXME: matching against opaque type
     plt(
       info: info,
       types: types,
diff --git a/apps/language_server/lib/language_server/providers/workspace_symbols.ex b/apps/language_server/lib/language_server/providers/workspace_symbols.ex
@@ -106,10 +106,11 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do
 
   @impl GenServer
   def handle_call({:query, key, query}, from, state) do
-    Task.start_link(fn ->
-      results = get_results(state, key, query)
-      GenServer.reply(from, results)
-    end)
+    {:ok, _pid} =
+      Task.start_link(fn ->
+        results = get_results(state, key, query)
+        GenServer.reply(from, results)
+      end)
 
     {:noreply, state}
   end
@@ -426,16 +427,19 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do
   defp index_async(key, fun) do
     self = self()
 
-    Task.start_link(fn ->
-      results = fun.()
+    {:ok, _pid} =
+      Task.start_link(fn ->
+        results = fun.()
 
-      send(self, {:indexing_complete, key, results})
+        send(self, {:indexing_complete, key, results})
 
-      JsonRpc.log_message(
-        :info,
-        "[ElixirLS WorkspaceSymbols] #{length(results)} #{key} added to index"
-      )
-    end)
+        JsonRpc.log_message(
+          :info,
+          "[ElixirLS WorkspaceSymbols] #{length(results)} #{key} added to index"
+        )
+      end)
+
+    :ok
   end
 
   @spec get_results(state_t, key_t, String.t()) :: [symbol_information_t]
diff --git a/apps/language_server/mix.exs b/apps/language_server/mix.exs
@@ -21,7 +21,7 @@ defmodule ElixirLS.LanguageServer.Mixfile do
   end
 
   def application do
-    [mod: {ElixirLS.LanguageServer, []}, extra_applications: [:mix, :logger]]
+    [mod: {ElixirLS.LanguageServer, []}, extra_applications: [:mix, :logger, :dialyzer]]
   end
 
   defp deps do
@@ -30,7 +30,8 @@ defmodule ElixirLS.LanguageServer.Mixfile do
       {:elixir_sense, github: "elixir-lsp/elixir_sense"},
       {:forms, "~> 0.0.1"},
       {:erl2ex, github: "dazuma/erl2ex"},
-      {:dialyxir, "~> 1.0.0", runtime: false}
+      {:dialyxir, "~> 1.0.0", runtime: false},
+      {:jason_vendored, github: "elixir-lsp/jason", branch: "vendored"}
     ]
   end
 
diff --git a/mix.exs b/mix.exs
@@ -9,7 +9,18 @@ defmodule ElixirLS.Mixfile do
       start_permanent: Mix.env() == :prod,
       build_per_environment: false,
       deps: deps(),
-      elixir: ">= 1.7.0"
+      elixir: ">= 1.7.0",
+      dialyzer: [
+        plt_add_apps: [:dialyxir],
+        flags: [
+          # enable only to verify error handling
+          # :unmatched_returns,
+          :error_handling,
+          :race_conditions,
+          :unknown,
+          :underspecs
+        ]
+      ]
     ]
   end
 

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`[`
`2`	`2`	`inputs: [`
`3`		`- "mix.exs",`
	`3`	`+ "*.exs",`
`4`	`4`	`"config/*/.exs",`
`5`	`5`	`"apps//{config,lib,test}//.{ex,exs}",`
`6`	`6`	`"apps/*/mix.exs"`