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

Style/ArgumentsForwarding mis-corrects when the called method has additional arguments. #12087

Closed
ccutrer opened this issue Jul 31, 2023 · 1 comment
Labels

Comments

@ccutrer
Copy link
Contributor

ccutrer commented Jul 31, 2023

Style/ArgumentsForwarding will replace all arguments to the called method with ..., even if there are arguments besides the *args, **kwargs, &.

Given the following code:

def self.get(*args, **kwargs, &)
  CanvasHttp.request(Net::HTTP::Get, *args, **kwargs, &)
end

Expected behavior

def self.get(...)
  CanvasHttp.request(Net::HTTP::Get, ...)
end

Actual behavior

def self.get(...)
  CanvasHttp.request(...)
end

Note the additional Net::HTTP::Get inline argument has been removed.

Steps to reproduce the problem

Auto-correct the above code with the Style/ArgumentsForwarding cop.

RuboCop version

1.55.1 (using Parser 3.2.2.3, rubocop-ast 1.29.0, running on ruby 3.1.3) [arm64-darwin21]
  - rubocop-factory_bot 2.23.1
  - rubocop-graphql 1.3.0
  - rubocop-performance 1.18.0
  - rubocop-rails 2.20.2
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.23.0

Note that I'm not sure if adding leading arguments is supported by all Ruby versions. I seem to remember when this syntax was introduced it was very limited. If that's the case, I would expect this cop to not even find an offense if the target Ruby version doesn't support leading arguments.

@owst
Copy link
Contributor

owst commented Aug 2, 2023

Note that I'm not sure if adding leading arguments is supported by all Ruby versions. I seem to remember when this syntax was introduced it was very limited. If that's the case, I would expect this cop to not even find an offense if the target Ruby version doesn't support leading arguments.

An excellent point - as per my testing, this was not supported in Ruby 2.7 until 2.7.3 after it was backported from 3.0.0.

I think it'll be easiest to just not support forward-all-with-leading-args (and treat this as no offense) on Ruby < 3.0

koic added a commit to koic/rubocop that referenced this issue Aug 8, 2023
Fixes rubocop#12087, rubocop#12089, rubocop#12096, and rubocop#12100.

This PR fixes false positives for `Style/ArgumentsForwarding`
when leading argument with rest arguments.
@koic koic closed this as completed in 08e614f Aug 8, 2023
koic added a commit that referenced this issue Aug 8, 2023
[Fix #12087] Fix false positives for Style/ArgumentsForwarding
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants