It would be great for the new Rails/HttpPositionalArguments to be aware of the rails version it's inspecting to not suggest changes that incompatible with the running version.
The get :method, params: { ... } syntax is only valid in rails 5 and above. However, rubocop points it as an offense to use positional arguments even in rails 4.2 applications. In those, using the get :method, params: { ... } syntax breaks tests.
I would expect Rubocop to not point out those as offenses if verifying rails 4.2 (or lower) applications.
Rubocop points HttpPositionalArguments arguments but fixing them breaks the tests in Rails 4.2.
get :show, id: 1$ rubocop -V
0.44.1 (using Parser 2.3.1.4, running on ruby 2.3.1 x86_64-darwin15)
There's not support for Rails versions in cops, so right now in such cases cops should just be disabled. I'm guessing we can add a TargetRailsVersion similar to TargetRubyVersion to handle this problem.
Oh... I had no idea there was not target rails...
That seems like a reasonable amount of work then. I'll give it a shot later and try to put a PR together if you think this would be valuable.
There is an obvious workaround so not rush at all:
I disabled that specific rule (using .rubocop.yml) on my project and just have a TODO to reenable when I migrate to Rails 5.
Yea I knew about this issue when I created the new cop. Best scenario I could come up with was figuring out the rails gem version using something like Gem::Specification.latest_specs and searching for the rails version. However, since rubocop is not always executed from the root of the project with bundle exec there is no promise that the correct gem would be found.
So what if we added a rails version detection mechanism like so:
def rails_version
Gem::Specification.latest_specs.find {|f| f.name == 'rails' }.version
end
def rails5?
rails_version >= Gem::Version.new('5')
end
I still think we should go down the TargetRailsVersion route.
Is there an approach or issue for that?
I don't think there's a ticket but you basically need to replicate the existing functionality for TargetRubyVersion.
Most helpful comment
There's not support for Rails versions in cops, so right now in such cases cops should just be disabled. I'm guessing we can add a
TargetRailsVersionsimilar toTargetRubyVersionto handle this problem.