Rubocop: Block variables in parameters + yield

[Nondv]

Hey!
Sorry if someone opened something similar, couldn't google it.

I have some method that requires a block and I want it to be displayed in method parameters with some nice name. For example:

def some_process(&log)
  yield 'doing something'
  do_something
  yield 'doing something else'
  do_something_else
end

(sorry I can't come up with a better example)

Rubocop warns me that log is not used and suggests to rename it to _log.
If I replace yield with log.call it will ask me to use yield (performance).

So what should I do?

I think that block variable in parameters are very useful even if used only via yield.

$ rubocop --version
0.52.1

[mikegee]

So what should I do?

Disable one of those cops, or maybe give up on the idea that the block needs a name?

[Nondv]

Hi @mikegee !

Why would I give up on the idea if it's good (or isn't it?) ? When someone takes a look at method parameters he instantly knows that he should provide block and what this block should do. It's very convenient. Sorry for my english.

Disabling is not a good option too because both cops are nice. They just don't apply to this specific case.

[mikegee]

The named block argument is similar to a code comment. All Ruby methods optionally take a block, so it isn't required to make the method work and it would become misleading if the yield is removed.

I attempt to make the code more "self-documenting". In this case, keeping the method very small will let the next developer see the yield usage.

---

Disabling is not a good option too because both cops are nice. They just don't apply to this specific case.

You don't need to disable them for the whole project. Use an inline disable comment like this:

def some_process(&log) # rubocop:disable Lint/UnusedMethodArgument
  yield 'doing something'
  do_something
  yield 'doing something else'
  do_something_else
end

[Nondv]

You have a point.

But for me parameter with a good name IS "self-documenting". It is better than comment even though it is pretty similar in this case. Parameter can be displayed in some popup helpers via IDE (personally I don't use IDE but I know people who do) when comments not.

As for inline disabling, I would say it's pretty ugly :( I'm trying to avoid them.

[Nondv]

Still, I would like to see opinions from other people

[ledsun]

I propose that a new configurable attribute to ignore arguments that starts with & is added to the Lint/UnusedMethodArgument.

[md-work]

I propose that a new configurable attribute to ignore arguments that starts with & is added to the Lint/UnusedMethodArgument.

I'd go even further and would suggest a cop to require a &... parameter if yield is used.
E.g.

# bad
def some_process
  yield 'doing something'
  do_something
end

# good
def some_process(&log)
  yield 'doing something'
  do_something
end

Because &log is mandatory for this method and calls without it will always fail.

def some_process
  yield 'doing something'
  do_something
end

some_process
# -> in `some_process': no block given (yield) (LocalJumpError)

[bbatsov]

There hasn't been much activity on this ticket and our Core Team is spread too thin on so many tasks, so I'll just close it for the sake of having a cleaner lists of tasks to focus on. We'd gladly review a PR, but it's unlikely that'd we're going to tackle this ourselves in the foreseeable future.

[radiospiel]

@bbatsov I am late to this party :) but would suggest to split the Lint/UnusedMethodArgument into two parts: one that is looking at variadic, named, and keyword arguments only, and a new one Lint/UnusedBlockArgument which would raise an alarm if a block argument was defined in the method signature, but was not used directly (by its name) nor via yield.