Make defstruct work on quoted expressions, closes #2156

Author: José Valim

lib/elixir/lib/kernel.ex

@@ -3197,33 +3197,45 @@ defmodule Kernel do
 
   To define a struct, a developer needs to only define
   a function named `__struct__/0` that returns a map with the
-  structs field. This macro is simply a convenience for doing so.
-
-  Finally, this macro also defines a type t in the current
-  module unless one was previously defined.
+  structs field. This macro is a convenience for doing such
+  function and a type `t` in one pass.
 
   ## Examples
 
       defmodule MyRange do
         defstruct first: nil, last: nil
       end
 
+  Notice `defstruct` requires a keyword list of fields and values
+  at expansion time. In other words, `defstruct` works with quoted
+  expressions. In other words, a struct defined like:
+
+      defmodule MyRange do
+        defstruct first: nil, last: 1 + 1
+      end
+
+  Will execute `1 + 1` every time the struct is built. This also
+  implies the following leads to an error:
+
+      defmodule MyRange do
+        my_fields = [first: nil, last: nil]
+        defstruct my_fields
+      end
+
   """
-  defmacro defstruct(opts) do
-    quote bind_quoted: [opts: opts] do
-      opts = Enum.map(opts, fn
-        { key, _ } = pair when is_atom(key) -> pair
-        key when is_atom(key) -> { key, nil }
-        other -> raise ArgumentError, message: "struct fields must be atoms, got: #{inspect other}"
-      end)
+  defmacro defstruct(kv) do
+    kv = Macro.escape(kv, unquote: true)
+    quote bind_quoted: [kv: kv] do
+      # TODO: Use those types once we support maps typespecs.
+      { fields, _types } = Record.Backend.split_fields_and_types(:defstruct, kv)
 
       if :code.ensure_loaded(Kernel.Typespec) == { :module, Kernel.Typespec } and
          not Kernel.Typespec.defines_type?(__MODULE__, :t, 0) do
         @type t :: map
       end
 
       def __struct__() do
-        %{ unquote_splicing(Macro.escape(opts)), __struct__: __MODULE__ }
+        %{ unquote_splicing(fields), __struct__: __MODULE__ }
       end
     end
   end

lib/elixir/lib/record.ex

@@ -172,7 +172,7 @@ defmodule Record do
 
   """
   def extract(name, opts) do
-    Record.Extractor.retrieve(name, opts)
+    Record.Extractor.extract(name, opts)
   end
 
   @doc false

lib/elixir/lib/record/backend.ex

@@ -0,0 +1,37 @@
+defmodule Record.Backend do
+  # Callback functions invoked by defrecord, defrecordp and friends.
+  @moduledoc false
+
+  @doc """
+  Splits a keywords list into fields and types.
+
+  This logic is shared by records and structs.
+  """
+  def split_fields_and_types(tag, kv) when is_list(kv) do
+    split_fields_and_types(tag, kv, [], [])
+  end
+
+  def split_fields_and_types(tag, other) do
+    raise ArgumentError, message: "#{tag} fields must be a keyword list, got: #{Macro.to_string other}"
+  end
+
+  defp split_fields_and_types(tag, [{ field, { :::, _, [default, type] }}|t], fields, types) do
+    split_fields_and_types(tag, t, [{ field, default }|fields], [{ field, type }|types])
+  end
+
+  defp split_fields_and_types(tag, [{ field, default }|t], fields, types) when is_atom(field) do
+    split_fields_and_types(tag, t, [{ field, default }|fields], [{ field, quote(do: term) }|types])
+  end
+
+  defp split_fields_and_types(tag, [field|t], fields, types) when is_atom(field) do
+    split_fields_and_types(tag, t, [{ field, nil }|fields], [{ field, quote(do: term) }|types])
+  end
+
+  defp split_fields_and_types(tag, [other|_], _fields, _types) do
+    raise ArgumentError, message: "#{tag} fields must be atoms, got: #{Macro.to_string other}"
+  end
+
+  defp split_fields_and_types(_tag, [], fields, types) do
+    { :lists.reverse(fields), :lists.reverse(types) }
+  end
+end

lib/elixir/lib/record/extractor.ex

@@ -3,7 +3,7 @@ defmodule Record.Extractor do
 
   # Retrieve a record definition from an Erlang file using
   # the same lookup as the *include* attribute from Erlang modules.
-  def retrieve(name, from: file) when is_binary(file) do
+  def extract(name, from: file) when is_binary(file) do
     file = String.to_char_list!(file)
 
     realfile =
@@ -12,35 +12,35 @@ defmodule Record.Extractor do
         realfile -> realfile
       end
 
-    retrieve_record(name, realfile)
+    extract_record(name, realfile)
   end
 
   # Retrieve a record definition from an Erlang file using
   # the same lookup as the *include_lib* attribute from Erlang modules.
-  def retrieve(name, from_lib: file) when is_binary(file) do
+  def extract(name, from_lib: file) when is_binary(file) do
     [app|path] = :filename.split(String.to_char_list!(file))
 
     case :code.lib_dir(list_to_atom(app)) do
       { :error, _ } ->
         raise ArgumentError, message: "lib file #{file} could not be found"
       libpath ->
