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.

Sign up for GitHub

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

Jump to bottom

False-positive in Rails/HttpPositionalArguments #532

Closed
pyromaniac opened this issue Sep 1, 2021 · 0 comments · Fixed by #533
Closed

False-positive in Rails/HttpPositionalArguments #532

pyromaniac opened this issue Sep 1, 2021 · 0 comments · Fixed by #533
Labels
bug Something isn't working

Comments

@pyromaniac
Copy link

pyromaniac commented Sep 1, 2021 •
edited
Loading

When routes are defined in controller specs, they might use the same method manes as this cop checks. We probably have to determine is we are inside of the Rails.application.routes.draw do or routes do block (the latter one is provided by rspec) and ignore HTTP verb method calls there.

Expected behavior

I expect the cop to ignore HTTP method request helpers invocation inside of routes definitions.

Actual behavior

Rails/HttpPositionalArguments catches route definition helpers invocation and raises an offense.

Steps to reproduce the problem

# frozen_string_literal: true

RSpec.describe FooController do
  before do
    routes do
      get :list, on: :collection
    end

    Rails.application.routes.draw do
      get :list, on: :collection
    end
  end

  specify do
    get :list, filter: 42
  end
end

Produces:

rubocop spec/controllers/foo_controller_spec.rb
Inspecting 1 file
C

Offenses:

spec/controllers/foo_controller_spec.rb:6:18: C: [Correctable] Rails/HttpPositionalArguments: Use keyword arguments instead of positional arguments for http call: get.
      get :list, on: :collection
                 ^^^^^^^^^^^^^^^
spec/controllers/foo_controller_spec.rb:10:18: C: [Correctable] Rails/HttpPositionalArguments: Use keyword arguments instead of positional arguments for http call: get.
      get :list, on: :collection
                 ^^^^^^^^^^^^^^^
spec/controllers/foo_controller_spec.rb:15:16: C: [Correctable] Rails/HttpPositionalArguments: Use keyword arguments instead of positional arguments for http call: get.
    get :list, filter: 42
               ^^^^^^^^^^

1 file inspected, 3 offenses detected, 3 offenses auto-correctable

RuboCop version

$ [bundle exec] rubocop -V
1.20.0 (using Parser 3.0.2.0, rubocop-ast 1.11.0, running on ruby 2.7.4 x86_64-darwin20)
  - rubocop-performance 1.11.5
  - rubocop-rails 2.11.3
  - rubocop-rspec 2.4.0
@koic koic added the bug Something isn't working label Sep 2, 2021
koic added a commit to koic/rubocop-rails that referenced this issue Sep 2, 2021
@koic
[Fix rubocop#532] Fix a false positive for `Rails/HttpPositionalArgum…
b72c896 
…ents`

Fixes rubocop#532.

This PR fixes a false positive for `Rails/HttpPositionalArguments`
when defining `get` in `Rails.application.routes.draw` block.
@koic koic mentioned this issue Sep 2, 2021
Merged
9 tasks
koic added a commit to koic/rubocop-rails that referenced this issue Sep 3, 2021
@koic
[Fix rubocop#532] Fix a false positive for `Rails/HttpPositionalArgum…
4a81d3a 
…ents`

Fixes rubocop#532.

This PR fixes a false positive for `Rails/HttpPositionalArguments`
when defining `get` in `Rails.application.routes.draw` block.
@koic koic closed this as completed in #533 Sep 4, 2021
koic added a commit that referenced this issue Sep 4, 2021
@koic
Merge pull request #533 from koic/fix_false_positive_for_rails_http_p…
732a0db 
…osisional_arguments

[Fix #532] Fix a false positive for `Rails/HttpPositionalArguments`
smcabrera added a commit to smcabrera/rubocop-rails that referenced this issue Dec 27, 2022
@smcabrera
Update allowlist of methods to ignore for false positives
443e63a 
This PR rubocop@732a0db

This issue was reported a while back finding that the HTTP positional arguments cop could give false positives rubocop#532

I found another instance of false positives when the `append` method is used instead of `draw` or `routes`. If we add that to the list as well this problem seems to go away.
@smcabrera smcabrera mentioned this issue Dec 27, 2022
Draft
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #532] Fix a false positive for Rails/HttpPositionalArguments koic/rubocop-rails
2 participants
@koic @pyromaniac