From efa6099e9ce14829cd478dc1b54054c9abbf8037 Mon Sep 17 00:00:00 2001 From: Aaron Christiansen Date: Tue, 6 Jul 2021 21:35:51 +0100 Subject: [PATCH 1/3] Add #untyped? to Method for RBI and RBS --- lib/parlour/rbi_generator/method.rb | 10 ++++++++++ lib/parlour/rbs_generator/method.rb | 12 ++++++++++++ rbi/parlour.rbi | 6 ++++++ spec/rbi_generator_spec.rb | 16 ++++++++++++++++ spec/rbs_generator_spec.rb | 16 ++++++++++++++++ 5 files changed, 60 insertions(+) diff --git a/lib/parlour/rbi_generator/method.rb b/lib/parlour/rbi_generator/method.rb index d41ee2a1f..e9f752efa 100644 --- a/lib/parlour/rbi_generator/method.rb +++ b/lib/parlour/rbi_generator/method.rb @@ -223,6 +223,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 diff --git a/lib/parlour/rbs_generator/method.rb b/lib/parlour/rbs_generator/method.rb index 05cc1e508..b0208ae5a 100644 --- a/lib/parlour/rbs_generator/method.rb +++ b/lib/parlour/rbs_generator/method.rb @@ -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 diff --git a/rbi/parlour.rbi b/rbi/parlour.rbi index 360bed5d8..fd9ee20bc 100644 --- a/rbi/parlour.rbi +++ b/rbi/parlour.rbi @@ -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 @@ -1638,6 +1641,9 @@ module Parlour sig { override.returns(String) } def describe; end + + sig { returns(T::Boolean) } + def untyped?; end end class MethodSignature diff --git a/spec/rbi_generator_spec.rb b/spec/rbi_generator_spec.rb index e44f3f78c..c15aae16e 100644 --- a/spec/rbi_generator_spec.rb +++ b/spec/rbi_generator_spec.rb @@ -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 diff --git a/spec/rbs_generator_spec.rb b/spec/rbs_generator_spec.rb index 684958105..76712606a 100644 --- a/spec/rbs_generator_spec.rb +++ b/spec/rbs_generator_spec.rb @@ -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 From 2915cfb82dfa6be5f0b1d36fe656615370898b07 Mon Sep 17 00:00:00 2001 From: Aaron Christiansen Date: Tue, 6 Jul 2021 23:03:21 +0100 Subject: [PATCH 2/3] Add special case to conflict resolver for untyped method merging --- lib/parlour/conflict_resolver.rb | 47 ++++++++++++++++++++ lib/parlour/rbi_generator/method.rb | 3 ++ spec/conflict_resolver_spec.rb | 68 ++++++++++++++++++++++++++++- 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/lib/parlour/conflict_resolver.rb b/lib/parlour/conflict_resolver.rb index 48add0e12..44565b84d 100644 --- a/lib/parlour/conflict_resolver.rb +++ b/lib/parlour/conflict_resolver.rb @@ -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")) diff --git a/lib/parlour/rbi_generator/method.rb b/lib/parlour/rbi_generator/method.rb index e9f752efa..2641b2b0e 100644 --- a/lib/parlour/rbi_generator/method.rb +++ b/lib/parlour/rbi_generator/method.rb @@ -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) } diff --git a/spec/conflict_resolver_spec.rb b/spec/conflict_resolver_spec.rb index ed234279e..627850f2f 100644 --- a/spec/conflict_resolver_spec.rb +++ b/spec/conflict_resolver_spec.rb @@ -706,4 +706,70 @@ def foo; end expect(x.children.length).to be 2 end -end \ No newline at end of file + + it 'resolves conflicts where all but one copy of the method is untyped' 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: 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 From cab89ad6ee0b549026b437a5875c37fb8aa9f8df Mon Sep 17 00:00:00 2001 From: Aaron Christiansen Date: Tue, 6 Jul 2021 23:19:41 +0100 Subject: [PATCH 3/3] Tweak test to omit signature on one definition --- spec/conflict_resolver_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/conflict_resolver_spec.rb b/spec/conflict_resolver_spec.rb index 627850f2f..38b274eb2 100644 --- a/spec/conflict_resolver_spec.rb +++ b/spec/conflict_resolver_spec.rb @@ -710,7 +710,6 @@ def foo; 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 - sig { params(i: T.untyped).returns(T.untyped) } def int_to_str(i); end sig { params(i: Integer).returns(String) }