Style/ArgumentsForwarding seems a bit heavy-handed
searls opened this issue · 5 comments
Just updated standard to 1.34.0 and saw this failure:
def set_time_zone(&block)
Time.use_zone(current_user.time_zone, &block)
end
Which auto-fixes to this:
def set_time_zone(&)
Time.use_zone(current_user.time_zone, &)
end
I was kinda annoyed by this, and maybe even more annoyed when I realized that only [blk, block, proc]
trip the rule.
Am I just being an old crank because I have 20 years of muscle memory writing &blk
?
It is worth mentioning that Sorbet currently doesn't allow arguments forwarding, including for block arguments. I'm glad to have found this workaround by using a different block argument name. IMO the rule seems to cause more trouble than it is worth.
Malformed `sig`. Type not specified for argument & [5003](https://srb.help/5003)
Yeah, I have come to personally dislike being forced into this one, and I noticed as well that when I went back to maintain mocktail I had to add a bunch of #ignore directives.
Personally, I think Standard should be compatible with Sorbet codebases and we should disable this one at least until it is.
@camilopayan what do you think?
Yeah, I have come to personally dislike being forced into this one, and I noticed as well that when I went back to maintain mocktail I had to add a bunch of #ignore directives.
Agreed
I just upgraded standard
and the version of Rubocop that came with it (which is not the latest FWIW) rewrote some code from this rule and broke some code:
This original code:
def method_missing(meth, *args, **opts, &blk)
if delegated_class_method?(meth)
opts[:account_id] = @account.id if @account
opts[:env] = @env.merge(opts[:env] || {})
opts[:session] = @session.merge(opts[:session] || {})
@auth_class.send(meth, *args, **opts, &blk)
elsif delegated_instance_method?(meth)
internal { send(meth, *args, **opts, &blk) }
else
super
end
end
Was re-written into:
def method_missing(meth, *, **opts, &)
if delegated_class_method?(meth)
opts[:account_id] = @account.id if @account
opts[:env] = @env.merge(opts[:env] || {})
opts[:session] = @session.merge(opts[:session] || {})
@auth_class.send(meth, *, **opts, &)
elsif delegated_instance_method?(meth)
internal { send(meth, *args, **opts, &blk) }
else
super
end
end
This breaks, because there is still a reference to *args
that gets unchanged.
I believe this is rubocop/rubocop#12875, which is fixed in newer versions. Leaving this here just as a breadcrumb to others in similar position.
I haven't been able to upgrade rubocop
to a fixed version because the latest standard
(understandably) has quite a tight version restriction. If I force a newer Rubocop, bundler actually downgrades standardrb
to 1.35.0:
For now, I'm going to avoid this standard
upgrade until this rule is loosened as I agree that it is faaar too heavy-handed and in many cases worsens readability (obv subjective, but that's my take!)
Ok if @bjeanes is on board then I think I'd support a merge if someone sends a PR to disable it.