Skip to content

Commit

Permalink
Better detection of enumerable type from an instance (#3358)
Browse files Browse the repository at this point in the history
* Add failing tests

* Change fallback type detection of Enumerable instance

When detecting the type of an enumerable from an instance, `TypedEnumerable`
tries to match it to a couple of well-known enumerable classes
(like `Array`, `Hash`, etc). The fallback case, however, tries to match it
to the class of the type that the instance is being validated against,
which does not make a lot sense.

That behaviour means, if the instance is being compared against
`T::Array[Foo]`, we would try to infer the type of the enumerable instance
as a kind of `T::Array`. But, if it was being compared against a `T::Set[Foo]`,
then the type of the instance would be inferred to be a `T::Set`. This leads
to the problem outlined in #2808.

In reality, Sorbet should not try to infer the type based on what
type the instance is being compared against, but should return the type of
the instance as is.
  • Loading branch information
paracycle committed Aug 12, 2020
1 parent 09ad24e commit 73847a1
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 1 deletion.
3 changes: 2 additions & 1 deletion gems/sorbet-runtime/lib/types/types/typed_enumerable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ def describe_obj(obj)
# enumerating the object is a destructive operation and might hang.
obj.class
else
self.class.new(type_from_instances(obj))
# This is a specialized enumerable type, just return the class.
Object.instance_method(:class).bind(obj).call
end
end

Expand Down
22 changes: 22 additions & 0 deletions gems/sorbet-runtime/test/types/method_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,28 @@ def @mod.foo
assert_empty(lines[3..-1])
end

class TestEnumerable
include Enumerable

def each
yield "something"
end
end

it "raises a sensible error for custom enumerable validation errors" do
@mod.sig { returns(T::Array[String]) }
def @mod.foo
TestEnumerable.new
end

err = assert_raises(TypeError) do
@mod.foo
end
assert_match(
"Return value: Expected type T::Array[String], got Opus::Types::Test::MethodValidationTest::TestEnumerable",
err.message.lines[0])
end

describe 'ranges' do
describe 'return type is non-nilable integer' do
it 'permits a range that has integers on start and end' do
Expand Down
14 changes: 14 additions & 0 deletions gems/sorbet-runtime/test/types/types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,12 @@ def subtype_of?(other)
end

describe "TypedArray" do
class TestEnumerable
include Enumerable

def each; yield "something"; end
end

it 'fails if value is not an array' do
type = T::Array[Integer]
value = 3
Expand Down Expand Up @@ -442,6 +448,14 @@ def subtype_of?(other)
allocs_when_invalid = counting_allocations {type.valid?(invalid)}
assert_equal(0, allocs_when_invalid)
end

it 'gives the right error when passed an unrelated enumerable' do
type = T::Array[String]
msg = check_error_message_for_obj(type, TestEnumerable.new)
assert_equal(
"Expected type T::Array[String], got Opus::Types::Test::TypesTest::TestEnumerable",
msg)
end
end

describe "TypedHash" do
Expand Down

0 comments on commit 73847a1

Please sign in to comment.