Prefer numbered block params over proc conversion

[dduugg]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

The occasional style of converting a method to a proc for use as a block argument isn't one I've seen elsewhere. I propose replacing it with something more idiomatic (assuming the conversions below are correct, it's honestly a bit confusing to me). This also has the advantage of providing type checking.

(I realize numbered block params have a mixed reception, though I think they're strictly clearer in the cases in this PR. I won't be upset if folks feel otherwise though).

[apainintheneck]

I agree that this looks cleaner and clearer while also being less verbose.

I wondered if this syntax always captured all of the parameters but it looks like it is equivalent after I checked it out in the console. If _1 is used by itself, it seems to capture everything.

irb(main):001:0> [(1..10).to_a].map { _1 }
=> [[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]]
irb(main):002:0> [(1..10).to_a].map { _2 }
=> [2]
irb(main):003:0> [(1..10).to_a].map { _9 }
=> [9]
irb(main):004:0> [(1..10).to_a].map { [_1, _2] }
=> [[1, 2]]

[MikeMcQuaid]

Sorry, I much prefer the prior version. I think it's much more obvious what's going on to non-Rubyists.

Would welcome a RuboCop being created or enabled that enforces the prior style if we're using it inconsistently.

[dduugg]

Sorry, I much prefer the prior version. I think it's much more obvious what's going on to non-Rubyists.

Would welcome a RuboCop being created or enabled that enforces the prior style if we're using it inconsistently.

There is a Style/SymbolProc cop for the normal case, where the block param is the receiver (e.g. arr.map(&:to_s)). (To be clear, I don't suggest replacing this style – it's concise, idiomatic, type-checked, and respects visibility modifiers.)

It may be possible to extend that to cover cases where the block param is used as an argument within the block, using the method conversion we see in the cases in this PR (e.g. arr.map(&String.method(:new))). (This is the case that i would describe as being verbose and niche. It also skirts type-checking and visibility modifiers, though the latter can be addressed by using public_method.)

Closing out – sorry for the noise!

[MikeMcQuaid]

It also skirts type-checking

@dduugg This is the bit that I'm curious about (and may encourage me to say you can reopen this): can you elaborate a bit?

If this approach is better for type-checking/Sorbet: I'm definitely much more into it than before!

[dduugg]

@dduugg This is the bit that I'm curious about (and may encourage me to say you can reopen this): can you elaborate a bit?

It's a bit contrived (since it's using instance methods), but here you go

We can chew on this one a bit before re-opening though. I have some other, more substantive tickets to open & draft code for…

[MikeMcQuaid]

You changed my mind, let's do it!

If there's a rubocop to be able to enforce this way of doing things: that'd be great. If not: writing one might be nice!

[Bo98]

I do think that &Class.(public_)method(:name) is pretty horrible - I've never really liked that syntax so I do see this PR as an improvement for that reason.

Whether it's _1 or |name| ... name however I don't really mind.

[apainintheneck]

If there's a rubocop to be able to enforce this way of doing things: that'd be great. If not: writing one might be nice!

I'm surprised nothing like that exists in RuboCop yet. Might be nice to upstream it if they're interested.