Deterministically merge FileGroup resolutions (#250)

This is an incomplete solution to the fact that our existing resolution
merge was non-deterministic with regard to non-qualified names. We now
always take the newer value for a conflicting key, which gets us closer
to the expected behavior.

We unfortunately still aren't implementing the correct scoping rules.
For example, this still allows a non-qualified name to "leak" from one
included file into another. This should be revisited and addressed with
a more complete solution.
This commit is contained in:
Jon Parise 2017-03-07 09:01:35 -08:00 committed by GitHub
parent d57f1bf14a
commit 11810e5999
2 changed files with 35 additions and 1 deletions

View File

@ -102,7 +102,12 @@ defmodule Thrift.Parser.FileGroup do
end)
|> Map.new
resolutions = Map.merge(file_group.resolutions, to_update)
# Merge the non-qualified names into the resolution map. We always replace
# existing entries with the same key. This produces a predictable result
# but is wrong in the sense that it doesn't implement the expected scoping
# rules. For example, this still allows a non-qualified name to "leak"
# from one included file into another. TODO: revisit resolution scoping
resolutions = Map.merge(file_group.resolutions, to_update, fn _k, _v1, v2 -> v2 end)
default_namespace = if file_group.opts[:namespace] do
%Namespace{:name => :elixir, :path => file_group.opts[:namespace]}

View File

@ -192,4 +192,33 @@ defmodule ResolverTest do
assert %TEnum{} = FileGroup.resolve(file_group, :"enums.JobStatus")
end
end
test "it should be able resolve qualified and non-qualified names" do
with_thrift_files([
"included.thrift": """
enum SortType {
DESC = 100,
ASC = 110
}
""",
"local.thrift": """
include "included.thrift"
enum SortType {
DESC,
ASC
}
const SortType SORT1 = SortType.ASC;
const included.SortType SORT2 = included.SortType.ASC;
""", as: :file_group, parse: "local.thrift"]) do
sort1 = FileGroup.resolve(file_group, :"local.SORT1")
assert %TEnum{name: :"local.SortType"} = FileGroup.resolve(file_group, sort1.type)
assert 2 == FileGroup.resolve(file_group, sort1.value)
sort2 = FileGroup.resolve(file_group, :"local.SORT2")
assert %TEnum{name: :"included.SortType"} = FileGroup.resolve(file_group, sort2.type)
assert 110 == FileGroup.resolve(file_group, sort2.value)
end
end
end