From a8eba4e371cbc1de7338ee3804710f9e180dc97d Mon Sep 17 00:00:00 2001 From: Anthony Robin Date: Fri, 21 Oct 2022 21:47:25 +0200 Subject: [PATCH] Extends `Rails/HttpStatus` cop to check `routes.rb` Fixes #822. This PR extends `Rails/HttpStatus` to detects offenses in `routes.rb` on redirections: ```ruby Rails.application.routes.draw do get '/foobar', to: redirect('/foobar/baz', status: 301) end ``` --- ...ange_extends_http_status_to_detect_redirect_routes.md | 1 + lib/rubocop/cop/rails/http_status.rb | 7 ++++++- spec/rubocop/cop/rails/http_status_spec.rb | 9 +++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 changelog/change_extends_http_status_to_detect_redirect_routes.md diff --git a/changelog/change_extends_http_status_to_detect_redirect_routes.md b/changelog/change_extends_http_status_to_detect_redirect_routes.md new file mode 100644 index 0000000000..4ece15cb20 --- /dev/null +++ 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 index b80498c27e..7b9d6e4d33 100644 --- a/lib/rubocop/cop/rails/http_status.rb +++ b/lib/rubocop/cop/rails/http_status.rb @@ -12,6 +12,7 @@ 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 @@ -19,6 +20,7 @@ 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) # # @example EnforcedStyle: numeric # # bad @@ -27,6 +29,7 @@ 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 @@ -34,18 +37,20 @@ module Rails # 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 index 4500be01ce..f254746e47 100644 --- 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