Skip to content

Commit

Permalink
Store FileGroup namespace values as strings (#436)
Browse files Browse the repository at this point in the history
We previously stored a mapping of module_name -> %Namespace{}, but we
only need the namespace's value to resolve the destination module.

Just storing the namespace values directly (in their native string form)
simplifies this code, including the "default namespace" logic (because
it no longer needs to be represented as a full %Namespace{}).
  • Loading branch information
jparise committed Oct 17, 2018
1 parent 99b77cc commit 85a7caa
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 24 deletions.
33 changes: 14 additions & 19 deletions lib/thrift/parser/file_group.ex
Expand Up @@ -14,7 +14,6 @@ defmodule Thrift.Parser.FileGroup do
Constant,
Exception,
Field,
Namespace,
Schema,
Service,
Struct,
Expand All @@ -28,7 +27,7 @@ defmodule Thrift.Parser.FileGroup do
initial_file: Path.t(),
parsed_files: %{FileRef.thrift_include() => %ParsedFile{}},
schemas: %{FileRef.thrift_include() => %Schema{}},
ns_mappings: %{atom => %Namespace{}},
namespaces: %{atom => String.t() | nil},
opts: Parser.opts()
}

Expand All @@ -38,7 +37,7 @@ defmodule Thrift.Parser.FileGroup do
schemas: %{},
resolutions: %{},
immutable_resolutions: %{},
ns_mappings: %{},
namespaces: %{},
opts: Keyword.new()

@spec new(Path.t(), Parser.opts()) :: t
Expand Down Expand Up @@ -109,14 +108,9 @@ defmodule Thrift.Parser.FileGroup do
end)
|> Map.new()

default_namespace =
if file_group.opts[:namespace] do
%Namespace{:scope => :elixir, :value => file_group.opts[:namespace]}
end
namespaces = build_namespaces(file_group.schemas, file_group.opts[:namespace])

ns_mappings = build_ns_mappings(file_group.schemas, default_namespace)

%FileGroup{file_group | resolutions: resolutions, ns_mappings: ns_mappings}
%FileGroup{file_group | resolutions: resolutions, namespaces: namespaces}
end

@spec resolve(t, any) :: any
Expand Down Expand Up @@ -226,13 +220,13 @@ defmodule Thrift.Parser.FileGroup do
|> Enum.at(1)
|> initialcase()

case file_group.ns_mappings[module_name] do
case file_group.namespaces[module_name] do
nil ->
Module.concat([struct_name])

namespace = %Namespace{} ->
namespace ->
namespace_parts =
namespace.value
namespace
|> String.split(".")
|> Enum.map(&Macro.camelize/1)

Expand All @@ -258,12 +252,13 @@ defmodule Thrift.Parser.FileGroup do
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}} ->
namespace = Map.get(ns, :elixir, default_namespace)
{String.to_atom(module_name), namespace}
defp build_namespaces(schemas, default_namespace) do
Map.new(schemas, fn
{module_name, %Schema{namespaces: %{:elixir => namespace}}} ->
{String.to_atom(module_name), namespace.value}

{module_name, _} ->
{String.to_atom(module_name), default_namespace}
end)
|> Map.new()
end
end
7 changes: 2 additions & 5 deletions test/thrift/parser/parser_test.exs
Expand Up @@ -918,17 +918,14 @@ defmodule Thrift.Parser.ParserTest do

describe "namespace option" do
setup do
path = Path.join(@test_file_dir, "namespace.thrift")
path = Path.join(@test_file_dir, "module.thrift")
File.write!(path, "struct Struct { 1: i32 id }")
{:ok, [path: path]}
end

defp parse_namespace(context, namespace) do
result = parse_file(context[:path], namespace: namespace)

if is_map(result.ns_mappings.namespace) do
result.ns_mappings.namespace.value
end
result.namespaces[:module]
end

test "is empty", context do
Expand Down

0 comments on commit 85a7caa

Please sign in to comment.