From aff8cb821e11d0a81f89de11457c5fd5c0d436db Mon Sep 17 00:00:00 2001 From: Eksperimental Date: Sun, 3 Dec 2023 21:05:56 -0500 Subject: [PATCH] Add --warnings-as-errors flag for non-zero exit code --- lib/ex_doc/cli.ex | 68 +++++++++++++++----------- lib/ex_doc/config.ex | 6 ++- lib/ex_doc/formatter/html.ex | 2 +- lib/ex_doc/formatter/html/templates.ex | 3 +- lib/ex_doc/utils.ex | 33 +++++++++++-- lib/mix/tasks/docs.ex | 16 +++++- test/ex_doc/cli_test.exs | 67 ++++++++++++++++++++++++- test/ex_doc/formatter/epub_test.exs | 47 +++++++++++++++++- test/ex_doc/formatter/html_test.exs | 66 +++++++++++++++++++++++-- test/mix/tasks/docs_test.exs | 41 ++++++++++++++++ 10 files changed, 304 insertions(+), 45 deletions(-) diff --git a/lib/ex_doc/cli.ex b/lib/ex_doc/cli.ex index c8e76ee30..1f692fdbd 100644 --- a/lib/ex_doc/cli.ex +++ b/lib/ex_doc/cli.ex @@ -35,14 +35,25 @@ defmodule ExDoc.CLI do 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.Utils.warned?() -> + IO.puts( + :stderr, + "Doc generation failed due to warnings while using the --warnings-as-errors option" + ) + + exit({:shutdown, 1}) + + true -> + generate(args, opts, generator) end end @@ -164,29 +175,30 @@ defmodule ExDoc.CLI do ex_doc "Project" "1.0.0" "_build/dev/lib/project/ebin" -c "docs.exs" Options: - PROJECT Project name - VERSION Version number - BEAMS Path to compiled beam files - --canonical Indicate the preferred URL with rel="canonical" link element - -c, --config Give configuration through a file instead of a command line. - See "Custom config" section below for more information. - -f, --formatter Docs formatter to use (html or epub), default: html and epub - --homepage-url URL to link to for the site name - --language Identify the primary language of the documents, its value must be - a valid [BCP 47](https://tools.ietf.org/html/bcp47) language tag, default: "en" - -l, --logo Path to the image logo of the project (only PNG or JPEG accepted) - The image size will be 64x64 and copied to the assets directory - -m, --main The entry-point page in docs, default: "api-reference" - -o, --output Path to output docs, default: "doc" - --package Hex package name - --paths Prepends the given path to Erlang code path. The path might contain a glob - pattern but in that case, remember to quote it: --paths "_build/dev/lib/*/ebin". - This option can be given multiple times - --proglang The project's programming language, default: "elixir" - -q, --quiet Only output warnings and errors - --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 + PROJECT Project name. + VERSION Version number. + BEAMS Path to compiled beam files. + --canonical Indicate the preferred URL with rel="canonical" link element. + -c, --config Give configuration through a file instead of a command line. + See "Custom config" section below for more information. + -f, --formatter Docs formatter to use (html or epub), default: html and epub. + --homepage-url URL to link to for the site name. + --language Identify the primary language of the documents, its value must be + a valid [BCP 47](https://tools.ietf.org/html/bcp47) language tag, default: "en". + -l, --logo Path to the image logo of the project (only PNG or JPEG accepted). + The image size will be 64x64 and copied to the assets directory. + -m, --main The entry-point page in docs, default: "api-reference". + -o, --output Path to output docs, default: "doc". + --package Hex package name. + --paths Prepends the given path to Erlang code path. The path might contain a glob + pattern but in that case, remember to quote it: --paths "_build/dev/lib/*/ebin". + This option can be given multiple times. + --proglang The project's programming language, default: "elixir". + -q, --quiet Only output warnings and errors. + --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 4d4c8abca..d1b917046 100644 --- a/lib/ex_doc/config.ex +++ b/lib/ex_doc/config.ex @@ -44,7 +44,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()), @@ -82,7 +83,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/html.ex b/lib/ex_doc/formatter/html.ex index cd2088ccb..59070e5fc 100644 --- a/lib/ex_doc/formatter/html.ex +++ b/lib/ex_doc/formatter/html.ex @@ -246,7 +246,7 @@ defmodule ExDoc.Formatter.HTML do html = Templates.extra_template(config, node, extra_type(extension), nodes_map, refs) if File.regular?(output) do - ExDoc.Utils.warn("warning: file #{Path.relative_to_cwd(output)} already exists", []) + ExDoc.Utils.warn("file #{Path.relative_to_cwd(output)} already exists", []) end File.write!(output, html) diff --git a/lib/ex_doc/formatter/html/templates.ex b/lib/ex_doc/formatter/html/templates.ex index e82e1ff6c..04b6de320 100644 --- a/lib/ex_doc/formatter/html/templates.ex +++ b/lib/ex_doc/formatter/html/templates.ex @@ -193,8 +193,7 @@ defmodule ExDoc.Formatter.HTML.Templates do end def module_summary(module_node) do - entries = - docs_groups(module_node.docs_groups, module_node.docs ++ module_node.typespecs) + entries = docs_groups(module_node.docs_groups, module_node.docs ++ module_node.typespecs) Enum.reject(entries, fn {_type, nodes} -> nodes == [] end) end diff --git a/lib/ex_doc/utils.ex b/lib/ex_doc/utils.ex index 480ceec39..cd4de4ab8 100644 --- a/lib/ex_doc/utils.ex +++ b/lib/ex_doc/utils.ex @@ -1,19 +1,42 @@ defmodule ExDoc.Utils do @moduledoc false + @elixir_gte_1_14? Version.match?(System.version(), ">= 1.14.0") + @doc """ Emits a warning. """ - if Version.match?(System.version(), ">= 1.14.0") do - def warn(message, stacktrace_info) do + def warn(message, stacktrace_info) do + put_warned() + + # TODO: remove check when we require Elixir v1.14 + if @elixir_gte_1_14? do IO.warn(message, stacktrace_info) - end - else - def warn(message, _stacktrace_info) do + else IO.warn(message, []) end end + @doc false + def put_warned() do + unless warned?() do + :persistent_term.put({__MODULE__, :warned?}, true) + end + + true + end + + @doc false + def delete_warned() do + if warned?() do + :persistent_term.put({__MODULE__, :warned?}, false) + end + end + + def warned?() do + :persistent_term.get({__MODULE__, :warned?}, false) + end + @doc """ Runs the `before_closing_head_tag` callback. """ diff --git a/lib/mix/tasks/docs.ex b/lib/mix/tasks/docs.ex index 5c9e44e6a..8b602049e 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. @@ -323,7 +325,8 @@ defmodule Mix.Tasks.Docs do language: :string, open: :boolean, output: :string, - proglang: :string + proglang: :string, + warnings_as_errors: :boolean ] @aliases [ @@ -391,7 +394,16 @@ defmodule Mix.Tasks.Docs do browser_open(index) end - index + if options[:warnings_as_errors] == true and ExDoc.Utils.warned?() do + Mix.shell().info([ + :red, + "Doc generation failed due to warnings while using the --warnings-as-errors option" + ]) + + exit({:shutdown, 1}) + else + index + end end end diff --git a/test/ex_doc/cli_test.exs b/test/ex_doc/cli_test.exs index d0f35f6e5..860c60c5a 100644 --- a/test/ex_doc/cli_test.exs +++ b/test/ex_doc/cli_test.exs @@ -1,5 +1,6 @@ defmodule ExDoc.CLITest do - use ExUnit.Case, async: true + use ExUnit.Case, async: false + import ExUnit.CaptureIO @ebin "_build/test/lib/ex_doc/ebin" @@ -67,6 +68,70 @@ defmodule ExDoc.CLITest do assert io == "ExDoc v#{ExDoc.version()}\n" end + describe "--warnings-as-errors" do + @describetag :warnings + + test "exits with code 0 when no warnings" do + ExDoc.Utils.delete_warned() + + {[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 + + test "exits with 1 when there is a warning" do + ExDoc.Utils.put_warned() + + 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 + + test "exits with 1 when there are multiple warnings" do + ExDoc.Utils.put_warned() + ExDoc.Utils.put_warned() + ExDoc.Utils.put_warned() + + 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 "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..35d764aff 100644 --- a/test/ex_doc/formatter/epub_test.exs +++ b/test/ex_doc/formatter/epub_test.exs @@ -1,5 +1,7 @@ defmodule ExDoc.Formatter.EPUBTest do - use ExUnit.Case, async: true + use ExUnit.Case, async: false + + import ExUnit.CaptureIO @moduletag :tmp_dir @@ -248,4 +250,47 @@ defmodule ExDoc.Formatter.EPUBTest do after File.rm_rf!("test/tmp/epub_assets") end + + describe "warnings" do + @describetag :warnings + + test "multiple warnings are registered when using warnings_as_errors: true", context do + ExDoc.Utils.delete_warned() + + output = + capture_io(:stderr, fn -> + generate_docs( + doc_config(context, + skip_undefined_reference_warnings_on: [], + warnings_as_errors: true + ) + ) + end) + + # TODO: remove check when we require Elixir v1.16 + if Version.match?(System.version(), ">= 1.16.0-rc") do + assert output =~ ~S|moduledoc `Warnings.bar/0`| + assert output =~ ~S|typedoc `Warnings.bar/0`| + assert output =~ ~S|doc callback `Warnings.bar/0`| + assert output =~ ~S|doc `Warnings.bar/0`| + end + + assert ExDoc.Utils.warned?() == true + end + + test "warnings are registered even with warnings_as_errors: false", context do + ExDoc.Utils.delete_warned() + + capture_io(:stderr, fn -> + generate_docs( + doc_config(context, + skip_undefined_reference_warnings_on: [], + warnings_as_errors: false + ) + ) + end) + + assert ExDoc.Utils.warned?() == true + end + end end diff --git a/test/ex_doc/formatter/html_test.exs b/test/ex_doc/formatter/html_test.exs index 658f06f77..db66ac673 100644 --- a/test/ex_doc/formatter/html_test.exs +++ b/test/ex_doc/formatter/html_test.exs @@ -1,7 +1,8 @@ defmodule ExDoc.Formatter.HTMLTest do - use ExUnit.Case, async: true + use ExUnit.Case, async: false import ExUnit.CaptureIO + alias ExDoc.Formatter.HTML @moduletag :tmp_dir @@ -120,7 +121,9 @@ defmodule ExDoc.Formatter.HTMLTest do generate_docs(doc_config(context, main: "Randomerror")) end) - assert output =~ "index.html redirects to Randomerror.html, which does not exist\n" + assert output =~ + ~r"warning:\e\[0m .*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 @@ -131,7 +134,7 @@ defmodule ExDoc.Formatter.HTMLTest do generate_docs(doc_config(context, skip_undefined_reference_warnings_on: [])) end) - assert out =~ ~s| documentation references function \"Warnings.bar/0\" but| + assert out =~ ~s|documentation references function \"Warnings.bar/0\" but| # TODO: remove check when we require Elixir v1.16 if Version.match?(System.version(), ">= 1.16.0-rc") do @@ -142,6 +145,63 @@ defmodule ExDoc.Formatter.HTMLTest do end end + describe "warnings" do + @describetag :warnings + + test "single warning is registered when using warnings_as_errors: true", context do + ExDoc.Utils.delete_warned() + + output = + capture_io(:stderr, fn -> + generate_docs(doc_config(context, main: "DoesNotExist", warnings_as_errors: true)) + end) + + assert output =~ + ~r"warning:\e\[0m .*index.html redirects to DoesNotExist.html, which does not exist\n" + + assert ExDoc.Utils.warned?() == true + end + + test "multiple warnings are registered when using warnings_as_errors: true", context do + ExDoc.Utils.delete_warned() + + output = + capture_io(:stderr, fn -> + generate_docs( + doc_config(context, + skip_undefined_reference_warnings_on: [], + warnings_as_errors: true + ) + ) + end) + + # TODO: remove check when we require Elixir v1.16 + if Version.match?(System.version(), ">= 1.16.0-rc") do + assert output =~ ~S|moduledoc `Warnings.bar/0`| + assert output =~ ~S|typedoc `Warnings.bar/0`| + assert output =~ ~S|doc callback `Warnings.bar/0`| + assert output =~ ~S|doc `Warnings.bar/0`| + end + + assert ExDoc.Utils.warned?() == true + end + + test "warnings are registered even with warnings_as_errors: false", context do + ExDoc.Utils.delete_warned() + + capture_io(:stderr, fn -> + generate_docs( + doc_config(context, + skip_undefined_reference_warnings_on: [], + warnings_as_errors: false + ) + ) + end) + + assert ExDoc.Utils.warned?() == true + 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..59694512e 100644 --- a/test/mix/tasks/docs_test.exs +++ b/test/mix/tasks/docs_test.exs @@ -390,4 +390,45 @@ 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 + assert [ + {"ex_doc", "dev", + [ + formatter: "html", + formatters: ["html", "epub"], + deps: _, + apps: [:ex_doc], + source_beam: _, + warnings_as_errors: false, + proglang: :elixir + ]}, + {"ex_doc", "dev", + [ + formatter: "epub", + formatters: ["html", "epub"], + deps: _, + apps: [:ex_doc], + source_beam: _, + warnings_as_errors: false, + proglang: :elixir + ]} + ] = run(context, [], app: :ex_doc, docs: [warnings_as_errors: false]) + end + + test "exits with 1 due to warning, with flag --warnings_as_errors", context do + ExDoc.Utils.put_warned() + + assert catch_exit(run(context, [], app: :ex_doc, docs: [warnings_as_errors: true])) == + {:shutdown, 1} + end + + test "exits with 1 due to multiple warnings, with flag --warnings_as_errors", context do + ExDoc.Utils.put_warned() + ExDoc.Utils.put_warned() + ExDoc.Utils.put_warned() + + assert catch_exit(run(context, [], app: :ex_doc, docs: [warnings_as_errors: true])) == + {:shutdown, 1} + end end