Skip to content

Add --warnings-as-errors flag for non-zero exit code #1412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/ex_doc/application.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
68 changes: 40 additions & 28 deletions lib/ex_doc/cli.ex
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,25 @@ defmodule ExDoc.CLI do
source_ref: :string,
version: :boolean,
formatter: :keep,
quiet: :boolean
quiet: :boolean,
warnings_as_errors: :boolean
]
)

if List.keymember?(opts, :version, 0) do
print_version()
else
generate(args, opts, generator)
cond do
List.keymember?(opts, :version, 0) ->
print_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

Expand Down Expand Up @@ -159,29 +170,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
-n, --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
-p, --homepage-url URL to link to for the site 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
--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"
--package Hex package name
--proglang The project's programming language, default: "elixir"
--source-ref Branch/commit/tag used for source link inference, default: "master"
-u, --source-url URL to the source code
-o, --output Path to output docs, default: "doc"
-v, --version Print ExDoc version
-q, --quiet Only output warnings and errors
PROJECT Project name
VERSION Version number
BEAMS Path to compiled beam files
-n, --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
-p, --homepage-url URL to link to for the site 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
--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"
--package Hex package name
--proglang The project's programming language, default: "elixir"
--source-ref Branch/commit/tag used for source link inference, default: "master"
-u, --source-url URL to the source code
-o, --output Path to output docs, default: "doc"
-v, --version Print ExDoc version
-q, --quiet Only output warnings and errors
--warnings-as-errors Exit with non-zero status if doc generation has one or more warnings

## Custom config

Expand Down
6 changes: 4 additions & 2 deletions lib/ex_doc/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ defmodule ExDoc.Config do
version: nil,
authors: nil,
skip_undefined_reference_warnings_on: [],
package: nil
package: nil,
warnings_as_errors: false

@type t :: %__MODULE__{
apps: [atom()],
Expand Down Expand Up @@ -75,7 +76,8 @@ defmodule ExDoc.Config do
version: nil | String.t(),
authors: nil | [String.t()],
skip_undefined_reference_warnings_on: [String.t()],
package: :atom | nil
package: :atom | nil,
warnings_as_errors: boolean()
}

@spec build(String.t(), String.t(), Keyword.t()) :: ExDoc.Config.t()
Expand Down
2 changes: 2 additions & 0 deletions lib/ex_doc/language.ex
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ 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
ExDoc.WarningCounter.increment()

IO.warn(
"skipping module #{module}, reason: unsupported language (#{language})",
[]
Expand Down
1 change: 1 addition & 0 deletions lib/ex_doc/markdown/earmark.ex
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ defmodule ExDoc.Markdown.Earmark do
defp print_messages(messages, options) do
for {severity, line, message} <- messages do
file = options[:file]
ExDoc.WarningCounter.increment()
IO.warn("#{inspect(__MODULE__)} (#{severity}) #{file}:#{line} #{message}", [])
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/ex_doc/retriever.ex
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ defmodule ExDoc.Retriever do
docs

{:error, reason} ->
ExDoc.WarningCounter.increment()
IO.warn("skipping module #{inspect(module)}, reason: #{reason}", [])
false
end
Expand Down
21 changes: 21 additions & 0 deletions lib/ex_doc/warning_counter.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
defmodule ExDoc.WarningCounter do
@moduledoc false

use Agent

def start_link(_opts) do
Agent.start_link(fn -> 0 end, name: __MODULE__)
end

def count do
Agent.get(__MODULE__, & &1)
end

def increment do
Agent.update(__MODULE__, &(&1 + 1))
end

def reset do
Agent.update(__MODULE__, fn _ -> 0 end)
end
end
16 changes: 14 additions & 2 deletions lib/mix/tasks/docs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ defmodule Mix.Tasks.Docs do

* `--open` - open browser window pointed to the documentation

* `--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.

Expand Down Expand Up @@ -309,7 +311,8 @@ defmodule Mix.Tasks.Docs do
formatter: :keep,
language: :string,
output: :string,
open: :boolean
open: :boolean,
warnings_as_errors: :boolean
]

@aliases [n: :canonical, f: :formatter, o: :output]
Expand Down Expand Up @@ -363,7 +366,16 @@ defmodule Mix.Tasks.Docs do
browser_open(index)
end

index
if options[:warnings_as_errors] == true and ExDoc.WarningCounter.count() > 0 do
Mix.shell().info([
:red,
"Doc generation failed due to warnings while using the --warnings-as-errors option"
])

exit({:shutdown, ExDoc.WarningCounter.count()})
else
index
end
end
end

Expand Down
32 changes: 32 additions & 0 deletions test/ex_doc/cli_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,38 @@ defmodule ExDoc.CLITest do
assert io == "ExDoc v#{ExDoc.version()}\n"
end

describe "--warnings-as-errors" do
test "exits with code 0 when no warnings" do
{[html, epub], _io} = run(["ExDoc", "1.2.3", @ebin, "--warnings-as-errors"])

assert html ==
{"ExDoc", "1.2.3",
[formatter: "html", apps: [:ex_doc], source_beam: @ebin, warnings_as_errors: true]}

assert epub ==
{"ExDoc", "1.2.3",
[formatter: "epub", apps: [:ex_doc], source_beam: @ebin, warnings_as_errors: true]}
end

test "exits with 1 with warnings" do
ExDoc.WarningCounter.increment()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is running in async mode, and you're modifying a global state here that could affect other tests, which is what happening with test/mix/tasks/docs_test.exs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the simplest way to solve this?
I have tried some but it requires a large refactoring.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do a ExDoc.WarningCounter.reset() on the tests that use this global state would that work?

Copy link
Contributor

@eksperimental eksperimental Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do a ExDoc.WarningCounter.reset() on the tests that use this global state would that work?

the issue is that since they are in separate files they could be executed concurrently.
If you execute reset in one file, it will be resetting the ones in the other tests. The only guarantee is that if you use async: false is that the tests within the same file are not going to be running asynchronously.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like this is going to be more work than I was expecting to get added 😞 I started a new job and am no longer working with elixir day to day so I likely won't be finishing this up anymore. If someone else wants to pick it up feel free.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like this is going to be more work than I was expecting to get added disappointed I started a new job and am no longer working with elixir day to day so I likely won't be finishing this up anymore. If someone else wants to pick it up feel free.

Sure. No worries. I was already working with your PR so I'm familiar with it.
Thanks a lot for the PR, it is almost there.


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"
after
ExDoc.WarningCounter.reset()
end
end

test "too many arguments" do
assert catch_exit(run(["ExDoc", "1.2.3", "/", "kaboom"])) == {:shutdown, 1}
end
Expand Down
6 changes: 6 additions & 0 deletions test/mix/tasks/docs_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,10 @@ defmodule Mix.Tasks.DocsTest do
] = run([], app: :umbrella, apps_path: "apps/", docs: [ignore_apps: [:foo]])
end)
end

test "accepts warnings_as_errors in :warnings_as_errors" do
assert catch_exit(run([], app: :ex_doc, docs: [warnings_as_errors: true])) == {:shutdown, 1}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Individually, this test should fail, it's assuming that the state of the WarningCounter is greater than zero when is not the case.

after
ExDoc.WarningCounter.reset()
end
end