Add --all-warnings option to Elixir compiler (#6224)

* Track Elixir build warnings in the manifest

* Add --all-warnings option to Elixir compiler

* Document "each_warning" option in ParallelCompiler

* Handle old manifest version :v6 when parsing manifest

* Track warnings during compile using a map in the Agent's state

Author: JakeBecker

lib/elixir/lib/kernel/parallel_compiler.ex

@@ -30,6 +30,9 @@ defmodule Kernel.ParallelCompiler do
     * `:each_module` - for each module compiled, invokes the callback passing
       the file, module and the module bytecode
 
+    * `:each_warning` - for each warning, invokes the callback passing
+      the file, line number, and warning message
+
     * `:dest` - the destination directory for the BEAM files. When using `files/2`,
       this information is only used to properly annotate the BEAM files before
       they are loaded into memory. If you want a file to actually be written to
@@ -230,6 +233,12 @@ defmodule Kernel.ParallelCompiler do
         end
         spawn_compilers(state)
 
+      {:warning, file, line, message} ->
+        if callback = Keyword.get(options, :each_warning) do
+          callback.(file, line, message)
+        end
+        wait_for_messages(state)
+
       {:file_compiled, child_pid, file, :ok} ->
         discard_down(child_pid)
 

lib/elixir/src/elixir_errors.erl

@@ -12,6 +12,12 @@
 warn(none, File, Warning) ->
   warn(0, File, Warning);
 warn(Line, File, Warning) when is_integer(Line), is_binary(File) ->
+  CompilerPid = get(elixir_compiler_pid),
+  if
+    CompilerPid =/= undefined ->
+      CompilerPid ! {warning, File, Line, Warning};
+    true -> ok
+  end,
   warn([Warning, "\n  ", file_format(Line, File), $\n]).
 
 -spec warn(unicode:chardata()) -> ok.

lib/mix/lib/mix/compilers/elixir.ex

@@ -1,7 +1,7 @@
 defmodule Mix.Compilers.Elixir do
   @moduledoc false
 
-  @manifest_vsn :v6
+  @manifest_vsn :v7
 
   import Record
 
@@ -13,7 +13,8 @@ defmodule Mix.Compilers.Elixir do
     runtime_references: [],
     compile_dispatches: [],
     runtime_dispatches: [],
-    external: []
+    external: [],
+    warnings: []
   ]
 
   @doc """
@@ -78,6 +79,8 @@ defmodule Mix.Compilers.Elixir do
     stale   = changed -- removed
     sources = update_stale_sources(all_sources, removed, changed)
 
+    if opts[:all_warnings], do: show_warnings(sources)
+
     cond do
       stale != [] ->
         compile_manifest(manifest, exts, modules, sources, stale, dest, timestamp, opts)
@@ -149,18 +152,19 @@ defmodule Mix.Compilers.Elixir do
 
     # Starts a server responsible for keeping track which files
     # were compiled and the dependencies between them.
-    {:ok, pid} = Agent.start_link(fn -> {modules, sources} end)
+    {:ok, pid} = Agent.start_link(fn -> {modules, sources, %{}} end)
     long_compilation_threshold = opts[:long_compilation_threshold] || 10
 
     try do
       _ = Kernel.ParallelCompiler.files stale,
             [each_module: &each_module(pid, cwd, &1, &2, &3),
              each_long_compilation: &each_long_compilation(&1, long_compilation_threshold),
+             each_warning: &each_warning(pid, cwd, &1, &2, &3),
              long_compilation_threshold: long_compilation_threshold,
              dest: dest] ++ extra
-      Agent.cast pid, fn {modules, sources} ->
+      Agent.cast pid, fn {modules, sources, warnings} ->
         write_manifest(manifest, modules, sources, dest, timestamp)
-        {modules, sources}
+        {modules, sources, warnings}
       end
     after
       Agent.stop(pid, :normal, :infinity)
@@ -201,7 +205,7 @@ defmodule Mix.Compilers.Elixir do
     source   = Path.relative_to(source, cwd)
     external = get_external_resources(module, cwd)
 
-    Agent.cast pid, fn {modules, sources} ->
+    Agent.cast pid, fn {modules, sources, warnings} ->
       source_external = case List.keyfind(sources, source, source(:source)) do
         source(external: old_external) -> external ++ old_external
         nil -> external
@@ -227,12 +231,13 @@ defmodule Mix.Compilers.Elixir do
         runtime_references: runtime_references,
         compile_dispatches: compile_dispatches,
         runtime_dispatches: runtime_dispatches,
-        external: source_external
+        external: source_external,
+        warnings: Map.get(warnings, source, [])
       )
 
       modules = List.keystore(modules, module, module(:module), new_module)
       sources = List.keystore(sources, source, source(:source), new_source)
-      {modules, sources}
+      {modules, sources, warnings}
     end
   end
 
@@ -262,6 +267,15 @@ defmodule Mix.Compilers.Elixir do
     Mix.shell.info "Compiling #{source} (it's taking more than #{threshold}s)"
   end
 
+  defp each_warning(pid, cwd, file, line, message) do
+    Agent.cast pid, fn {modules, sources, warnings} ->
+      source_path = Path.relative_to(file, cwd)
+      warning = {line, message}
+      warnings = Map.update(warnings, source_path, [warning], &([warning | &1]))
+      {modules, sources, warnings}
+    end
+  end
+
   ## Resolution
 
   defp update_stale_sources(sources, removed, changed) do
@@ -350,6 +364,15 @@ defmodule Mix.Compilers.Elixir do
     _ = :code.delete(module)
   end
 
+  defp show_warnings(sources) do
+    for source(source: source, warnings: warnings) <- sources do
+      file = Path.join(File.cwd!, source)
+      for {line, message} <- warnings do
+        :elixir_errors.warn(line, file, message)
+      end
+    end
+  end
+
   ## Manifest handling
 
   # Similar to read_manifest, but supports data migration.
@@ -361,7 +384,7 @@ defmodule Mix.Compilers.Elixir do
     else
       [@manifest_vsn | data] ->
         split_manifest(data, compile_path)
-      [v | data] when v in [:v4, :v5] ->
+      [v | data] when v in [:v4, :v5, :v6] ->
         for module(beam: beam) <- data, do: File.rm(Path.join(compile_path, beam))
         {[], []}
       _ ->

lib/mix/lib/mix/tasks/compile.elixir.ex

@@ -25,6 +25,7 @@ defmodule Mix.Tasks.Compile.Elixir do
       return a non-zero exit code
     * `--long-compilation-threshold N` - sets the "long compilation" threshold
       (in seconds) to `N` (see the docs for `Kernel.ParallelCompiler.files/2`)
+    * `--all-warnings` - prints warnings even from files that do not need to be recompiled
 
   ## Configuration
 
@@ -41,7 +42,8 @@ defmodule Mix.Tasks.Compile.Elixir do
 
   @switches [force: :boolean, docs: :boolean, warnings_as_errors: :boolean,
              ignore_module_conflict: :boolean, debug_info: :boolean,
-             verbose: :boolean, long_compilation_threshold: :integer]
+             verbose: :boolean, long_compilation_threshold: :integer,
+             all_warnings: :boolean]
 
   @doc """
   Runs this task.

lib/mix/test/mix/tasks/compile.elixir_test.exs

@@ -331,4 +331,35 @@ defmodule Mix.Tasks.Compile.ElixirTest do
       refute_received {:mix_shell, :info, ["Compiled lib/b.ex"]}
     end
   end
+
+  test "prints warnings from non-stale files with --all-warnings" do
+    in_fixture "no_mixfile", fn ->
+      File.write!("lib/a.ex", """
+      defmodule A do
+        def my_fn(unused), do: :ok
+      end
+      """)
+
+      # First compilation should print unused variable warning
+      import ExUnit.CaptureIO
+      output = capture_io(:standard_error, fn ->
+        Mix.Tasks.Compile.Elixir.run([]) == :ok
+      end)
+
+      # Should also print warning
+      assert capture_io(:standard_error, fn ->
+        Mix.Tasks.Compile.Elixir.run(["--all-warnings"])
+      end) == output
+
+      # Should not print warning once fixed
+      File.write!("lib/a.ex", """
+      defmodule A do
+        def my_fn(_unused), do: :ok
+      end
+      """)
+      assert capture_io(:standard_error, fn ->
+        Mix.Tasks.Compile.Elixir.run(["--all-warnings"])
+      end) == ""
+    end
+  end
 end