Skip to content

Commit 37a6760

Browse files
koickoic
authored
Merge pull request #1520 from tuxagon/unprocessable-content-status-cop
Add `Rails/HttpStatusNameConsistency` cop
2 parents 9e925de + 45b6bbb commit 37a6760

File tree

5 files changed

+318
-0
lines changed

5 files changed

+318
-0
lines changed

‎changelog/new_deprecated_http_status_names_cop.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1520](https://github.com/rubocop/rubocop-rails/pull/1520): New `Rails/HttpStatusNameConsistency` cop. ([@tuxagon][])

‎config/default.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,14 @@ Rails/HttpStatus:
614614
- numeric
615615
- symbolic
616616

617+
Rails/HttpStatusNameConsistency:
618+
Description: 'Enforces consistency by using the current HTTP status names.'
619+
Enabled: pending
620+
Severity: warning
621+
VersionAdded: '<<next>>'
622+
Include:
623+
- '**/app/controllers/**/*.rb'
624+
617625
Rails/I18nLazyLookup:
618626
Description: 'Checks for places where I18n "lazy" lookup can be used.'
619627
StyleGuide: 'https://rails.rubystyle.guide/#lazy-lookup'

‎lib/rubocop/cop/rails/http_status_name_consistency.rb‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# Enforces consistency by using the current HTTP status names.
7+
#
8+
# @example
9+
# # bad
10+
# render json: { error: "Invalid data" }, status: :unprocessable_entity
11+
# head :payload_too_large
12+
#
13+
# # good
14+
# render json: { error: "Invalid data" }, status: :unprocessable_content
15+
# head :content_too_large
16+
#
17+
class HttpStatusNameConsistency < Base
18+
extend AutoCorrector
19+
20+
requires_gem 'rack', '>= 3.1.0'
21+
22+
RESTRICT_ON_SEND = %i[render redirect_to head assert_response assert_redirected_to].freeze
23+
24+
PREFERRED_STATUSES = {
25+
unprocessable_entity: :unprocessable_content,
26+
payload_too_large: :content_too_large
27+
}.freeze
28+
29+
MSG = 'Prefer `:%<preferred>s` over `:%<current>s`.'
30+
31+
def_node_matcher :status_method_call, <<~PATTERN
32+
{
33+
(send nil? {:render :redirect_to} _ $hash)
34+
(send nil? {:render :redirect_to} $hash)
35+
(send nil? {:head :assert_response} $_ ...)
36+
(send nil? :assert_redirected_to _ $hash ...)
37+
(send nil? :assert_redirected_to $hash ...)
38+
}
39+
PATTERN
40+
41+
def_node_matcher :status_hash_value, <<~PATTERN
42+
(hash <(pair (sym :status) $_) ...>)
43+
PATTERN
44+
45+
def on_send(node)
46+
status_method_call(node) do |status_node|
47+
if status_node.hash_type?
48+
# Handle hash arguments like { status: :unprocessable_entity }
49+
status_hash_value(status_node) do |status_value|
50+
check_status_name_consistency(status_value)
51+
end
52+
else
53+
# Handle positional arguments like head :unprocessable_entity
54+
check_status_name_consistency(status_node)
55+
end
56+
end
57+
end
58+
59+
private
60+
61+
def check_status_name_consistency(node)
62+
if node.sym_type? && PREFERRED_STATUSES.key?(node.value)
63+
current_status = node.value
64+
preferred_status = PREFERRED_STATUSES[current_status]
65+
66+
message = format(MSG, current: current_status, preferred: preferred_status)
67+
68+
add_offense(node, message: message) do |corrector|
69+
corrector.replace(node, ":#{preferred_status}")
70+
end
71+
else
72+
node.children.each do |child|
73+
check_status_name_consistency(child) if child.is_a?(Parser::AST::Node)
74+
end
75+
end
76+
end
77+
end
78+
end
79+
end
80+
end

‎lib/rubocop/cop/rails_cops.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
require_relative 'rails/helper_instance_variable'
6868
require_relative 'rails/http_positional_arguments'
6969
require_relative 'rails/http_status'
70+
require_relative 'rails/http_status_name_consistency'
7071
require_relative 'rails/i18n_lazy_lookup'
7172
require_relative 'rails/i18n_locale_assignment'
7273
require_relative 'rails/i18n_locale_texts'

‎spec/rubocop/cop/rails/http_status_name_consistency_spec.rb‎

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::HttpStatusNameConsistency, :config do
4+
context 'when Rack is older than 3.1' do
5+
let(:gem_versions) { { 'rack' => '3.0.0' } }
6+
7+
it 'does nothing' do
8+
expect_no_offenses(<<~RUBY)
9+
render json: { error: 'Invalid data' }, status: :unprocessable_entity
10+
head :payload_too_large
11+
RUBY
12+
end
13+
end
14+
15+
context 'when Rack is 3.1 or later' do
16+
let(:gem_versions) { { 'rack' => '3.1.0' } }
17+
18+
context 'with :unprocessable_entity' do
19+
it 'registers an offense when using :unprocessable_entity in render' do
20+
expect_offense(<<~RUBY)
21+
render json: { error: 'Invalid data' }, status: :unprocessable_entity
22+
^^^^^^^^^^^^^^^^^^^^^ Prefer `:unprocessable_content` over `:unprocessable_entity`.
23+
RUBY
24+
25+
expect_correction(<<~RUBY)
26+
render json: { error: 'Invalid data' }, status: :unprocessable_content
27+
RUBY
28+
end
29+
30+
it 'registers an offense when using :unprocessable_entity in head' do
31+
expect_offense(<<~RUBY)
32+
head :unprocessable_entity
33+
^^^^^^^^^^^^^^^^^^^^^ Prefer `:unprocessable_content` over `:unprocessable_entity`.
34+
RUBY
35+
36+
expect_correction(<<~RUBY)
37+
head :unprocessable_content
38+
RUBY
39+
end
40+
41+
it 'registers an offense when using :unprocessable_entity in redirect_to' do
42+
expect_offense(<<~RUBY)
43+
redirect_to some_path, status: :unprocessable_entity
44+
^^^^^^^^^^^^^^^^^^^^^ Prefer `:unprocessable_content` over `:unprocessable_entity`.
45+
RUBY
46+
47+
expect_correction(<<~RUBY)
48+
redirect_to some_path, status: :unprocessable_content
49+
RUBY
50+
end
51+
52+
it 'registers an offense when using :unprocessable_entity in assert_response' do
53+
expect_offense(<<~RUBY)
54+
assert_response :unprocessable_entity
55+
^^^^^^^^^^^^^^^^^^^^^ Prefer `:unprocessable_content` over `:unprocessable_entity`.
56+
RUBY
57+
58+
expect_correction(<<~RUBY)
59+
assert_response :unprocessable_content
60+
RUBY
61+
end
62+
63+
it 'registers an offense when using :unprocessable_entity in assert_redirected_to' do
64+
expect_offense(<<~RUBY)
65+
assert_redirected_to some_path, status: :unprocessable_entity
66+
^^^^^^^^^^^^^^^^^^^^^ Prefer `:unprocessable_content` over `:unprocessable_entity`.
67+
RUBY
68+
69+
expect_correction(<<~RUBY)
70+
assert_redirected_to some_path, status: :unprocessable_content
71+
RUBY
72+
end
73+
74+
it 'registers an offense when using :unprocessable_entity in ternary expression' do
75+
expect_offense(<<~RUBY)
76+
render json: { error: 'Invalid data' }, status: some_condition ? :unprocessable_entity : :ok
77+
^^^^^^^^^^^^^^^^^^^^^ Prefer `:unprocessable_content` over `:unprocessable_entity`.
78+
RUBY
79+
80+
expect_correction(<<~RUBY)
81+
render json: { error: 'Invalid data' }, status: some_condition ? :unprocessable_content : :ok
82+
RUBY
83+
end
84+
85+
it 'does not register an offense when using :unprocessable_content' do
86+
expect_no_offenses(<<~RUBY)
87+
render json: { error: 'Invalid data' }, status: :unprocessable_content
88+
RUBY
89+
end
90+
91+
it 'does not register an offense when using :unprocessable_entity in hash key' do
92+
expect_no_offenses(<<~RUBY)
93+
{ unprocessable_entity: 'Invalid data' }
94+
RUBY
95+
end
96+
97+
it 'does not register an offense when status is provided via variable' do
98+
expect_no_offenses(<<~RUBY)
99+
status_var = :unprocessable_entity
100+
head status_var
101+
render json: { error: 'Invalid data' }, status: status_var
102+
redirect_to some_path, status: status_var
103+
assert_response status_var
104+
RUBY
105+
end
106+
107+
it 'does not register an offense when status is provided via method call' do
108+
expect_no_offenses(<<~RUBY)
109+
head get_status_code
110+
render json: { error: 'Invalid data' }, status: calculate_status
111+
RUBY
112+
end
113+
end
114+
115+
context 'with :payload_too_large' do
116+
it 'registers an offense when using :payload_too_large in render' do
117+
expect_offense(<<~RUBY)
118+
render json: { error: 'File too big' }, status: :payload_too_large
119+
^^^^^^^^^^^^^^^^^^ Prefer `:content_too_large` over `:payload_too_large`.
120+
RUBY
121+
122+
expect_correction(<<~RUBY)
123+
render json: { error: 'File too big' }, status: :content_too_large
124+
RUBY
125+
end
126+
127+
it 'registers an offense when using :payload_too_large in head' do
128+
expect_offense(<<~RUBY)
129+
head :payload_too_large
130+
^^^^^^^^^^^^^^^^^^ Prefer `:content_too_large` over `:payload_too_large`.
131+
RUBY
132+
133+
expect_correction(<<~RUBY)
134+
head :content_too_large
135+
RUBY
136+
end
137+
138+
it 'registers an offense when using :payload_too_large in redirect_to' do
139+
expect_offense(<<~RUBY)
140+
redirect_to some_path, status: :payload_too_large
141+
^^^^^^^^^^^^^^^^^^ Prefer `:content_too_large` over `:payload_too_large`.
142+
RUBY
143+
144+
expect_correction(<<~RUBY)
145+
redirect_to some_path, status: :content_too_large
146+
RUBY
147+
end
148+
149+
it 'registers an offense when using :payload_too_large in assert_response' do
150+
expect_offense(<<~RUBY)
151+
assert_response :payload_too_large
152+
^^^^^^^^^^^^^^^^^^ Prefer `:content_too_large` over `:payload_too_large`.
153+
RUBY
154+
155+
expect_correction(<<~RUBY)
156+
assert_response :content_too_large
157+
RUBY
158+
end
159+
160+
it 'registers an offense when using :payload_too_large in assert_redirected_to' do
161+
expect_offense(<<~RUBY)
162+
assert_redirected_to some_path, status: :payload_too_large
163+
^^^^^^^^^^^^^^^^^^ Prefer `:content_too_large` over `:payload_too_large`.
164+
RUBY
165+
166+
expect_correction(<<~RUBY)
167+
assert_redirected_to some_path, status: :content_too_large
168+
RUBY
169+
end
170+
171+
it 'registers an offense when using :payload_too_large in ternary expression' do
172+
expect_offense(<<~RUBY)
173+
render json: { error: 'File too big' }, status: some_condition ? :payload_too_large : :ok
174+
^^^^^^^^^^^^^^^^^^ Prefer `:content_too_large` over `:payload_too_large`.
175+
RUBY
176+
177+
expect_correction(<<~RUBY)
178+
render json: { error: 'File too big' }, status: some_condition ? :content_too_large : :ok
179+
RUBY
180+
end
181+
182+
it 'does not register an offense when using :content_too_large' do
183+
expect_no_offenses(<<~RUBY)
184+
render json: { error: 'File too big' }, status: :content_too_large
185+
RUBY
186+
end
187+
188+
it 'does not register an offense when using :payload_too_large in hash key' do
189+
expect_no_offenses(<<~RUBY)
190+
{ payload_too_large: 'File too big' }
191+
RUBY
192+
end
193+
194+
it 'does not register an offense when status is provided via variable' do
195+
expect_no_offenses(<<~RUBY)
196+
status_var = :payload_too_large
197+
head status_var
198+
render json: { error: 'Invalid data' }, status: status_var
199+
redirect_to some_path, status: status_var
200+
assert_response status_var
201+
RUBY
202+
end
203+
204+
it 'does not register an offense when status is provided via method call' do
205+
expect_no_offenses(<<~RUBY)
206+
head get_status_code
207+
render json: { error: 'Invalid data' }, status: calculate_status
208+
RUBY
209+
end
210+
end
211+
212+
context 'with partial preferred statuses' do
213+
it 'handles status preference in the same code' do
214+
expect_offense(<<~RUBY)
215+
head :unprocessable_entity
216+
^^^^^^^^^^^^^^^^^^^^^ Prefer `:unprocessable_content` over `:unprocessable_entity`.
217+
head :payload_too_large
218+
^^^^^^^^^^^^^^^^^^ Prefer `:content_too_large` over `:payload_too_large`.
219+
RUBY
220+
221+
expect_correction(<<~RUBY)
222+
head :unprocessable_content
223+
head :content_too_large
224+
RUBY
225+
end
226+
end
227+
end
228+
end

0 commit comments

Comments
 (0)