Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make TypeNameResolver more compatible with Ruby #1373

Merged
merged 1 commit into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions lib/rbs/resolver/type_name_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,50 @@ def resolve(type_name, context:)
end

try_cache([type_name, context]) do
resolve_in(type_name, context)
head, tail = partition(type_name)

head = resolve_in(head, context)

if head
if tail
has_name?(tail.with_prefix(head.to_namespace))
else
head
end
end
end
end

def partition(type_name)
if type_name.namespace.empty?
head = type_name.name
tail = nil
else
head, *tail = type_name.namespace.path

head or raise

tail = TypeName.new(
name: type_name.name,
namespace: Namespace.new(absolute: false, path: tail)
)
end

[head, tail]
end

def resolve_in(type_name, context)
def resolve_in(head, context)
if context
parent, child = context
case child
when false
resolve_in(type_name, parent)
resolve_in(head, parent)
when TypeName
name = type_name.with_prefix(child.to_namespace)
has_name?(name) || resolve_in(type_name, parent)
name = TypeName.new(name: head, namespace: child.to_namespace)
has_name?(name) || resolve_in(head, parent)
end
else
has_name?(type_name.absolute!)
has_name?(TypeName.new(name: head, namespace: Namespace.root))
end
end

Expand Down
4 changes: 3 additions & 1 deletion sig/resolver/type_name_resolver.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ module RBS

def try_cache: (query) { () -> TypeName? } -> TypeName?

def resolve_in: (TypeName, context) -> TypeName?
def resolve_in: (Symbol, context) -> TypeName?

def partition: (TypeName) -> [Symbol, TypeName?]
end
end
end
20 changes: 20 additions & 0 deletions test/rbs/resolver/type_name_resolver_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,26 @@ class Bar
end
end

def test_resolve_failure
SignatureManager.new do |manager|
manager.files[Pathname("foo.rbs")] = <<EOF
class Foo
class Foo
end

class Bar
end
end
EOF
manager.build do |env|
resolver = Resolver::TypeNameResolver.new(env)

assert_nil resolver.resolve(type_name("Foo::Bar"), context: [[nil, TypeName("::Foo")], TypeName("::Foo::Foo")])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It tried ::Foo::Foo::Foo::Bar, ::Foo::Foo::Bar, and ::Foo::Bar and resolved to ::Foo::Bar, but it was not how Ruby resolves constants.

With this PR, it tries only with the first component of given type name Foo as ::Foo::Foo::Foo and ::Foo::Foo, and resolves to ::Foo::Foo. Then it adds the tail part Bar as ::Foo::Foo::Bar, but it doesn't exist. So, it returns nil.

end
end
end


def test_duplicated_resolve
SignatureManager.new do |manager|
manager.files[Pathname("foo.rbs")] = <<EOF
Expand Down