Skip to content
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

Add --warnings-as-errors flag for non-zero exit code (Take 2) #1831

Merged
Merged
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ ex_doc-*.tar

node_modules/
/test/fixtures/umbrella/_build/
/test/fixtures/single/_build/
/test/fixtures/single/doc/
/test/tmp/
/tmp/
/npm-debug.log
Expand Down
80 changes: 54 additions & 26 deletions lib/ex_doc/cli.ex
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,37 @@ 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)
results = generate(args, opts, generator)
error_results = Enum.filter(results, &(elem(&1, 0) == :error))

if error_results == [] do
Enum.map(results, fn {:ok, value} -> value end)
else
formatters = Enum.map(error_results, &elem(&1, 1).formatter)

format_message =
case formatters do
[formatter] -> "#{formatter} format"
_ -> "#{Enum.join(formatters, ", ")} formats"
end

message =
"Documents have been generated, but generation for #{format_message} failed due to warnings while using the --warnings-as-errors option."

message_formatted = IO.ANSI.format([:red, message, :reset])

IO.puts(:stderr, message_formatted)

exit({:shutdown, 1})
Comment on lines +63 to +67
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
message_formatted = IO.ANSI.format([:red, message, :reset])
IO.puts(:stderr, message_formatted)
exit({:shutdown, 1})
message_formatted = IO.ANSI.format([:red, message, :reset])
IO.puts(:stderr, message_formatted)
exit({:shutdown, 1})

end
end
end

Expand Down Expand Up @@ -71,7 +94,11 @@ defmodule ExDoc.CLI do
quiet? ||
IO.puts(IO.ANSI.format([:green, "View #{inspect(formatter)} docs at #{inspect(index)}"]))

index
if opts[:warnings_as_errors] == true and ExDoc.Utils.warned?() do
{:error, %{reason: :warnings_as_errors, formatter: formatter}}
else
{:ok, index}
end
end
end

Expand Down Expand Up @@ -164,29 +191,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 a logo image for the project. Must be PNG, JPEG or SVG. The image will
be placed in the output "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 a logo image for the project. Must be PNG, JPEG or SVG. The image will
be placed in the output "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 produces 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 @@ -49,7 +49,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()),
Expand Down Expand Up @@ -88,7 +89,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()
Expand Down
7 changes: 5 additions & 2 deletions lib/ex_doc/formatter/epub.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ defmodule ExDoc.Formatter.EPUB do
@assets_dir "OEBPS/assets"
alias __MODULE__.{Assets, Templates}
alias ExDoc.Formatter.HTML
alias ExDoc.Utils

@doc """
Generate EPUB documentation for the given modules.
Generates EPUB documentation for the given modules.
"""
@spec run([ExDoc.ModuleNode.t()], [ExDoc.ModuleNode.t()], ExDoc.Config.t()) :: String.t()
def run(project_nodes, filtered_modules, config) when is_map(config) do
Utils.unset_warned()

config = normalize_config(config)
File.rm_rf!(config.output)
File.mkdir_p!(Path.join(config.output, "OEBPS"))
Expand Down Expand Up @@ -66,7 +69,7 @@ defmodule ExDoc.Formatter.EPUB do
html = Templates.extra_template(config, title, title_content, content)

if File.regular?(output) do
ExDoc.Utils.warn("file #{Path.relative_to_cwd(output)} already exists", [])
Utils.warn("file #{Path.relative_to_cwd(output)} already exists", [])
end

File.write!(output, html)
Expand Down
6 changes: 4 additions & 2 deletions lib/ex_doc/formatter/html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ defmodule ExDoc.Formatter.HTML do
@assets_dir "assets"

@doc """
Generate HTML documentation for the given modules.
Generates HTML documentation for the given modules.
"""
@spec run([ExDoc.ModuleNode.t()], [ExDoc.ModuleNode.t()], ExDoc.Config.t()) :: String.t()
def run(project_nodes, filtered_modules, config) when is_map(config) do
Utils.unset_warned()

config = normalize_config(config)
config = %{config | output: Path.expand(config.output)}

Expand Down Expand Up @@ -528,7 +530,7 @@ defmodule ExDoc.Formatter.HTML do

