-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add MissingDependenciesError exception #13689
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
Changes from all commits
699b4ed
2c4008a
92e6635
e593af1
ad344ed
3ecae69
b95daaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2131,6 +2131,56 @@ defmodule UnicodeConversionError do | |
end | ||
end | ||
|
||
defmodule MissingApplicationsError do | ||
@moduledoc """ | ||
An exception that is raised when an application depends on one or more | ||
missing applications. | ||
|
||
This exception is used by Mix and other tools. It can also be used by library authors | ||
when their library only requires an external application (like a dependency) for a subset | ||
of features. | ||
|
||
The fields of this exception are public. See `t:t/0`. | ||
|
||
*Available since v1.18.0.* | ||
|
||
## Examples | ||
|
||
unless Application.spec(:plug, :vsn) do | ||
raise MissingApplicationsError, | ||
description: "application :plug is required for testing Plug-related functionality", | ||
apps: [{:plug, "~> 1.0"}] | ||
end | ||
|
||
""" | ||
|
||
@moduledoc since: "1.18.0" | ||
|
||
@type t() :: %__MODULE__{ | ||
apps: [{Application.app(), Version.requirement()}, ...], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mentioned earlier that a potential use case for this is raising on missing Elixir/OTP apps. If so, apps being non-empty makes it a bit awkward imo: raise MissingApplicationsError,
description: "please add :ssl to :extra_applications",
apps: [{:ssl, ">= 0.0.0"}]
It's OK if this exception is not intended for that. Alternatively, allow :apps to be empty? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can detect if the application comes from OTP and change the message accordingly. So no concerns here. |
||
description: String.t() | ||
whatyouhide marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
defexception apps: [], description: "missing applications found" | ||
|
||
@impl true | ||
def message(%__MODULE__{apps: apps, description: description}) do | ||
# We explicitly format these as tuples so that they're easier to copy-paste | ||
# into dependencies. | ||
formatted_apps = | ||
Enum.map(apps, fn {app_name, requirement} -> | ||
~s(\n {#{inspect(app_name)}, "#{requirement}"}) | ||
end) | ||
|
||
""" | ||
#{description} | ||
|
||
To address this, include these applications as your dependencies: | ||
#{formatted_apps}\ | ||
""" | ||
end | ||
end | ||
|
||
defmodule Enum.OutOfBoundsError do | ||
@moduledoc """ | ||
An exception that is raised when a function expects an enumerable to have | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -954,6 +954,21 @@ defmodule ExceptionTest do | |
test "ErlangError" do | ||
assert %ErlangError{original: :sample} |> message == "Erlang error: :sample" | ||
end | ||
|
||
test "MissingApplicationsError" do | ||
assert %MissingApplicationsError{ | ||
apps: [{:logger, "~> 1.18"}, {:ex_unit, Version.parse_requirement!(">= 0.0.0")}], | ||
description: "applications are required" | ||
} | ||
|> message == """ | ||
applications are required | ||
|
||
To address this, include these applications as your dependencies: | ||
|
||
{:logger, "~> 1.18"} | ||
{:ex_unit, ">= 0.0.0"}\ | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is my suggestion for the raising:
and it should raise:
I would keep them as tuples, because it is easier to copy and paste. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good call. |
||
end | ||
end | ||
|
||
describe "error_info" do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, we have gone through some many bikesheds. I think last we talked it was meant to be singular? MissingApplicationError? I am fine either way, we should just double check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We agreed on plural IIRC, because we have a list of apps in the exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought some found it weird because we don’t have many plurals in module names. Your call!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it's still the singular
Error
so let's go withMissingApplicationsError
😄