Merge pull request #823 from anthony-robin/http-status-routes

[Fix #822] Extends `Rails/HttpStatus` cop to check `routes.rb`
rubocop · Oct 22, 2022 · e40d8b9 · e40d8b9
2 parents 6051ed8 + 1dcbda1
commit e40d8b9
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 1 deletion.
diff --git a/changelog/change_extends_http_status_to_detect_redirect_routes.md b/changelog/change_extends_http_status_to_detect_redirect_routes.md
@@ -0,0 +1 @@
+* [#822](https://github.com/rubocop/rubocop-rails/issues/822): Extends `Rails/HttpStatus` cop to check `routes.rb`. ([@anthony-robin][])
diff --git a/lib/rubocop/cop/rails/http_status.rb b/lib/rubocop/cop/rails/http_status.rb
@@ -12,13 +12,15 @@ module Rails
  # render plain: 'foo/bar', status: 304
  # redirect_to root_url, status: 301
  # head 200
+ # get '/foobar', to: redirect('/foobar/baz', status: 301)
  #
  # # good
  # render :foo, status: :ok
  # render json: { foo: 'bar' }, status: :ok
  # render plain: 'foo/bar', status: :not_modified
  # redirect_to root_url, status: :moved_permanently
  # head :ok
+ # get '/foobar', to: redirect('/foobar/baz', status: :moved_permanently)
  #
  # @example EnforcedStyle: numeric
  # # bad
@@ -27,25 +29,28 @@ module Rails
  # render plain: 'foo/bar', status: :not_modified
  # redirect_to root_url, status: :moved_permanently
  # head :ok
+ # get '/foobar', to: redirect('/foobar/baz', status: :moved_permanently)
  #
  # # good
  # render :foo, status: 200
  # render json: { foo: 'bar' }, status: 404
  # render plain: 'foo/bar', status: 304
  # redirect_to root_url, status: 301
  # head 200
+ # get '/foobar', to: redirect('/foobar/baz', status: 301)
  #
  class HttpStatus < Base
  include ConfigurableEnforcedStyle
  extend AutoCorrector
 
- RESTRICT_ON_SEND = %i[render redirect_to head].freeze
+ RESTRICT_ON_SEND = %i[render redirect_to head redirect].freeze
 
  def_node_matcher :http_status, <<~PATTERN
  {
  (send nil? {:render :redirect_to} _ $hash)
  (send nil? {:render :redirect_to} $hash)
  (send nil? :head ${int sym} ...)
+ (send nil? :redirect _ $hash)
  }
  PATTERN
 

diff --git a/spec/rubocop/cop/rails/http_status_spec.rb b/spec/rubocop/cop/rails/http_status_spec.rb
@@ -24,6 +24,8 @@
  ^^^ Prefer `:ok` over `200` to define HTTP status code.
  head 200, location: 'accounts'
  ^^^ Prefer `:ok` over `200` to define HTTP status code.
+ get '/foobar', to: redirect('/foobar/baz', status: 301)
+ ^^^ Prefer `:moved_permanently` over `301` to define HTTP status code.
  RUBY
 
  expect_correction(<<~RUBY)
@@ -36,6 +38,7 @@
  redirect_to root_path(utm_source: :pr, utm_medium: :web), status: :moved_permanently
  head :ok
  head :ok, location: 'accounts'
+ get '/foobar', to: redirect('/foobar/baz', status: :moved_permanently)
  RUBY
  end
 
@@ -47,6 +50,7 @@
  redirect_to root_url, status: :moved_permanently
  redirect_to root_path(utm_source: :pr, utm_medium: :web), status: :moved_permanently
  head :ok
+ get '/foobar', to: redirect('/foobar/baz', status: :moved_permanently)
  RUBY
  end
 
@@ -58,6 +62,7 @@
  redirect_to root_url, status: 550
  redirect_to root_path(utm_source: :pr, utm_medium: :web), status: 550
  head 550
+ get '/foobar', to: redirect('/foobar/baz', status: 550)
  RUBY
  end
  end
@@ -85,6 +90,8 @@
  ^^^ Prefer `200` over `:ok` to define HTTP status code.
  head :ok, location: 'accounts'
  ^^^ Prefer `200` over `:ok` to define HTTP status code.
+ get '/foobar', to: redirect('/foobar/baz', status: :moved_permanently)
+ ^^^^^^^^^^^^^^^^^^ Prefer `301` over `:moved_permanently` to define HTTP status code.
  RUBY
 
  expect_correction(<<~RUBY)
@@ -97,6 +104,7 @@
  redirect_to root_path(utm_source: :pr, utm_medium: :web), status: 301
  head 200
  head 200, location: 'accounts'
+ get '/foobar', to: redirect('/foobar/baz', status: 301)
  RUBY
  end
 
@@ -108,6 +116,7 @@
  redirect_to root_url, status: 301
  redirect_to root_path(utm_source: :pr, utm_medium: :web), status: 301
  head 200
+ get '/foobar', to: redirect('/foobar/baz', status: 301)
  RUBY
  end