defp generate_redirect(filename, config, redirect_to) do
unless case_sensitive_file_regular?("#{config.output}/#{redirect_to}") do
ExDoc.Utils.warn("#{filename} redirects to #{redirect_to}, which does not exist", [])
Utils.warn("#{filename} redirects to #{redirect_to}, which does not exist", [])
end

content = Templates.redirect_template(config, redirect_to)
Expand Down
40 changes: 35 additions & 5 deletions lib/ex_doc/utils.ex
Original file line number Diff line number Diff line change
@@ -1,19 +1,49 @@
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
set_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 """
Stores that a warning has been generated.
"""
def set_warned() do
unless warned?() do
:persistent_term.put({__MODULE__, :warned?}, true)
end

true
end

@doc """
Removes that a warning has been generated.
"""
def unset_warned() do
if warned?() do
:persistent_term.put({__MODULE__, :warned?}, false)
end
end

@doc """
Returns `true` if any warning has been generated during the document building. Otherwise returns `false`.
"""
def warned?() do
:persistent_term.get({__MODULE__, :warned?}, false)
end

@doc """
Runs the `before_closing_head_tag` callback.
"""
Expand Down
52 changes: 45 additions & 7 deletions lib/mix/tasks/docs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -325,7 +327,8 @@ defmodule Mix.Tasks.Docs do
language: :string,
open: :boolean,
output: :string,
proglang: :string
proglang: :string,
warnings_as_errors: :boolean
]

@aliases [
Expand Down Expand Up @@ -383,17 +386,52 @@ defmodule Mix.Tasks.Docs do
|> normalize_formatters()
|> put_package(config)

Code.prepend_path(options[:source_beam])

for path <- Keyword.get_values(options, :paths),
path <- Path.wildcard(path) do
Code.prepend_path(path)
end

Mix.shell().info("Generating docs...")

for formatter <- options[:formatters] do
index = generator.(project, version, Keyword.put(options, :formatter, formatter))
Mix.shell().info([:green, "View #{inspect(formatter)} docs at #{inspect(index)}"])
results =
for formatter <- options[:formatters] do
index = generator.(project, version, Keyword.put(options, :formatter, formatter))
Mix.shell().info([:green, "View #{inspect(formatter)} docs at #{inspect(index)}"])

if cli_opts[:open] do
browser_open(index)
if cli_opts[:open] do
browser_open(index)
end

if options[:warnings_as_errors] == true and ExDoc.Utils.warned?() do
{:error, %{reason: :warnings_as_errors, formatter: formatter}}
else
{:ok, index}
end
end

index
error_results = Enum.filter(results, &(elem(&1, 0) == :error))

if error_results == [] do
Enum.map(results, fn {:ok, value} -> value end)
else
formatters = Enum.map(error_results, &elem(&1, 1).formatter)

format_message =
case formatters do
[formatter] -> "#{formatter} format"
_ -> "#{Enum.join(formatters, ", ")} formats"
end

message =
"Documents have been generated, but generation for #{format_message} failed due to warnings while using the --warnings-as-errors option."

message_formatted = IO.ANSI.format([:red, message, :reset])

IO.puts(:stderr, message_formatted)

exit({:shutdown, 1})
Comment on lines +430 to +434
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
message_formatted = IO.ANSI.format([:red, message, :reset])
IO.puts(:stderr, message_formatted)
exit({:shutdown, 1})
message_formatted = IO.ANSI.format([:red, message, :reset])
IO.puts(:stderr, message_formatted)
exit({:shutdown, 1})

end
end

Expand Down
49 changes: 48 additions & 1 deletion test/ex_doc/formatter/epub_test.exs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
defmodule ExDoc.Formatter.EPUBTest do
use ExUnit.Case, async: true
use ExUnit.Case, async: false

import ExUnit.CaptureIO

alias ExDoc.Utils

@moduletag :tmp_dir

Expand Down Expand Up @@ -237,4 +241,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
Utils.unset_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 Utils.warned?() == true
end

test "warnings are registered even with warnings_as_errors: false", context do
Utils.unset_warned()

capture_io(:stderr, fn ->
generate_docs(
doc_config(context,
skip_undefined_reference_warnings_on: [],
warnings_as_errors: false
)
)
end)

assert Utils.warned?() == true
end
end
end
Loading
Loading