Merge pull request #1740 from yrashk/record-string-fields

Allow for string names when instantiating or updating records

Author: José Valim

lib/elixir/lib/record.ex

@@ -139,6 +139,12 @@ defmodule Record do
   records flexibility at the cost of performance since
   there is more work happening at runtime.
 
+  The above calls (new and update) can interchangeably accept both
+  atom and string keys for field names, however not both at the same time.
+  Please also note that atom keys are faster. This feature allows to
+  "sanitize" untrusted dictionaries and initialize/update records without
+  using `binary_to_existing_atom/1`.
+
   To sum up, `defrecordp` should be used when you don't want
   to expose the record information while `defrecord` should be used
   whenever you want to share a record within your code or with other
@@ -624,17 +630,27 @@ defmodule Record do
     # an ordered dict of options (opts) and it will try to fetch
     # the given key from the ordered dict, falling back to the
     # default value if one does not exist.
-    selective = lc { k, v } inlist values do
+    atom_selective = lc { k, v } inlist values do
       quote do: Keyword.get(opts, unquote(k), unquote(v))
     end
+    string_selective = lc { k, v } inlist values do
+      k = atom_to_binary(k)
+      quote do 
+        case :lists.keyfind(unquote(k), 1, opts) do
+          false -> unquote(v)
+          {_, v} -> v
+        end
+      end
+    end
 
     quote do
       @doc false
       def new(), do: new([])
 
       @doc false
       def new([]), do: { __MODULE__, unquote_splicing(defaults) }
-      def new(opts) when is_list(opts), do: { __MODULE__, unquote_splicing(selective) }
+      def new([{key, _}|_] = opts) when is_atom(key), do: { __MODULE__, unquote_splicing(atom_selective) }
+      def new([{key, _}|_] = opts) when is_binary(key), do: { __MODULE__, unquote_splicing(string_selective) }
     end
   end
 
@@ -725,24 +741,38 @@ defmodule Record do
   # Define an updater method that receives a
   # keyword list and updates the record.
   defp updater(values) do
-    fields =
+    atom_fields = 
+      lc {key, _default} inlist values do
+        index = find_index(values, key, 1)
+        quote do: Keyword.get(keywords, unquote(key), elem(record, unquote(index)))
+      end
+
+    string_fields =
       lc {key, _default} inlist values do
         index = find_index(values, key, 1)
+        key = atom_to_binary(key)
         quote do
-          Keyword.get(keywords, unquote(key), elem(record, unquote(index)))
+          case :lists.keyfind(unquote(key), 1, keywords) do
+            false -> elem(record, unquote(index))
+            {_, value} -> value
+          end
         end
       end
 
-    contents = quote do: { __MODULE__, unquote_splicing(fields) }
+    atom_contents = quote do: { __MODULE__, unquote_splicing(atom_fields) }
+    string_contents = quote do: { __MODULE__, unquote_splicing(string_fields) }
 
     quote do
       @doc false
       def update([], record) do
         record
       end
 
-      def update(keywords, record) do
-        unquote(contents)
+      def update([{key, _}|_] = keywords, record) when is_atom(key) do
+        unquote(atom_contents)
+      end
+      def update([{key, _}|_] = keywords, record) when is_binary(key) do
+        unquote(string_contents)
       end
     end
   end
@@ -765,14 +795,15 @@ defmodule Record do
   defp core_specs(values) do
     types   = lc { _, _, spec } inlist values, do: spec
     options = if values == [], do: [], else: [options_specs(values)]
+    values_specs = if values == [], do: [], else: values_specs(values)
 
     quote do
       unless Kernel.Typespec.defines_type?(__MODULE__, :t, 0) do
         @type t :: { __MODULE__, unquote_splicing(types) }
       end
 
       unless Kernel.Typespec.defines_type?(__MODULE__, :options, 0) do
-        @type options :: unquote(options)
+        @type options :: unquote(options) | [{String.t, unquote(values_specs)}]
       end
 
       @spec new :: t
@@ -790,6 +821,11 @@ defmodule Record do
       { :|, [], [{ k, v }, acc] }
     end, { k, v }, t
   end
+  defp values_specs([{ _, _, v }|t]) do
+    :lists.foldl fn { _, _, v }, acc ->
+      { :|, [], [v, acc] }
+    end, v, t
+  end
 
   defp accessor_specs([{ :__exception__, _, _ }|t], 1, acc) do
     accessor_specs(t, 2, acc)

lib/elixir/test/elixir/record_test.exs

@@ -70,6 +70,8 @@ defmodule RecordTest.Macros do
   defrecord Record, [a: 1, b: 2]
 end
 
+defrecord RecordTest.FooTest, foo: nil, bar: nil 
+
 defmodule RecordTest do
   use ExUnit.Case, async: true
 
@@ -162,6 +164,22 @@ defmodule RecordTest do
     assert is_record(namespace, :xmlNamespace)
   end
 
+  test :string_names do
+    a = RecordTest.FooTest.new([{"foo", 1}, {"bar", 1}])
+    assert a.foo == 1
+    assert a.bar == 1
+    a = a.update([{"foo", 2}, {"bar", 2}])
+    assert a.foo == 2
+    assert a.bar == 2
+  end
+
+  test :string_names_import do
+    record   = RecordTest.FileInfo.new([{"type", :regular}, {"access", 100}])
+    assert record.type == :regular
+    assert record.access == 100
+    assert record.update([{"access", 101}]).access == 101
+  end
+
   defp empty_tuple, do: {}
   defp a_tuple, do: { :foo, :bar, :baz }
   defp a_list,  do: [ :foo, :bar, :baz ]