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

_EnumEach interface #424

Closed
HoneyryderChuck opened this issue Oct 16, 2020 · 7 comments · Fixed by #1915
Closed

_EnumEach interface #424

HoneyryderChuck opened this issue Oct 16, 2020 · 7 comments · Fixed by #1915

Comments

@HoneyryderChuck
Copy link
Contributor

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
Copy link
Member

soutaro commented Oct 17, 2020

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

@HoneyryderChuck
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

soutaro commented Oct 19, 2020

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
Copy link
Contributor Author

@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).

ParadoxV5 added a commit to ParadoxV5/rbs that referenced this issue Jun 29, 2024
ParadoxV5 added a commit to ParadoxV5/rbs that referenced this issue Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants