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

OptionHash should accept reasonable usages #5652

Closed
swordray opened this issue Mar 8, 2018 · 1 comment
Closed

OptionHash should accept reasonable usages #5652

swordray opened this issue Mar 8, 2018 · 1 comment

Comments

@swordray
Copy link

swordray commented Mar 8, 2018

This is a case that override Rails image_tag for lazyload js. There is no need to expand options to keyword arguments. And the original method signature is exectly image_tag(source, options = {}).

module ApplicationHelper
  def image_tag(source, options = {})
    options['data-src'] = image_path(source).presence
    source = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=='
    super
  end
end

So should we check if options is used only accessing values or not?

Expected behavior

Don't flag an offense.

Actual behavior

Flag an offense.

Steps to reproduce the problem

Style/OptionHash:
  Enabled: true

RuboCop version

$ rubocop -V
0.53.0 (using Parser 2.5.0.2, running on ruby 2.5.0 x86_64-darwin17)
@Drenmi
Copy link
Collaborator

Drenmi commented Mar 16, 2018

RuboCop isn't capable of reason, so it can't really judge what is reasonable. 🙂

However, I think your example is valuable, because it calls super, which will pass the arguments on, indicating that the hash is expected. We could certainly account for this case.

Wei-LiangChew added a commit to Wei-LiangChew/rubocop that referenced this issue Apr 16, 2018
…r passing to super

Previously the cop would add an offense if a method is defined with an
options hash parameter, intended to be passed into a call to super.

The fix was to check if super is called in the method body, and not
register an offense if found.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants