Skip to content

Commit

Permalink
Revert "Merge pull request rubocop#823 from anthony-robin/http-status…
Browse files Browse the repository at this point in the history
…-routes"

This reverts commit e40d8b9, reversing
changes made to 6051ed8.

The auto-corrected code is not compatible with Rails/Puma, resulting the
error:

    Puma caught this error: undefined method `to_i' for :moved_permanently:Symbol

              status = status.to_i
                             ^^^^^
    Did you mean?  to_s (NoMethodError)
    /.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/puma-5.6.5/lib/puma/request.rb:82:in `handle_request'
    /.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/puma-5.6.5/lib/puma/server.rb:443:in `process_client'
    /.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/puma-5.6.5/lib/puma/thread_pool.rb:147:in `block in spawn_thread'
  • Loading branch information
jdufresne committed Oct 23, 2022
1 parent 1acb097 commit 9df4e20
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 15 deletions.
1 change: 1 addition & 0 deletions changelog/change_revert_merge_pull_request_823_from.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#x](https://github.com/rubocop/rubocop-rails/pull/x): Reverted "Extends `Rails/HttpStatus` cop to check `routes.rb`" introduced in 2.17.0. ([@jdufresne][])
7 changes: 1 addition & 6 deletions lib/rubocop/cop/rails/http_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@ 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
Expand All @@ -29,28 +27,25 @@ 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 redirect].freeze
RESTRICT_ON_SEND = %i[render redirect_to head].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

Expand Down
9 changes: 0 additions & 9 deletions spec/rubocop/cop/rails/http_status_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
^^^ 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)
Expand All @@ -38,7 +36,6 @@
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

Expand All @@ -50,7 +47,6 @@
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

Expand All @@ -62,7 +58,6 @@
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
Expand Down Expand Up @@ -90,8 +85,6 @@
^^^ 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)
Expand All @@ -104,7 +97,6 @@
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

Expand All @@ -116,7 +108,6 @@
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

Expand Down

0 comments on commit 9df4e20

Please sign in to comment.