-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add local_for_callback option to Macro.Env.expand_import #14620
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 6 commits
54cabc4
997eb4c
884e989
f9c418d
201952d
1be2d9d
c80ffa2
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 |
|---|---|---|
|
|
@@ -96,7 +96,7 @@ defmodule Macro.Env do | |
| ] | ||
|
|
||
| @type expand_import_opts :: [ | ||
| allow_locals: boolean(), | ||
| allow_locals: boolean() | (Macro.metadata(), atom(), arity(), [atom()], t() -> any()), | ||
| check_deprecations: boolean(), | ||
| trace: boolean() | ||
| ] | ||
|
|
@@ -555,8 +555,17 @@ defmodule Macro.Env do | |
|
|
||
| ## Options | ||
|
|
||
| * `:allow_locals` - when set to `false`, it does not attempt to capture | ||
| local macros defined in the current module in `env` | ||
| * `:allow_locals` - controls how local macros are resolved. | ||
| Defaults to `true`. | ||
|
|
||
| - When `false`, does not attempt to capture local macros defined in the | ||
| current module in `env` | ||
| - When `true`, uses a default resolver that looks for public macros in | ||
| the current module | ||
| - When a function, uses the function as a custom local resolver. The function | ||
| must have the signature: `(meta, name, arity, kinds, env) -> function() | false` | ||
|
||
| where `kinds` is a list of atoms indicating the types of symbols being | ||
| searched (e.g., `[:defmacro, :defmacrop]`) | ||
|
|
||
| * `:check_deprecations` - when set to `false`, does not check for deprecations | ||
| when expanding macros | ||
|
|
@@ -580,10 +589,16 @@ defmodule Macro.Env do | |
| trace = Keyword.get(opts, :trace, true) | ||
| module = env.module | ||
|
|
||
| # When allow_locals is a callback, we don't need to pass module macros as extra | ||
| # because the callback will handle local macro resolution | ||
| extra = | ||
| case allow_locals and function_exported?(module, :__info__, 1) do | ||
| true -> [{module, module.__info__(:macros)}] | ||
| false -> [] | ||
| if is_function(allow_locals, 5) do | ||
| [] | ||
| else | ||
| case allow_locals and function_exported?(module, :__info__, 1) do | ||
| true -> [{module, module.__info__(:macros)}] | ||
josevalim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| false -> [] | ||
| end | ||
| end | ||
|
|
||
| case :elixir_dispatch.expand_import(meta, name, arity, env, extra, allow_locals, trace) 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.
Do we return
anyor does it have to be AST?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.
Actually the return type is
false | function()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.
Ah, so we should update it!
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