-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix dialyzer opaqueness warnings on module attrs in OTP28 #14755
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
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 |
---|---|---|
|
@@ -808,6 +808,10 @@ defmodule Macro do | |
nodes. Note this option changes the semantics of escaped code and | ||
it should only be used when escaping ASTs. Defaults to `false`. | ||
* `:generated` - (since v1.19.0) Whether the AST should be considered as generated | ||
by the compiler or not. This means the compiler and tools like Dialyzer may not | ||
emit certain warnings. | ||
Comment on lines
+811
to
+813
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. Basically the same as https://hexdocs.pm/elixir/1.19.0-rc.0/Macro.html#t:metadata/0 |
||
As an example for `:prune_metadata`, `ExUnit` stores the AST of every | ||
assertion, so when an assertion fails we can show code snippets to users. | ||
Without this option, each time the test module is compiled, we would get a | ||
|
@@ -908,7 +912,17 @@ defmodule Macro do | |
def escape(expr, opts \\ []) do | ||
unquote = Keyword.get(opts, :unquote, false) | ||
kind = if Keyword.get(opts, :prune_metadata, false), do: :prune_metadata, else: :none | ||
:elixir_quote.escape(expr, kind, unquote) | ||
generated = Keyword.get(opts, :generated, false) | ||
|
||
case :elixir_quote.escape(expr, kind, unquote) do | ||
# mark module attrs as shallow-generated since the ast for their representation | ||
# might contain opaque terms | ||
{caller, meta, args} when generated and is_list(meta) -> | ||
{caller, [generated: true] ++ meta, args} | ||
Comment on lines
+920
to
+921
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. Should we wrap with a 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 think there is no need, yeah. |
||
|
||
ast -> | ||
ast | ||
end | ||
end | ||
|
||
# TODO: Deprecate me on Elixir v1.22 | ||
|
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.
@sabiwara the new options should be included in
escape_opts
typeThere 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.
Wooops, thank you for catching this one ! 💜
Will fix
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.
Fixed and backported #14761 ✅