ruby / rbs

Type Signature for Ruby
Other
1.94k stars 212 forks source link

_EnumEach interface #424

Closed HoneyryderChuck closed 2 months ago

HoneyryderChuck commented 3 years ago

https://github.com/ruby/rbs/blob/c64d1f5ba5a4fbc95ffb1d72d783ee39b17d60fe/stdlib/builtin/builtin.rbs already contains an _Each interface, but one where an enumerable can also be returned is missing. These are fairly common btw. Here's a proposal:



interface _Each[out A, out B]
  def each: { (A) -> void } -> B
                | () -> Enumerable[A, void]
end
soutaro commented 3 years ago

Could you show me an example that requires no-block-given version?

HoneyryderChuck commented 3 years ago

Enumerable#each is the most obvious example, I think. For example:

:0> [].each
=> #<Enumerator: []:each>

Currently, the way it's defined, the rbs definition doens't match this case.

marcandre commented 3 years ago

Enumerable#each is the most obvious example, I think. For example:

:0> [].each
=> #<Enumerator: []:each>

Currently, the way it's defined, the rbs definition doens't match this case.

This is the definition you are looking for.

It's basically correct, except the second should return ::Enumerator[Elem, self], so I opened a PR to fix that.

HoneyryderChuck commented 3 years ago

I was actually arguing for this definition to be part of a common interface, instead of being rewritten in every project implementing each with no block, which I think it's common enough, wouldn't you agree?

marcandre commented 3 years ago

It could definitely be useful as a shorthand, but maybe should be named something different like _Iterable?

I think _MethodName should be restricted to interfaces that implement that method_name minimally. So @soutaro's point was that if you want to accept that an object that implements each (which is what the name _Each suggests), then you probably don't want to impose that each without a block work, only that each with a block does. If you want to specify that your class implements each "correctly", then yes, you should have these two interfaces and a shorthand might be useful. I'm not sure how often this is, and how often that method is necessarily named each...

soutaro commented 3 years ago

I think I got the point.

class Foo
  def each
    yield 1
    yield 2
  end

  include Enumerable
end

Foo.new.each         # => error!!

Okay, we need to revise the definition of the interface!

HoneyryderChuck commented 3 years ago

@soutaro just for clarity, I'm not arguing for a revision of the existing one, but to add a new one. The reasoning is, the current _Each already describes the requirement for implementing a class decorated with Enumerable. But an _EnumEach is common enough (IMO) to warrant defining it as a common interface (as most core and stdlib enumerables already do it).