-        retrieve_record name, :filename.join([libpath|path])
+        extract_record name, :filename.join([libpath|path])
     end
   end
 
   # Retrieve the record with the given name from the given file
-  defp retrieve_record(name, file) do
+  defp extract_record(name, file) do
     form = read_file(file)
-    records = retrieve_records(form)
+    records = extract_records(form)
     if record = List.keyfind(records, name, 0) do
       parse_record(record, form)
     else
       raise ArgumentError, message: "no record #{name} found at #{file}"
     end
   end
 
-  # Parse the given file and retrieve all existent records.
-  defp retrieve_records(form) do
+  # Parse the given file and extract all existent records.
+  defp extract_records(form) do
     for { :attribute, _, :record, record } <- form, do: record
   end
 

lib/elixir/src/elixir_compiler.erl

@@ -175,6 +175,7 @@ core_main() ->
     <<"lib/elixir/lib/module.ex">>,
     <<"lib/elixir/lib/list.ex">>,
     <<"lib/elixir/lib/record.ex">>,
+    <<"lib/elixir/lib/record/backend.ex">>,
     <<"lib/elixir/lib/macro.ex">>,
     <<"lib/elixir/lib/macro/env.ex">>,
     <<"lib/elixir/lib/exception.ex">>,

lib/elixir/src/elixir_map.erl

@@ -52,7 +52,7 @@ translate_map(Meta, Args, S) ->
 
 translate_struct(Meta, Name, { '%{}', MapMeta, Args }, S) ->
   { Assocs, TUpdate, US } = extract_assoc_update(Args, S),
-  Struct = load_struct(Meta, Name),
+  Struct = load_struct(Meta, Name, S),
 
   case is_map(Struct) of
     true  ->
@@ -89,24 +89,31 @@ translate_struct(Meta, Name, { '%{}', MapMeta, Args }, S) ->
 
 %% Helpers
 
-load_struct(Meta, Name) ->
+load_struct(Meta, Name, S) ->
   Local =
     elixir_module:is_open(Name) andalso
     (case lists:keyfind(struct, 1, Meta) of
       { struct, context } -> true;
       _ -> wait_for_struct(Name)
     end),
 
-  case Local of
-    true ->
-      try
-        (elixir_locals:local_for(Name, '__struct__', 0, def))()
-      catch
-        error:undef  -> Name:'__struct__'();
-        error:badarg -> Name:'__struct__'()
-      end;
-    false ->
-      Name:'__struct__'()
+  try
+    case Local of
+      true ->
+        try
+          (elixir_locals:local_for(Name, '__struct__', 0, def))()
+        catch
+          error:undef  -> Name:'__struct__'();
+          error:badarg -> Name:'__struct__'()
+        end;
+      false ->
+        Name:'__struct__'()
+    end
+  catch
+    error:undef ->
+      Inspected = elixir_aliases:inspect(Name),
+      compile_error(Meta, S#elixir_scope.file, "~ts.__struct__/0 is undefined, "
+        "cannot expand struct ~ts", [Inspected, Inspected])
   end.
 
 wait_for_struct(Module) ->

lib/elixir/test/elixir/kernel/errors_test.exs

@@ -195,6 +195,16 @@ defmodule Kernel.ErrorsTest do
       '%{ :a, :b }{ a: :b }'
   end
 
+  test :struct_fields_on_defstruct do
+    assert_compile_fail ArgumentError,
+      "defstruct fields must be a keyword list, got: my_fields",
+      '''
+      defmodule TZ do
+        defstruct my_fields
+      end
+      '''
+  end
+
   test :struct_access_on_body do
     assert_compile_fail CompileError,
       "nofile:3: cannot access struct TZ in body of the module that defines it " <>
@@ -218,8 +228,8 @@ defmodule Kernel.ErrorsTest do
   end
 
   test :struct_errors do
-    assert_compile_fail UndefinedFunctionError,
-      "undefined function: BadStruct.__struct__/0",
+    assert_compile_fail CompileError,
+      "nofile:1: BadStruct.__struct__/0 is undefined, cannot expand struct BadStruct",
       '%BadStruct{}'
 
     defmodule BadStruct do

lib/elixir/test/elixir/map_test.exs

@@ -125,7 +125,11 @@ defmodule MapTest do
   end
 
   defmodule LocalUser do
-    defstruct name: "josé"
+    defmodule NestedUser do
+      defstruct []
+    end
+
+    defstruct name: "josé", nested: %NestedUser{}
 
     def new do
       %LocalUser{}
@@ -139,8 +143,8 @@ defmodule MapTest do
   end
 
   test "local user" do
-    assert LocalUser.new == %LocalUser{name: "josé"}
-    assert LocalUser.Context.new == %LocalUser{name: "josé"}
+    assert LocalUser.new == %LocalUser{ name: "josé", nested: %LocalUser.NestedUser{} }
+    assert LocalUser.Context.new == %LocalUser{ name: "josé", nested: %LocalUser.NestedUser{} }
   end
 
   defmodule NilUser do