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: prefer argument forwarding syntax #7549

Closed
pocke opened this issue Dec 4, 2019 · 2 comments · Fixed by #7646
Closed

Cop idea: prefer argument forwarding syntax #7549

pocke opened this issue Dec 4, 2019 · 2 comments · Fixed by #7646

Comments

@pocke
Copy link
Collaborator

pocke commented Dec 4, 2019

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

Since Ruby 2.7, we will be able to use ... to forward arguments to another method.

# 2.6
def foo(*args, &block)
  bar(*args, &block)
end

# 2.7
def foo(...)
  bar(...)
end

I think the ... syntax has two advantages.

First, we can simply avoid keyword parameter warnings with ... syntax.
For example

# with Ruby 2.7

def bar(*args, a: 1, &block)
  p [args, a]
end

# It displays the warning.
def foo(*args, &block)
  bar(*args, &block)
end
foo 1, 2, a: 1

# It displays no warning, but it is a bit of long.
undef foo
def foo(*args, **kwargs, &block)
  bar(*args, **kwargs, &block)
end
foo 1, 2, a: 1

# No warning, and short!
undef foo
def foo(...)
  bar(...)
end

foo 1, 2, a: 1

Second, we can avoid missing &block forwarding with ... syntax.
I guess many people forget passing &block to the forwarded method unexpectedly.
But we can avoid the problem with ... syntax.

Describe the solution you'd like

Add an offense for the following code.

# We can replace the parameter with `...`.
# It can avoid the warning.
# It does not change the behavior at least Ruby 2.7.
# The behavior will be changed in the future.
def foo(*args, &block)
  bar(*args, &block)
end

# We can replace the parameter with `...`.
# The behaviors are completely the same in all Rubies.
# So it is cosmetic change only.
def foo(*args, **kwargs, &block)
  bar(*args, **kwargs, &block)
end

# We can replace the parameter with `...`, but it will change the behavior.
# Because `...` forwards block also.
def foo(*args)
  bar(*args)
end

The last example changes the method behavior actually, so probably we should add Strict option to disable/enable replacing the last case.

Describe alternatives you've considered

nothing

Additional context

nothing

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 5, 2020

That's a great idea and we should totally implement such a cop!

@stale
Copy link

stale bot commented Jul 3, 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 Jul 3, 2020
@koic koic removed the stale Issues that haven't been active in a while label Jul 3, 2020
koic added a commit to koic/ruby-style-guide that referenced this issue Oct 8, 2020
Follow rubocop/rubocop#7646.

This PR adds new "Arguments Forwarding" rule that prefers
Ruby 2.7's arguments forwarding syntax.

```ruby
# bad
def some_method(*args, &block)
  other_method(*args, &block)
end

# bad
def some_method(*args, **kwargs, &block)
  other_method(*args, **kwargs, &block)
end

# bad
# Please note that it can cause unexpected incompatible behavior
# because `...` forwards block also.
# rubocop/rubocop#7549
def some_method(*args)
  other_method(*args)
end

# good
def some_method(...)
  other_method(...)
end
```
bbatsov pushed a commit to rubocop/ruby-style-guide that referenced this issue Oct 8, 2020
Follow rubocop/rubocop#7646.

This PR adds new "Arguments Forwarding" rule that prefers
Ruby 2.7's arguments forwarding syntax.

```ruby
# bad
def some_method(*args, &block)
  other_method(*args, &block)
end

# bad
def some_method(*args, **kwargs, &block)
  other_method(*args, **kwargs, &block)
end

# bad
# Please note that it can cause unexpected incompatible behavior
# because `...` forwards block also.
# rubocop/rubocop#7549
def some_method(*args)
  other_method(*args)
end

# good
def some_method(...)
  other_method(...)
end
```
koic added a commit to koic/rubocop that referenced this issue Oct 22, 2020
Closes rubocop#7549.

This PR adds `Style/ArgumentsForwarding` cop.

As shown at rubocop#7549, the following code is unsafe to be
arguments forwarding.

```ruby
def foo(*args)
  bar(*args)
end
```

So by setting `AllowOnlyRestArgument` configuration option to true by default,
This PR makes the cop safe by default.
koic added a commit that referenced this issue Oct 22, 2020
[Fix #7549] Add `Style/ArgumentsForwarding` cop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants