Merge pull request #230 from dantswain/incursion

Handle constants clobbering other modules or double-defining

lib/thrift/generator.ex

@@ -156,10 +156,22 @@ defmodule Thrift.Generator do
   defp generate_const_modules(schema) do
     # schema.constants is a map %{name: constant} but constant includes the
     # name and all we really need is the values
-    constants = Map.values(schema.constants)
-    # name of the generated module
-    full_name = FileGroup.dest_module(schema.file_group, Constant)
-    [{full_name, ConstantGenerator.generate(full_name, constants, schema)}]
+    #
+    # we also only want constants that are defined in the main file from this
+    # file group
+    constants = schema.constants
+    |> Map.values
+    |> Enum.filter(&FileGroup.own_constant?(schema.file_group, &1))
+
+    if Enum.empty?(constants) do
+      # if we filtered out all of the constants, we don't need to write
+      # anything
+      []
+    else
+      # name of the generated module
+      full_name = FileGroup.dest_module(schema.file_group, Constant)
+      [{full_name, ConstantGenerator.generate(full_name, constants, schema)}]
+    end
   end
 
   defp generate_struct_modules(schema) do

lib/thrift/parser/file_group.ex

@@ -194,6 +194,16 @@ defmodule Thrift.Parser.FileGroup do
     end
   end
 
+  # check if the given model is defined in the root file of the file group
+  #   this should eventually be replaced if we find a way to only parse files
+  #   once
+  @spec own_constant?(t, Constant.t) :: boolean
+  def own_constant?(file_group, %Constant{} = constant) do
+    basename = Path.basename(file_group.initial_file, ".thrift")
+    initial_file = file_group.parsed_files[basename]
+    Enum.member?(Map.keys(initial_file.schema.constants), constant.name)
+  end
+
   defp build_ns_mappings(schemas, default_namespace) do
     schemas
     |> Enum.map(fn {module_name, %Schema{namespaces: ns}} ->

test/thrift/generator/binary_protocol_test.exs

@@ -575,6 +575,37 @@ defmodule Thrift.Generator.BinaryProtocolTest do
     } == XMan.phoenix
   end
 
+  @thrift_file name: "included_constants.thrift", contents: """
+  const i8 Z = 26
+  """
+
+  @thrift_file name: "includes_constants.thrift", contents: """
+  include "included_constants.thrift"
+
+  struct IncludesConstants {
+    1: string name
+    2: i8 z = included_constants.Z
+  }
+  """
+
+  @thrift_file name: "also_includes.thrift", contents: """
+  include "included_constants.thrift"
+
+  struct SomeOtherName {
+    1: i32 id
+  }
+  """
+
+  thrift_test "including a file with constants" do
+    assert 26 == IncludedConstants.z
+    assert %IncludesConstants{z: 26} == %IncludesConstants{}
+    assert_raise UndefinedFunctionError, fn -> 26 == IncludesConstants.z end
+    assert %SomeOtherName{} == SomeOtherName.new()
+    assert Code.ensure_loaded?(IncludedConstants)
+    # AlsoIncludes should not define anything
+    refute Code.ensure_loaded?(AlsoIncludes)
+  end
+
   thrift_test "lists serialize into maps" do
     binary = <<13, 0, 2, 3, 3, 0, 0, 0, 1, 91, 92, 0>>
     assert binary == %Byte{val_map: %{91 => 92}} |> Byte.serialize() |> IO.iodata_to_binary