Constant modules now reuse suitable existing names (#257)

We were previously writing constants to modules named after their
originating files. This approach was unfortunately too naive because the
resulting module names could conflict with other generated modules that
are spelled the same but different only in letter case.

For example, for a file named `myservice.thrift`:

    const float PI = 3.14
    service MyService {}

We would previously write the constants to `myservice.Myservice`
(`myservice.ex`) and the service definition to `myservice.MyService`
(`my_service.ex`). We now write both to `myservice.MyService` (thanks to
the generator's constants-merging logic) and no longer run into module
name conflicts.
This commit is contained in:
Jon Parise 2017-03-28 13:03:14 -07:00 committed by GitHub
parent 70d6e66db7
commit 4954dee60c
2 changed files with 39 additions and 6 deletions

View File

@ -175,12 +175,27 @@ defmodule Thrift.Parser.FileGroup do
end
def dest_module(file_group, Constant) do
# for constants the name of the module is
# <Namespace>.<filename>
initial_file = file_group.initial_file
base = Path.basename(initial_file, Path.extname(initial_file))
name = String.to_atom(base <> "." <> Macro.camelize(base))
dest_module(file_group, name)
# Default to naming the constants module after the namespaced, camelized
# basename of its file. For foo.thrift, this would be `foo.Foo`.
base = Path.basename(file_group.initial_file, ".thrift")
default = base <> "." <> Macro.camelize(base)
# However, if we're already going to generate an equivalent module name
# (ignoring case), use that instead to avoid generating two modules with
# the same spellings but different cases.
schema = file_group.schemas[base]
symbols = List.flatten([
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)
]) |> Enum.map(&Atom.to_string/1)
target = String.downcase(default)
name = Enum.find(symbols, default, fn s -> String.downcase(s) == target end)
dest_module(file_group, String.to_atom(name))
end
def dest_module(file_group, name) do

View File

@ -0,0 +1,18 @@
defmodule Thrift.Parser.FileGroupTest do
use ExUnit.Case
use ThriftTestHelpers
alias Thrift.Parser.FileGroup
alias Thrift.Parser.Models.Constant
test "constant module uses suitable existing name" do
with_thrift_files([
"myservice.thrift": """
const float PI = 3.14
service MyService {}
""", as: :file_group, parse: "myservice.thrift"]) do
assert :"Elixir.MyService" == FileGroup.dest_module(file_group, Constant)
end
end
end