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

Macro calls placement in a module #198

Open
virtual-light opened this issue Oct 19, 2020 · 1 comment
Open

Macro calls placement in a module #198

virtual-light opened this issue Oct 19, 2020 · 1 comment
Labels

Comments

@virtual-light
Copy link

virtual-light commented Oct 19, 2020

Is there some rule about macro calls placement in a module?

For example imagine such module:

defmodule MyApp.Schema do
  use Ecto.Schema

  import EctoEnum 

  defenum Status, :status, ~w(active disabled)a
  defenum State, :state, ~w(new accepted cancelled finished)a

  @required ~w(name number)a
  @optional ~w(description)a

  @type t() :: %__MODULE__{}
  @type id() :: pos_integer()

  schema "schema" do
    field :name, :string
    field :description, :string
    field :number, :integer
    field :status, Status
    field :state, State
  end

  def changeset(schema, params) do
  # ...
end

What is correct placement for defenum and schema?

From my intuition macro calls placement depends on what code they inject.
The schema block creates struct so it should have same placement as defstruct block while defenum creates nested module so should have same placement as defmodule.

From such perspective and taking into account current version of this styleguide, module layout should be reorganized in such way:

defmodule MyApp.Schema do
 use Ecto.Schema

 import EctoEnum 

 @required ~w(name number)a
 @optional ~w(description)a

 schema "schema" do
   field :name, :string
   field :description, :string
   field :number, :integer
   field :status, Status
   field :state, State
 end

 @type t() :: %__MODULE__{}
 @type id() :: pos_integer()

 defenum Status, :status, ~w(active disabled)a
 defenum State, :state, ~w(new accepted cancelled finished)a

 def changeset(schema, params) do
 # ...
end

However if you try to compile something like this it fails with error:

== Compilation error in file lib/my_app/schema.ex ==
** (ArgumentError) invalid or unknown type Title for field :title
    lib/ecto/schema.ex:2037: Ecto.Schema.check_field_type!/3
    lib/ecto/schema.ex:1745: Ecto.Schema.__field__/4
    lib/my_app/schema.ex:13: (module)
    (stdlib 3.12.1) erl_eval.erl:680: :erl_eval.do_apply/6

which means that my intuition about macro calls placement is wrong or at least priority of nested module block should be higher than defstruct block.

Maybe this should be discussed in separate issue but current version of the guide says that defstruct priority is much higher than defmodule what looks inconsistent from my point of view because code like this

defmodule Example
  defstruct origin: __MODULE__, callback_module: Callback 

  defmodule Callback do
  #  ...
  end
end

can't compile even if you use full module name like callback_module: __MODULE__.Callback.

To summarize, this issue has such questions

  • Should defmodule block have lower priority than defstruct?
  • What is correct macro calls placement in a module and should this guide contain some rules about it?
@virtual-light virtual-light changed the title Macro call placement in a module Macro calls placement in a module Oct 19, 2020
@christopheradams
Copy link
Owner

Things do get tricky with macros, but this guide can't anticipate how every library's macros should be laid out. In the case of Ecto, I'd follow their documentation or other popular projects that use the library.

If the recommended order of the module affects compilation in a particular case, I'd treat that as an exception rather than changing the rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants