diff --git a/lib/ex_doc/application.ex b/lib/ex_doc/application.ex index a9816c5ee..e4bc48318 100644 --- a/lib/ex_doc/application.ex +++ b/lib/ex_doc/application.ex @@ -17,6 +17,11 @@ defmodule ExDoc.Application do match?("makeup_" <> _, Atom.to_string(app)), do: Application.ensure_all_started(app) - Supervisor.start_link([ExDoc.Refs], strategy: :one_for_one) + children = [ + ExDoc.Refs, + ExDoc.WarningCounter + ] + + Supervisor.start_link(children, strategy: :one_for_one) end end diff --git a/lib/ex_doc/autolink.ex b/lib/ex_doc/autolink.ex index 2dbe7ce93..af0860612 100644 --- a/lib/ex_doc/autolink.ex +++ b/lib/ex_doc/autolink.ex @@ -131,14 +131,12 @@ defmodule ExDoc.Autolink do end defp warn(message, {file, line}, id) do - warning = IO.ANSI.format([:yellow, "warning: ", :reset]) - stacktrace = " #{file}" <> if(line, do: ":#{line}", else: "") <> if(id, do: ": #{id}", else: "") - IO.puts(:stderr, [warning, message, ?\n, stacktrace, ?\n]) + ExDoc.Utils.warning([message, ?\n, stacktrace, ?\n]) end defp warn(ref, file_line, id, visibility, metadata) diff --git a/lib/ex_doc/cli.ex b/lib/ex_doc/cli.ex index c8e76ee30..192dc14ab 100644 --- a/lib/ex_doc/cli.ex +++ b/lib/ex_doc/cli.ex @@ -34,15 +34,25 @@ defmodule ExDoc.CLI do proglang: :string, quiet: :boolean, source_ref: :string, - source_url: :string, - version: :boolean + version: :boolean, + warnings_as_errors: :boolean ] ) - if List.keymember?(opts, :version, 0) do - IO.puts("ExDoc v#{ExDoc.version()}") - else - generate(args, opts, generator) + cond do + List.keymember?(opts, :version, 0) -> + IO.puts("ExDoc v#{ExDoc.version()}") + + opts[:warnings_as_errors] == true and ExDoc.WarningCounter.count() > 0 -> + IO.puts( + :stderr, + "Doc generation failed due to warnings while using the --warnings-as-errors option" + ) + + exit({:shutdown, ExDoc.WarningCounter.count()}) + + true -> + generate(args, opts, generator) end end @@ -187,6 +197,7 @@ defmodule ExDoc.CLI do --source-ref Branch/commit/tag used for source link inference, default: "master" -u, --source-url URL to the source code -v, --version Print ExDoc version + --warnings-as-errors Exit with non-zero status if doc generation has one or more warnings ## Custom config diff --git a/lib/ex_doc/config.ex b/lib/ex_doc/config.ex index f76714f97..6b0d72ce3 100644 --- a/lib/ex_doc/config.ex +++ b/lib/ex_doc/config.ex @@ -43,7 +43,8 @@ defmodule ExDoc.Config do source_url: nil, source_url_pattern: nil, title: nil, - version: nil + version: nil, + warnings_as_errors: false @type t :: %__MODULE__{ annotations_for_docs: (map() -> list()), @@ -80,7 +81,8 @@ defmodule ExDoc.Config do source_url: nil | String.t(), source_url_pattern: nil | String.t(), title: nil | String.t(), - version: nil | String.t() + version: nil | String.t(), + warnings_as_errors: boolean() } @spec build(String.t(), String.t(), Keyword.t()) :: ExDoc.Config.t() diff --git a/lib/ex_doc/formatter/epub.ex b/lib/ex_doc/formatter/epub.ex index d8f05e4a9..c54c0391f 100644 --- a/lib/ex_doc/formatter/epub.ex +++ b/lib/ex_doc/formatter/epub.ex @@ -75,7 +75,7 @@ defmodule ExDoc.Formatter.EPUB do html = Templates.extra_template(config, title, title_content, content) if File.regular?(output) do - IO.puts(:stderr, "warning: file #{Path.relative_to_cwd(output)} already exists") + ExDoc.Utils.warning("file #{Path.relative_to_cwd(output)} already exists") end File.write!(output, html) diff --git a/lib/ex_doc/formatter/html.ex b/lib/ex_doc/formatter/html.ex index d02a06b38..c31835db4 100644 --- a/lib/ex_doc/formatter/html.ex +++ b/lib/ex_doc/formatter/html.ex @@ -161,7 +161,7 @@ defmodule ExDoc.Formatter.HTML do :ok true -> - IO.warn( + ExDoc.Utils.warning( "ExDoc is outputting to an existing directory. " <> "Beware documentation output may be mixed with other entries" ) @@ -263,7 +263,7 @@ defmodule ExDoc.Formatter.HTML do html = Templates.extra_template(config, node, extra_type(extension), nodes_map, refs) if File.regular?(output) do - IO.puts(:stderr, "warning: file #{Path.relative_to_cwd(output)} already exists") + ExDoc.Utils.warning("file #{Path.relative_to_cwd(output)} already exists") end File.write!(output, html) @@ -556,7 +556,7 @@ defmodule ExDoc.Formatter.HTML do defp generate_redirect(filename, config, redirect_to) do unless case_sensitive_file_regular?("#{config.output}/#{redirect_to}") do - IO.puts(:stderr, "warning: #{filename} redirects to #{redirect_to}, which does not exist") + ExDoc.Utils.warning("#{filename} redirects to #{redirect_to}, which does not exist") end content = Templates.redirect_template(config, redirect_to) diff --git a/lib/ex_doc/language.ex b/lib/ex_doc/language.ex index 5a60ed64f..6c2f26f52 100644 --- a/lib/ex_doc/language.ex +++ b/lib/ex_doc/language.ex @@ -142,10 +142,7 @@ defmodule ExDoc.Language do def get(:erlang, _module), do: {:ok, ExDoc.Language.Erlang} def get(language, module) when is_atom(language) and is_atom(module) do - IO.warn( - "skipping module #{module}, reason: unsupported language (#{language})", - [] - ) + ExDoc.Utils.warning("skipping module #{module}, reason: unsupported language (#{language})") :error end diff --git a/lib/ex_doc/language/elixir.ex b/lib/ex_doc/language/elixir.ex index 65aec1e12..0a52819b6 100644 --- a/lib/ex_doc/language/elixir.ex +++ b/lib/ex_doc/language/elixir.ex @@ -40,7 +40,7 @@ defmodule ExDoc.Language.Elixir do } true -> - IO.warn("skipping docs for module #{inspect(module)}, reason: :no_debug_info", []) + ExDoc.Utils.warning("skipping docs for module #{inspect(module)}, reason: :no_debug_info") end end diff --git a/lib/ex_doc/language/erlang.ex b/lib/ex_doc/language/erlang.ex index 2363813ff..8770a5317 100644 --- a/lib/ex_doc/language/erlang.ex +++ b/lib/ex_doc/language/erlang.ex @@ -31,7 +31,7 @@ defmodule ExDoc.Language.Erlang do } } else - IO.warn("skipping docs for module #{inspect(module)}, reason: :no_debug_info", []) + ExDoc.Utils.warning("skipping docs for module #{inspect(module)}, reason: :no_debug_info") end end diff --git a/lib/ex_doc/markdown/earmark.ex b/lib/ex_doc/markdown/earmark.ex index 11f8316b5..8ac801951 100644 --- a/lib/ex_doc/markdown/earmark.ex +++ b/lib/ex_doc/markdown/earmark.ex @@ -48,7 +48,7 @@ defmodule ExDoc.Markdown.Earmark do defp print_messages(messages, options) do for {severity, line, message} <- messages do file = options[:file] - IO.warn("#{inspect(__MODULE__)} (#{severity}) #{file}:#{line} #{message}", []) + ExDoc.Utils.warning("#{inspect(__MODULE__)} (#{severity}) #{file}:#{line} #{message}") end end diff --git a/lib/ex_doc/retriever.ex b/lib/ex_doc/retriever.ex index 59abbb1c0..be3e16161 100644 --- a/lib/ex_doc/retriever.ex +++ b/lib/ex_doc/retriever.ex @@ -75,7 +75,8 @@ defmodule ExDoc.Retriever do docs {:error, reason} -> - IO.warn("skipping docs for module #{inspect(module)}, reason: #{reason}", []) + ExDoc.Utils.warning("skipping docs for module #{inspect(module)}, reason: #{reason}") + false end diff --git a/lib/ex_doc/utils.ex b/lib/ex_doc/utils.ex index beb77dcc5..2655d1566 100644 --- a/lib/ex_doc/utils.ex +++ b/lib/ex_doc/utils.ex @@ -147,4 +147,17 @@ defmodule ExDoc.Utils do nil end end + + @doc """ + Prints `message` as a warning in :stderr. + + It automatically increases the counter in `ExDoc.WarningCounter`. + """ + def warning(message) do + ExDoc.WarningCounter.increment() + + prefix = IO.ANSI.format([:yellow, "warning: ", :reset]) + + IO.puts(:stderr, [prefix, message]) + end end diff --git a/lib/ex_doc/warning_counter.ex b/lib/ex_doc/warning_counter.ex new file mode 100644 index 000000000..48fe01bad --- /dev/null +++ b/lib/ex_doc/warning_counter.ex @@ -0,0 +1,119 @@ +defmodule ExDoc.WarningCounter do + @moduledoc false + + use GenServer + + @type count :: non_neg_integer() + @type counters_ref :: :counters.counters_ref() + @typep caller :: pid() | :default + @typep state :: %{:default => counters_ref(), caller() => counters_ref()} + + defguardp is_caller(term) when is_pid(term) or term == :default + + ########################### + # Callback implementations + + @spec start_link(any()) :: GenServer.on_start() + def start_link(arg) do + GenServer.start_link(__MODULE__, arg, name: __MODULE__) + end + + @impl GenServer + @spec init(any()) :: {:ok, state} + def init(_args) do + counter_ref = new_counter() + + state = %{ + self() => counter_ref, + default: counter_ref + } + + {:ok, state} + end + + @impl GenServer + def handle_call({:count, caller}, _from, state) when is_caller(caller) do + counter_ref = get_counter_ref(state, caller) + count = :counters.get(counter_ref, 1) + + {:reply, count, state} + end + + @impl GenServer + def handle_info({:increment, caller}, state) when is_caller(caller) do + counter_ref = get_counter_ref(state, caller) + :counters.add(counter_ref, 1, 1) + + {:noreply, state} + end + + def handle_info({:register_caller, caller}, state) when is_caller(caller) do + counter_ref = new_counter() + state = Map.put(state, caller, counter_ref) + + {:noreply, state} + end + + def handle_info({:unregister_caller, caller}, state) when is_caller(caller) do + state = Map.delete(state, caller) + + {:noreply, state} + end + + ############# + # Public API + + @spec count(caller()) :: count() + def count(caller \\ self()) do + GenServer.call(__MODULE__, {:count, caller}) + end + + @spec increment() :: :ok + def increment() do + caller = self() + send(__MODULE__, {:increment, caller}) + + :ok + end + + @spec register_caller(caller()) :: :ok + def register_caller(caller) when is_caller(caller) do + send(__MODULE__, {:register_caller, caller}) + end + + @spec unregister_caller(caller()) :: :ok + def unregister_caller(caller) when is_caller(caller) do + send(__MODULE__, {:unregister_caller, caller}) + end + + ################## + # Private helpers + + defp new_counter() do + :counters.new(1, [:atomics]) + end + + defp get_counter_ref(state, caller) + when is_map(state) and is_caller(caller) and is_map_key(state, caller) do + Map.fetch!(state, caller) + end + + defp get_counter_ref(%{default: default_counter_ref} = state, caller) + when is_map(state) and is_caller(caller) do + callers = callers(caller) + + Enum.find_value(callers, default_counter_ref, fn the_caller -> + Map.get(state, the_caller) + end) + end + + defp callers(pid) when is_pid(pid) do + case Process.info(pid, :dictionary) do + {:dictionary, dictionary} -> + Keyword.get(dictionary, :"$callers", []) + + nil -> + [] + end + end +end diff --git a/lib/mix/tasks/docs.ex b/lib/mix/tasks/docs.ex index 7712dcb64..f91794fde 100644 --- a/lib/mix/tasks/docs.ex +++ b/lib/mix/tasks/docs.ex @@ -27,6 +27,8 @@ defmodule Mix.Tasks.Docs do * `--proglang` - Chooses the main programming language: "elixir" or "erlang" + * `--warnings-as-errors` - Exits with non-zero exit code if any warnings are found + The command line options have higher precedence than the options specified in your `mix.exs` file below. @@ -317,7 +319,8 @@ defmodule Mix.Tasks.Docs do language: :string, open: :boolean, output: :string, - proglang: :string + proglang: :string, + warnings_as_errors: :boolean ] @aliases [ @@ -385,7 +388,18 @@ defmodule Mix.Tasks.Docs do browser_open(index) end - index + warning_counter_count = ExDoc.WarningCounter.count() + + if options[:warnings_as_errors] == true and warning_counter_count > 0 do + Mix.shell().info([ + :red, + "Doc generation failed due to warnings while using the --warnings-as-errors option" + ]) + + exit({:shutdown, warning_counter_count}) + else + index + end end end diff --git a/test/ex_doc/cli_test.exs b/test/ex_doc/cli_test.exs index d0f35f6e5..d1237e926 100644 --- a/test/ex_doc/cli_test.exs +++ b/test/ex_doc/cli_test.exs @@ -1,6 +1,8 @@ defmodule ExDoc.CLITest do use ExUnit.Case, async: true + import ExUnit.CaptureIO + import TestHelper, only: [isolated_warning_counter: 1] @ebin "_build/test/lib/ex_doc/ebin" @@ -67,6 +69,76 @@ defmodule ExDoc.CLITest do assert io == "ExDoc v#{ExDoc.version()}\n" end + describe "--warnings-as-errors" do + @describetag :warning_counter + + test "exits with code 0 when no warnings" do + isolated_warning_counter do + {[html, epub], _io} = run(["ExDoc", "1.2.3", @ebin, "--warnings-as-errors"]) + + assert html == + {"ExDoc", "1.2.3", + [ + formatter: "html", + formatters: ["html", "epub"], + apps: [:ex_doc], + source_beam: @ebin, + warnings_as_errors: true + ]} + + assert epub == + {"ExDoc", "1.2.3", + [ + formatter: "epub", + formatters: ["html", "epub"], + apps: [:ex_doc], + source_beam: @ebin, + warnings_as_errors: true + ]} + end + end + + test "exits with 1 with warnings" do + isolated_warning_counter do + ExDoc.WarningCounter.increment() + + fun = fn -> + run(["ExDoc", "1.2.3", @ebin, "--warnings-as-errors"]) + end + + io = + capture_io(:stderr, fn -> + assert catch_exit(fun.()) == {:shutdown, 1} + end) + + assert io =~ + "Doc generation failed due to warnings while using the --warnings-as-errors option\n" + end + end + + test "exits with 3 with warnings" do + isolated_warning_counter do + ExDoc.WarningCounter.increment() + ExDoc.WarningCounter.increment() + ExDoc.WarningCounter.increment() + ExDoc.WarningCounter.increment() + ExDoc.WarningCounter.increment() + + fun = fn -> + run(["ExDoc", "1.2.3", @ebin, "--warnings-as-errors"]) + end + + io = + capture_io(:stderr, fn -> + assert catch_exit(fun.()) == {:shutdown, 5} + end) + + assert io =~ + "Doc generation failed due to warnings while using the --warnings-as-errors option\n" + end + end + end + test "too many arguments" do assert catch_exit(run(["ExDoc", "1.2.3", "/", "kaboom"])) == {:shutdown, 1} end diff --git a/test/ex_doc/formatter/epub_test.exs b/test/ex_doc/formatter/epub_test.exs index c1f02225c..469278bc2 100644 --- a/test/ex_doc/formatter/epub_test.exs +++ b/test/ex_doc/formatter/epub_test.exs @@ -1,6 +1,9 @@ defmodule ExDoc.Formatter.EPUBTest do use ExUnit.Case, async: true + import ExUnit.CaptureIO + import TestHelper, only: [isolated_warning_counter: 1] + @moduletag :tmp_dir @before_closing_head_tag_content_epub "UNIQUE:©BEFORE-CLOSING-HEAD-TAG-HTML" @@ -248,4 +251,47 @@ defmodule ExDoc.Formatter.EPUBTest do after File.rm_rf!("test/tmp/epub_assets") end + + describe "warning counter" do + @describetag :warning_counter + + test "4 warnings are counted when using warnings_as_errors: true", context do + isolated_warning_counter do + output = + capture_io(:stderr, fn -> + generate_docs( + doc_config(context, + skip_undefined_reference_warnings_on: [], + warnings_as_errors: true + ) + ) + end) + + assert output =~ ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:2: Warnings" + assert output =~ ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:18: Warnings.foo/0" + + assert output =~ + ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:13: c:Warnings.handle_foo/0" + + assert output =~ ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:8: t:Warnings.t/0" + + assert ExDoc.WarningCounter.count() == 4 + end + end + + test "warnings are still counted even with warnings_as_errors: false", context do + isolated_warning_counter do + capture_io(:stderr, fn -> + generate_docs( + doc_config(context, + skip_undefined_reference_warnings_on: [], + warnings_as_errors: false + ) + ) + end) + + assert ExDoc.WarningCounter.count() == 4 + end + end + end end diff --git a/test/ex_doc/formatter/html_test.exs b/test/ex_doc/formatter/html_test.exs index 9cde053da..07b73bb7f 100644 --- a/test/ex_doc/formatter/html_test.exs +++ b/test/ex_doc/formatter/html_test.exs @@ -2,6 +2,8 @@ defmodule ExDoc.Formatter.HTMLTest do use ExUnit.Case, async: true import ExUnit.CaptureIO + import TestHelper, only: [isolated_warning_counter: 1] + alias ExDoc.Formatter.HTML @moduletag :tmp_dir @@ -124,7 +126,9 @@ defmodule ExDoc.Formatter.HTMLTest do generate_docs(doc_config(context, main: "Randomerror")) end) - assert output =~ "warning: index.html redirects to Randomerror.html, which does not exist\n" + assert output =~ + ~R"warning: .*index.html redirects to Randomerror.html, which does not exist\n" + assert File.regular?(tmp_dir <> "/html/index.html") assert File.regular?(tmp_dir <> "/html/RandomError.html") end @@ -141,6 +145,63 @@ defmodule ExDoc.Formatter.HTMLTest do assert output =~ ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:8: t:Warnings.t/0" end + describe "warning counter" do + @describetag :warning_counter + + test "4 warnings are counted when using warnings_as_errors: true", context do + isolated_warning_counter do + output = + capture_io(:stderr, fn -> + generate_docs( + doc_config(context, + skip_undefined_reference_warnings_on: [], + warnings_as_errors: true + ) + ) + end) + + assert output =~ ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:2: Warnings" + assert output =~ ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:18: Warnings.foo/0" + + assert output =~ + ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:13: c:Warnings.handle_foo/0" + + assert output =~ ~r"Warnings.bar/0.*\n test/fixtures/warnings.ex:8: t:Warnings.t/0" + + assert ExDoc.WarningCounter.count() == 4 + end + end + + test "warnings are still counted even with warnings_as_errors: false", context do + isolated_warning_counter do + capture_io(:stderr, fn -> + generate_docs( + doc_config(context, + skip_undefined_reference_warnings_on: [], + warnings_as_errors: false + ) + ) + end) + + assert ExDoc.WarningCounter.count() == 4 + end + end + + test "1 warning is counted when using warnings_as_errors: true", context do + isolated_warning_counter do + output = + capture_io(:stderr, fn -> + generate_docs(doc_config(context, main: "DoesNotExist", warnings_as_errors: true)) + end) + + assert output =~ + ~R"warning: .*index.html redirects to DoesNotExist.html, which does not exist\n" + + assert ExDoc.WarningCounter.count() == 1 + end + end + end + test "generates headers for index.html and module pages", %{tmp_dir: tmp_dir} = context do generate_docs(doc_config(context, main: "RandomError")) content_index = File.read!(tmp_dir <> "/html/index.html") diff --git a/test/mix/tasks/docs_test.exs b/test/mix/tasks/docs_test.exs index 11f9bc511..a5c23c7de 100644 --- a/test/mix/tasks/docs_test.exs +++ b/test/mix/tasks/docs_test.exs @@ -5,6 +5,8 @@ defmodule Mix.Tasks.DocsTest do # Cannot run concurrently due to Mix compile/deps calls use ExUnit.Case, async: false + import TestHelper, only: [isolated_warning_counter: 1] + @moduletag :tmp_dir def run(context, args, opts) do @@ -390,4 +392,51 @@ defmodule Mix.Tasks.DocsTest do ] = run(context, [], app: :umbrella, apps_path: "apps/", docs: [ignore_apps: [:foo]]) end) end + + test "accepts warnings_as_errors in :warnings_as_errors", context do + isolated_warning_counter do + assert [ + {"ex_doc", "dev", + [ + formatter: "html", + formatters: ["html", "epub"], + deps: _, + apps: [:ex_doc], + source_beam: _, + warnings_as_errors: true, + proglang: :elixir + ]}, + {"ex_doc", "dev", + [ + formatter: "epub", + formatters: ["html", "epub"], + deps: _, + apps: [:ex_doc], + source_beam: _, + warnings_as_errors: true, + proglang: :elixir + ]} + ] = run(context, [], app: :ex_doc, docs: [warnings_as_errors: true]) + end + end + + test "exits with 1 due to warning with --warnings_as_errors", context do + isolated_warning_counter do + ExDoc.WarningCounter.increment() + + assert catch_exit(run(context, [], app: :ex_doc, docs: [warnings_as_errors: true])) == + {:shutdown, 1} + end + end + + test "exits with 3 due to warning with --warnings_as_errors", context do + isolated_warning_counter do + ExDoc.WarningCounter.increment() + ExDoc.WarningCounter.increment() + ExDoc.WarningCounter.increment() + + assert catch_exit(run(context, [], app: :ex_doc, docs: [warnings_as_errors: true])) == + {:shutdown, 3} + end + end end diff --git a/test/test_helper.exs b/test/test_helper.exs index bf13c7992..e92b87c8a 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -100,4 +100,21 @@ defmodule TestHelper do raise "not supported" end end + + defmacro isolated_warning_counter(do: code) do + quote location: :keep do + task = + Task.async(fn -> + ExDoc.WarningCounter.register_caller(self()) + + try do + unquote(code) + after + ExDoc.WarningCounter.unregister_caller(self()) + end + end) + + Task.await(task) + end + end end