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

Allow untyped method signatures to be merged with typed ones #119

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
47 changes: 47 additions & 0 deletions lib/parlour/conflict_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,53 @@ def resolve_conflicts(namespace, &resolver)
next
end

# Special case:
# - The candidates are methods
# - There is at least one candidate which is typed
# - All of the typed methods are mergeable with each other
# - All of the untyped methods are mergeable with each other
# - The typed methods and untyped methods have the same parameter
# set except the types (i.e. same name and kind)
# This covers the case of merging a definition in Ruby source with its
# definition in an RBI.
# Discard the untyped methods and merge the typed methods.
if children.all? { |c| c.is_a?(RbiGenerator::Method) }
children = T.cast(children, T::Array[Parlour::RbiGenerator::Method])

# Check the parameter names and kinds are the same
names_and_kinds = children.map do |meth|
meth.parameters.map { |param| [param.name, param.kind] }
end

if names_and_kinds.uniq.length == 1
# Separately check that all the untyped candidates and typed
# candidates are the same. (We checked the parameters, but untyped
# candidates could still differ, e.g. `abstract` or other
# modifiers)
untyped_candidates, typed_candidates = children.partition(&:untyped?)

if typed_candidates.any? &&
all_eql?(untyped_candidates) &&
all_eql?(typed_candidates)

# All criteria met!
# Discard the untyped candidates, add a typed one
children.each do |c|
namespace.children.delete(c)
end

# Re-add one typed, merged child
first, *rest = typed_candidates
first = T.must(first)
rest = T.must(rest)
first.merge_into_self(rest)
namespace.children << first

next
end
end
end

# Optimization for Special case: are they all clearly equal? If so, remove all but one
if all_eql?(children)
Debugging.debug_puts(self, Debugging::Tree.end("All children are identical"))
Expand Down
13 changes: 13 additions & 0 deletions lib/parlour/rbi_generator/method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ def mergeable?(others)
# @return [void]
def merge_into_self(others)
# We don't need to change anything! We only merge identical methods
# (That's not strictly true, we also sometimes merge typed methods and
# untyped methods. This is handled by a special case within the conflict
# resolver itself.)
end

sig { override.returns(String) }
Expand All @@ -223,6 +226,16 @@ def generalize_from_rbi!
parameters.each(&:generalize_from_rbi!)
end

sig { returns(T::Boolean) }
# Returns true if this method is completely untyped; that is, all
# parameters are untyped and the return type is untyped.
#
# @return [bool]
def untyped?
is_untyped = ->x{ x.is_a?(Types::Untyped) || x == 'T.untyped' }
parameters.map(&:type).all?(&is_untyped) && is_untyped.(return_type)
end

private

sig do
Expand Down
12 changes: 12 additions & 0 deletions lib/parlour/rbs_generator/method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,18 @@ def describe
# TODO: more info
"Method #{name} - #{signatures.length} signatures"
end

sig { returns(T::Boolean) }
# Returns true if this method is completely untyped; that is, all
# parameters are untyped and the return type is untyped.
#
# @return [bool]
def untyped?
is_untyped = ->x{ x.is_a?(Types::Untyped) || x == 'untyped' }
signatures.all? do |sig|
sig.parameters.map(&:type).all?(&is_untyped) && is_untyped.(sig.return_type)
end
end
end
end
end
6 changes: 6 additions & 0 deletions rbi/parlour.rbi
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,9 @@ module Parlour
sig { override.void }
def generalize_from_rbi!; end

sig { returns(T::Boolean) }
def untyped?; end

sig { overridable.params(indent_level: Integer, options: Options).returns(T::Array[String]) }
def generate_definition(indent_level, options); end

Expand Down Expand Up @@ -1638,6 +1641,9 @@ module Parlour

sig { override.returns(String) }
def describe; end

sig { returns(T::Boolean) }
def untyped?; end
end

class MethodSignature
Expand Down
67 changes: 66 additions & 1 deletion spec/conflict_resolver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -706,4 +706,69 @@ def foo; end

expect(x.children.length).to be 2
end
end

it 'resolves conflicts where all but one copy of the method is untyped' do
x = Parlour::TypeLoader.load_source(<<-RUBY).children.first
class A
def int_to_str(i); end

sig { params(i: Integer).returns(String) }
def int_to_str(i); end

sig { params(i: Integer).returns(String) }
def int_to_str(i); end

sig { params(i: T.untyped).returns(T.untyped) }
def int_to_str(i); end
end
RUBY

expect(x.children.length).to be 4

subject.resolve_conflicts(x) { |*| raise 'unable to resolve automatically' }

expect(x.children.length).to be 1
expect(x.children.first.return_type).to eq "String"
end

it 'keeps an untyped copy of a method if there is no typed copy' do
x = Parlour::TypeLoader.load_source(<<-RUBY).children.first
class A
sig { params(i: T.untyped).returns(T.untyped) }
def int_to_str(i); end

sig { params(i: T.untyped).returns(T.untyped) }
def int_to_str(i); end
end
RUBY

expect(x.children.length).to be 2

subject.resolve_conflicts(x) { |*| raise 'unable to resolve automatically' }

expect(x.children.length).to be 1
end

it 'does not resolve untyped copies with different parameters' do
x = Parlour::TypeLoader.load_source(<<-RUBY).children.first
class A
sig { params(i: Integer).returns(String) }
def int_to_str(i); end

sig { params(i: Integer).returns(String) }
def int_to_str(i); end

sig { params(x: T.untyped).returns(T.untyped) }
def int_to_str(x); end
end
RUBY

expect(x.children.length).to be 3

invocations = 0
subject.resolve_conflicts(x) { |*| invocations += 1; nil }

expect(invocations).to be 1
expect(x.children.length).to be 0
end
end
16 changes: 16 additions & 0 deletions spec/rbi_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,22 @@ def foo(a = 4); end
def box(a); end
RUBY
end

it 'return the correct value for #untyped?' do
meth_a = subject.root.create_method('a', parameters: [
pa('a', type: 'Integer')
], return_type: 'T.untyped')
meth_b = subject.root.create_method('b', parameters: [
pa('a', type: 'T.untyped')
], return_type: 'Integer')
meth_c = subject.root.create_method('c', parameters: [
pa('a', type: 'T.untyped')
], return_type: Parlour::Types::Untyped.new)

expect(meth_a.untyped?).to eq false
expect(meth_b.untyped?).to eq false
expect(meth_c.untyped?).to eq true
end
end

context 'attributes' do
Expand Down
16 changes: 16 additions & 0 deletions spec/rbs_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,22 @@ def foo: (String a) -> Integer
| () -> void
RUBY
end

it 'return the correct value for #untyped?' do
meth_a = subject.root.create_method('a', [Parlour::RbsGenerator::MethodSignature.new([
pa('a', type: 'Integer')
], 'untyped')])
meth_b = subject.root.create_method('b', [Parlour::RbsGenerator::MethodSignature.new([
pa('a', type: 'untyped')
], 'Integer')])
meth_c = subject.root.create_method('c', [Parlour::RbsGenerator::MethodSignature.new([
pa('a', type: 'untyped')
], Parlour::Types::Untyped.new)])

expect(meth_a.untyped?).to eq false
expect(meth_b.untyped?).to eq false
expect(meth_c.untyped?).to eq true
end
end

context 'attributes' do
Expand Down