Typedefs in other modules weren't working properly (#275)

* Typedefs in other modules weren't working properly

Due to the way typedefs work (they're different than everything else),
resolution wasn't working properly on them, this commit adds proper
resolution so you can define them in remote modules.

Fixes #268

lib/thrift.ex

@@ -10,7 +10,7 @@ defmodule Thrift do
 
   @typedoc "Thrift data types"
   @type data_type ::
-    :bool | :byte | :i16 | :i32 | :i64 | :double | :string |
+    :bool | :byte | :i8 | :i16 | :i32 | :i64 | :double | :string | :binary |
     {:map, data_type, data_type} | {:set, data_type} | {:list, data_type}
 
   @type i8   :: (-128..127)
@@ -21,4 +21,12 @@ defmodule Thrift do
 
   @typedoc "Thrift message types"
   @type message_type :: :call | :reply | :exception | :oneway
+
+  @doc """
+  Returns a list of atoms, each of which is a name of a Thrift primitive type.
+  """
+  @spec primitive_names() :: [Thrift.Parser.Types.Primitive.t]
+  def primitive_names do
+    [:bool, :i8, :i16, :i32, :i64, :binary, :double, :byte, :string]
+  end
 end

lib/thrift/generator/utils.ex

@@ -259,6 +259,12 @@ defmodule Thrift.Generator.Utils do
       MapSet.new(unquote(values))
     end
   end
+  def quote_value(set_elements, {:set, type}, schema) do
+    values = Enum.map(set_elements, &quote_value(&1, type, schema))
+    quote do
+      MapSet.new(unquote(values))
+    end
+  end
   def quote_value(list, {:list, type}, schema) when is_list(list) do
     Enum.map(list, &quote_value(&1, type, schema))
   end

lib/thrift/parser/file_group.ex

@@ -124,32 +124,33 @@ defmodule Thrift.Parser.FileGroup do
   end
 
   @spec resolve(t, any) :: any
-  for type <- [:bool, :byte, :i8, :i16, :i32, :i64, :double, :string, :binary] do
+  for type <- Thrift.primitive_names do
     def resolve(_, unquote(type)), do: unquote(type)
   end
-  def resolve(%FileGroup{} = group, %Field{type: %TypeRef{} = ref} = field) do
-    %Field{field | type: resolve(group, ref)}
+  def resolve(%FileGroup{} = group, %Field{type: type} = field) do
+    %Field{field | type: resolve(group, type)}
   end
-  def resolve(%FileGroup{} = group, %Field{type: {:list, elem_type}} = field) do
-    %Field{field | type: {:list, resolve(group, elem_type)}}
+  def resolve(%FileGroup{resolutions: resolutions} = group, %TypeRef{referenced_type: type_name}) do
+    resolve(group, resolutions[type_name])
   end
-  def resolve(%FileGroup{} = group, %Field{type: {:set, elem_type}} = field) do
-    %Field{field | type: {:set, resolve(group, elem_type)}}
+  def resolve(%FileGroup{resolutions: resolutions} = group, %ValueRef{referenced_value: value_name}) do
+    resolve(group, resolutions[value_name])
   end
-  def resolve(%FileGroup{} = group, %Field{type: {:map, {key_type, val_type}}} = field) do
-    %Field{field | type: {:map, {resolve(group, key_type), resolve(group, val_type)}}}
-  end
-  def resolve(%FileGroup{resolutions: resolutions}, %TypeRef{referenced_type: type_name}) do
-    resolutions[type_name]
-  end
-  def resolve(%FileGroup{resolutions: resolutions}, %ValueRef{referenced_value: value_name}) do
-    resolutions[value_name]
-  end
-  def resolve(%FileGroup{resolutions: resolutions}, path) when is_atom(path) do
+  def resolve(%FileGroup{resolutions: resolutions} = group, path) when is_atom(path) and not is_nil(path) do
     # this can resolve local mappings like :Weather or
     # remote mappings like :"common.Weather"
-    resolutions[path]
+    resolve(group, resolutions[path])
   end
+  def resolve(%FileGroup{} = group, {:list, elem_type}) do
+    {:list, resolve(group, elem_type)}
+  end
+  def resolve(%FileGroup{} = group, {:set, elem_type}) do
+    {:set, resolve(group, elem_type)}
+  end
+  def resolve(%FileGroup{} = group, {:map, {key_type, val_type}}) do
+    {:map, {resolve(group, key_type), resolve(group, val_type)}}
+  end
+
   def resolve(_, other) do
     other
   end
@@ -185,13 +186,15 @@ defmodule Thrift.Parser.FileGroup do
     # (ignoring case), use that instead to avoid generating two modules with
     # the same spellings but different cases.
     schema = file_group.schemas[base]
-    symbols = Enum.map(List.flatten([
+    symbols = [
       Enum.map(schema.enums, fn {_, s} -> s.name end),
       Enum.map(schema.exceptions, fn {_, s} -> s.name end),
       Enum.map(schema.structs, fn {_, s} -> s.name end),
       Enum.map(schema.services, fn {_, s} -> s.name end),
       Enum.map(schema.unions, fn {_, s} -> s.name end)
-    ]), &Atom.to_string/1)
+    ]
+    |> List.flatten
+    |> Enum.map(&Atom.to_string/1)
 
     target = String.downcase(default)
     name = Enum.find(symbols, default, fn s -> String.downcase(s) == target end)

lib/thrift/parser/models.ex

@@ -470,35 +470,134 @@ defmodule Thrift.Parser.Models do
     end
 
     defp merge(schema, %TEnum{} = enum) do
-      %Schema{schema | enums: put_new_strict(schema.enums, enum.name, canonicalize_name(schema, enum))}
+      %Schema{schema | enums: put_new_strict(schema.enums, enum.name, add_namespace_to_name(schema.module, enum))}
     end
 
     defp merge(schema, %Exception{} = exc) do
-      %Schema{schema | exceptions: put_new_strict(schema.exceptions, exc.name, canonicalize_name(schema, exc))}
+      fixed_fields = schema.module
+      |> add_namespace_to_name(exc)
+      |> add_namespace_to_fields()
+
+      %Schema{schema | exceptions: put_new_strict(schema.exceptions, exc.name, fixed_fields)}
     end
 
     defp merge(schema, %Struct{} = s) do
-      %Schema{schema | structs: put_new_strict(schema.structs, s.name, canonicalize_name(schema, s))}
+      fixed_fields = schema.module
+      |> add_namespace_to_name(s)
+      |> add_namespace_to_fields()
+
+      %Schema{schema | structs: put_new_strict(schema.structs, s.name, fixed_fields)}
     end
 
     defp merge(schema, %Union{} = union) do
-      %Schema{schema | unions: put_new_strict(schema.unions, union.name, canonicalize_name(schema, union))}
+      fixed_fields = schema.module
+      |> add_namespace_to_name(union)
+      |> add_namespace_to_fields()
+
+      %Schema{schema | unions: put_new_strict(schema.unions, union.name, fixed_fields)}
     end
 
     defp merge(schema, %Service{} = service) do
-      %Schema{schema | services: put_new_strict(schema.services, service.name, canonicalize_name(schema, service))}
+      %Schema{schema | services: put_new_strict(schema.services, service.name, add_namespace_to_name(schema.module, service))}
     end
 
     defp merge(schema, {:typedef, actual_type, type_alias}) do
-      %Schema{schema | typedefs: put_new_strict(schema.typedefs, atomify(type_alias), actual_type)}
+      %Schema{schema | typedefs: put_new_strict(schema.typedefs, atomify(type_alias), add_namespace_to_type(schema.module, actual_type))}
     end
 
-    defp canonicalize_name(%{module: nil}, model) do
+    defp add_namespace_to_name(nil, model) do
       model
     end
+    defp add_namespace_to_name(module, %{name: name} = model) do
+      %{model | name: add_namespace_to_type(module, name)}
+    end
 
-    defp canonicalize_name(schema, %{name: name} = model) do
-      %{model | name: :"#{schema.module}.#{name}"}
+    defp add_namespace_to_type(module, %TypeRef{referenced_type: t} = type) do
+      %TypeRef{type | referenced_type: add_namespace_to_type(module, t)}
+    end
+    defp add_namespace_to_type(module, {:set, elem_type}) do
+      {:set, add_namespace_to_type(module, elem_type)}
+    end
+    defp add_namespace_to_type(module, {:list, elem_type}) do
+      {:list, add_namespace_to_type(module, elem_type)}
+    end
+    defp add_namespace_to_type(module, {:map, {key_type, val_type}}) do
+      {:map, {add_namespace_to_type(module, key_type), add_namespace_to_type(module, val_type)}}
+    end
+    for type <- Thrift.primitive_names do
+      defp add_namespace_to_type(_, unquote(type)) do
+        unquote(type)
+      end
+    end
+    defp add_namespace_to_type(module, type_name) when is_atom(type_name) do
+      split_type_name = type_name
+      |> Atom.to_string
+      |> String.split(".")
+
+      case split_type_name do
+        [^module | _rest] ->
+          # this case accounts for types that already have the current module in them
+          type_name
+
+        _ ->
+          :"#{module}.#{type_name}"
+
+      end
+    end
+
+    defp add_namespace_to_fields(%{fields: fields} = model) do
+      %{model | fields: Enum.map(fields, &add_namespace_to_field/1)}
+    end
+
+    defp add_namespace_to_field(%Field{default: nil} = field) do
+      field
+    end
+    defp add_namespace_to_field(%Field{default: default, type: type} = field) do
+      %Field{field | default: add_namespace_to_defaults(type, default)}
+    end
+
+    defp add_namespace_to_defaults({:list, elem_type}, defaults) when is_list(defaults) do
+
+      for elem <- defaults do
+        add_namespace_to_defaults(elem_type, elem)
+      end
+    end
+    defp add_namespace_to_defaults({:set, elem_type}, %MapSet{} = defaults) do
+      for elem <- defaults, into: MapSet.new do
+        add_namespace_to_defaults(elem_type, elem)
+      end
+    end
+    defp add_namespace_to_defaults({:map, {_, _}}, %ValueRef{} = val) do
+      val
+    end
+    defp add_namespace_to_defaults({:map, {key_type, val_type}}, defaults) when is_map(defaults) do
+      for {key, val} <- defaults, into: %{} do
+        {add_namespace_to_defaults(key_type, key), add_namespace_to_defaults(val_type, val)}
+      end
+    end
+    defp add_namespace_to_defaults(%TypeRef{referenced_type: referenced_type}, %ValueRef{referenced_value: referenced_value} = val_ref) do
+      %ValueRef{val_ref | referenced_value: namespaced_module(referenced_type, referenced_value)}
+    end
+    defp add_namespace_to_defaults(%TypeRef{} = type, defaults) when is_list(defaults) do
+      for default_value <- defaults do
+        add_namespace_to_defaults(type, default_value)
+      end
+    end
+    defp add_namespace_to_defaults(ref, {key_type, val_type}) do
+      # this is used for a remote typedef that defines a map
+      {add_namespace_to_defaults(ref, key_type), add_namespace_to_defaults(ref, val_type)}
+    end
+    defp add_namespace_to_defaults(_t, val) do
+      val
+    end
+
+    defp namespaced_module(type, value) do
+      with string_val <- Atom.to_string(type),
+           [module, _value | _rest] <- String.split(string_val, ".") do
+        add_namespace_to_type(module, value)
+      else _ ->
+        value
+      end
     end
 
     defp put_new_strict(map, key, value) do

lib/thrift/parser/types.ex

@@ -3,7 +3,7 @@ defmodule Thrift.Parser.Types do
 
   defmodule Primitive do
     @moduledoc false
-    @type t :: :bool | :i8 | :i16 | :i64 | :binary | :double | :byte | :string
+    @type t :: :bool | :i8 | :i16 | :i32 | :i64 | :binary | :double | :byte | :string
   end
 
   defmodule Ident do

test/thrift/generator/binary_protocol_test.exs

@@ -637,4 +637,66 @@ defmodule Thrift.Generator.BinaryProtocolTest do
     assert binary == %Byte{val_set: MapSet.new([91])} |> Byte.serialize() |> IO.iodata_to_binary
     assert binary == %Byte{val_set: [91]            } |> Byte.serialize() |> IO.iodata_to_binary
   end
+
+  @thrift_file name: "additions.thrift", contents: """
+  enum ChocolateAdditionsType {
+    ALMONDS = 1,
+    NOUGAT  = 2,
+    HAIR    = 3
+  }
+
+  typedef set<ChocolateAdditionsType> ChocolateAdditions
+  typedef map<ChocolateAdditionsType, string> ChocolateMapping
+  """
+
+  @thrift_file name: "chocolate.thrift", contents: """
+  include "additions.thrift"
+
+  struct Chocolate {
+    1: optional additions.ChocolateAdditions extra_stuff = [ChocolateAdditionsType.HAIR]
+    2: optional additions.ChocolateAdditionsType secret_ingredient = ChocolateAdditionsType.HAIR
+  }
+
+  struct Allergies {
+     1: optional list<additions.ChocolateAdditionsType> may_contain = [ChocolateAdditionsType.ALMONDS]
+  }
+
+  struct OddSnackIngredients {
+     1: optional set<additions.ChocolateAdditionsType> other_things = [ChocolateAdditionsType.NOUGAT]
+  }
+
+  struct ChocoMappings {
+    1: optional map<additions.ChocolateAdditionsType, string> common_name = {ChocolateAdditionsType.HAIR: "love"}
+  }
+
+  struct AdditionalMappings {
+    1: optional additions.ChocolateMapping mapping = {ChocolateAdditionsType.ALMONDS: "almonds",
+                                                      ChocolateAdditionsType.NOUGAT: "nougat"}
+  }
+
+  struct AlreadyNamespaced {
+    1: optional additions.ChocolateAdditionsType namespaced = additions.ChocolateAdditionsType.ALMONDS
+  }
+  """
+
+  thrift_test "including a file with typedefs and defaults" do
+    choco = %Chocolate{extra_stuff: MapSet.new([1, 2])}
+
+    assert choco.secret_ingredient == ChocolateAdditionsType.hair
+
+    assert %Allergies{}.may_contain == [ChocolateAdditionsType.almonds]
+    assert %OddSnackIngredients{}.other_things == MapSet.new([ChocolateAdditionsType.nougat])
+    assert %ChocoMappings{}.common_name == %{ChocolateAdditionsType.hair => "love"}
+    assert %AdditionalMappings{}.mapping == %{ChocolateAdditionsType.almonds => "almonds",
+                                              ChocolateAdditionsType.nougat => "nougat"}
+
+    assert %AlreadyNamespaced{}.namespaced == ChocolateAdditionsType.almonds
+
+    actual = choco
+    |> Chocolate.serialize
+    |> IO.iodata_to_binary
+
+    expected = <<14, 0, 1, 8, 0, 0, 0, 2, 0, 0, 0, 1, 0, 0, 0, 2, 8, 0, 2, 0, 0, 0, 3, 0>>
+    assert actual == expected
+  end
 end

test/thrift/parser/annotation_test.exs

@@ -1,6 +1,7 @@
 defmodule Thrift.Parser.AnnotationTest do
   use ExUnit.Case, async: true
   import Thrift.Parser, only: [parse: 1]
+  alias Thrift.Parser.Models.Field
 
   setup_all do
     {:ok, schema} =
@@ -27,10 +28,10 @@ defmodule Thrift.Parser.AnnotationTest do
       :"java.final" => "",
       :"annotation.without.value" => "1"}
 
-    assert bar = find_field(struct.fields, :bar)
-    assert bar.annotations == %{:presence => "required"}
-    assert baz = find_field(struct.fields, :baz)
-    assert baz.annotations == %{:presence => "manual", :"cpp.use_pointer" => ""}
+    assert %Field{name: :bar, annotations: annotations} = find_field(struct.fields, :bar)
+    assert %{:presence => "required"} = annotations
+    assert %Field{name: :baz, annotations: baz_annotations} = find_field(struct.fields, :baz)
+    assert %{:presence => "manual", :"cpp.use_pointer" => ""} = baz_annotations
   end
 
   test "service annotations", context do
@@ -47,7 +48,7 @@ defmodule Thrift.Parser.AnnotationTest do
   test "exception annotations", context do
     assert %{exceptions: %{foo_error: exception}} = context[:schema]
     assert exception.annotations == %{:foo => "bar"}
-    assert field = find_field(exception.fields, :error_code)
-    assert field.annotations == %{:foo => "bar"}
+    assert %Field{name: :error_code, annotations: annotations} = find_field(exception.fields, :error_code)
+    assert %{:foo => "bar"} = annotations
   end
 end