rubocop-rails icon indicating copy to clipboard operation
rubocop-rails copied to clipboard

Rails/HttpPositionalArguments autocorrection does the wrong thing for the "format" keyword

Open andreynering opened this issue 5 years ago • 3 comments

Expected behavior

This:

get :foo, id: 5, format: :json

Should be converted to:

get :foo, params: { id: 5 }, format: :json

Actual behavior

But it's actually converted to:

get :foo, params: { id: 5, format: :json }

In some situations, I've seen it been put inside the session keyword (?):

get :foo, session: { format: :json }

RuboCop version

$ bundle exec rubocop -V
0.76.0 (using Parser 2.7.0.2, running on ruby 2.5.5 x86_64-darwin18)

andreynering avatar Jan 09 '20 14:01 andreynering

I think format is not supported keyword. See https://api.rubyonrails.org/classes/ActionController/TestCase/Behavior.html#method-i-get It is fair for rubocop to think of it as any other parameter.

In fact, we have an explicit test-case for this: https://github.com/rubocop-hq/rubocop-rails/blob/38840ddbdcd0ac3a0b56ee4c169a10e5dca00cad/spec/rubocop/cop/rails/http_positional_arguments_spec.rb#L368-L377

tejasbubane avatar Apr 17 '20 11:04 tejasbubane

I've the same problem with a supported argument:

     def authorized_post(path, options = {})
-      post path, { headers: { 'Api-Key': api_access.key }, as: :json }.merge(options)
+      post path, params: { headers: { 'Api-Key': api_access.key }, as: :json }.merge(options)
     end

The suggested change is wrong and I don't think this is easily auto-correctable because the correction would go into this direction:

post path, params: options[:params],
           headers: { 'Api-Key': api_access.key }.merge(options[:headers] || {}),
           as: :json

schmijos avatar May 20 '20 08:05 schmijos

@schmijos This seems different from the original issue. It is mostly because of the parens and merge.

tejasbubane avatar May 20 '20 13:05 tejasbubane