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

Cop idea: Not enough arguments for enum_for/to_enum #7753

Closed
pocke opened this issue Feb 25, 2020 · 2 comments · Fixed by #8941
Closed

Cop idea: Not enough arguments for enum_for/to_enum #7753

pocke opened this issue Feb 25, 2020 · 2 comments · Fixed by #8941
Labels
feature request good first issue Easy task, suitable for newcomers to the project help wanted

Comments

@pocke
Copy link
Collaborator

pocke commented Feb 25, 2020

Is your feature request related to a problem? Please describe.

We have an idiom of enum_for/to_enum method for defining an iterator.

class C
  def each_item(&block)
    return enum_for(__method__) unless block_given?

    iterate_someting_with_block block
  end
end

c = C.new

# We can call C#each with and without a block.
c.each_item do |item|
end
c.each_item.map do |item|
end

It is a useful idiom! But we sometimes make a bug if the iterator receives arguments. For example:

class C
  def each_item(type, &block)
    return enum_for(__method__) unless block_given?

    iterate_something_with_block type, block
  end
end
c = C.new

# It works well.
c.each_item(:foo) do |item|
end

# But it does not work well because enum_for should receive `type` but actually doesn't.
c.each_item(:foo).map do |item|
end

It should be return enum_for(__method__, type) unless block_given?.

In this case, it is not a serious problem because c.each_item(:foo).map{} raises an ArgumentError.
But it will be serious when the argument is optional, like def each_item(type = :default, &block).
It ignores type argument silently.

Describe the solution you'd like

Add a new cop to detect this problem.

Describe alternatives you've considered

Nothing

Additional context

  • to_enum is an alias of enum_for
  • Probably we should focus the idiom, it means the cop should accept any enum_for without return enum_for(__method__) unless block_given? form.
@stale
Copy link

stale bot commented Aug 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label Aug 23, 2020
@marcandre marcandre added good first issue Easy task, suitable for newcomers to the project help wanted and removed stale Issues that haven't been active in a while labels Aug 24, 2020
@marcandre
Copy link
Contributor

I've made that mistake myself a few times.

Cop should only apply when: the receiver is nil or self.
Maybe it should check that either the method name (in your example :each_item) or __method__ is used as the first argument too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request good first issue Easy task, suitable for newcomers to the project help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants