Skip to content

Commit 54cfc83

Browse files
committed
Add new Rails/HashLiteralKeysConversion cop
1 parent 7616bde commit 54cfc83

File tree

5 files changed

+377
-0
lines changed

5 files changed

+377
-0
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1332](https://github.com/rubocop/rubocop-rails/issues/1332): Add new `Rails/HashLiteralKeysConversion` cop. ([@fatkodima][])

config/default.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,11 @@ Rails/HasManyOrHasOneDependent:
540540
Include:
541541
- app/models/**/*.rb
542542

543+
Rails/HashLiteralKeysConversion:
544+
Description: 'Convert hash literal keys manually instead of using keys conversion methods.'
545+
Enabled: pending
546+
VersionAdded: '<<next>>'
547+
543548
Rails/HelperInstanceVariable:
544549
Description: 'Do not use instance variables in helpers.'
545550
Enabled: true
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# Detects when keys conversion methods are called on literal hashes, where it is redundant
7+
# or keys can be manually converted to the required type.
8+
#
9+
# @example
10+
# # bad
11+
# { a: 1, b: 2 }.symbolize_keys
12+
#
13+
# # bad
14+
# { a: 1, b: 2 }.stringify_keys
15+
#
16+
# # good
17+
# { 'a' => 1, 'b' => 2 }
18+
#
19+
# # good
20+
# { a: 1, var => 3 }.symbolize_keys
21+
#
22+
# # good
23+
# { a:, b: 2 }.stringify_keys
24+
# { a: 1, b: foo }.deep_stringify_keys
25+
#
26+
class HashLiteralKeysConversion < Base
27+
extend AutoCorrector
28+
29+
REDUNDANT_CONVERSION_MSG = 'Redundant hash keys conversion, all the keys have the required type.'
30+
MSG = 'Convert hash keys explicitly to the required type.'
31+
32+
CONVERSION_METHODS = {
33+
symbolize_keys: :sym,
34+
symbolize_keys!: :sym,
35+
stringify_keys: :str,
36+
stringify_keys!: :str,
37+
deep_symbolize_keys: :sym,
38+
deep_symbolize_keys!: :sym,
39+
deep_stringify_keys: :str,
40+
deep_stringify_keys!: :str
41+
}.freeze
42+
43+
RESTRICT_ON_SEND = CONVERSION_METHODS.keys
44+
45+
def on_send(node)
46+
return unless (receiver = node.receiver)&.hash_type?
47+
48+
type = CONVERSION_METHODS[node.method_name]
49+
deep = node.method_name.start_with?('deep_')
50+
return unless convertible_hash?(receiver, deep: deep)
51+
52+
check(node, receiver, type: type, deep: deep)
53+
end
54+
55+
# rubocop:disable Metrics/AbcSize
56+
def check(node, hash_node, type: :sym, deep: false)
57+
pair_nodes = pair_nodes(hash_node, deep: deep)
58+
59+
type_pairs, other_pairs = pair_nodes.partition { |pair_node| pair_node.key.type == type }
60+
61+
if type_pairs == pair_nodes
62+
add_offense(node.loc.selector, message: REDUNDANT_CONVERSION_MSG) do |corrector|
63+
corrector.remove(node.loc.dot)
64+
corrector.remove(node.loc.selector)
65+
end
66+
else
67+
add_offense(node.loc.selector) do |corrector|
68+
corrector.remove(node.loc.dot)
69+
corrector.remove(node.loc.selector)
70+
autocorrect_hash_keys(other_pairs, type, corrector)
71+
end
72+
end
73+
end
74+
# rubocop:enable Metrics/AbcSize
75+
76+
private
77+
78+
def convertible_hash?(node, deep: false)
79+
node.pairs.each do |pair|
80+
return false unless convertible_key?(pair)
81+
82+
_key, value = *pair
83+
84+
if deep
85+
if value.hash_type?
86+
return false unless convertible_hash?(value)
87+
elsif !value.literal?
88+
return false
89+
end
90+
end
91+
end
92+
93+
true
94+
end
95+
96+
def convertible_key?(pair)
97+
key, _value = *pair
98+
99+
(key.str_type? || key.sym_type?) && !pair.value_omission? && !key.value.match?(/\W/)
100+
end
101+
102+
def pair_nodes(hash_node, deep: false)
103+
if deep
104+
pair_nodes = []
105+
do_pair_nodes(hash_node, pair_nodes)
106+
pair_nodes
107+
else
108+
hash_node.pairs
109+
end
110+
end
111+
112+
def do_pair_nodes(hash_node, pair_nodes)
113+
hash_node.pairs.each do |pair_node|
114+
pair_nodes << pair_node
115+
do_pair_nodes(pair_node.value, pair_nodes) if pair_node.value.hash_type?
116+
end
117+
end
118+
119+
def autocorrect_hash_keys(pair_nodes, type, corrector)
120+
pair_nodes.each do |pair_node|
121+
if type == :sym
122+
corrector.replace(pair_node.key, ":#{pair_node.key.value}")
123+
else
124+
corrector.replace(pair_node.key, "'#{pair_node.key.source}'")
125+
end
126+
127+
corrector.replace(pair_node.loc.operator, '=>')
128+
end
129+
end
130+
end
131+
end
132+
end
133+
end

lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
require_relative 'rails/freeze_time'
6060
require_relative 'rails/has_and_belongs_to_many'
6161
require_relative 'rails/has_many_or_has_one_dependent'
62+
require_relative 'rails/hash_literal_keys_conversion'
6263
require_relative 'rails/helper_instance_variable'
6364
require_relative 'rails/http_positional_arguments'
6465
require_relative 'rails/http_status'
Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::HashLiteralKeysConversion, :config do
4+
it 'registers an offense and corrects when using `symbolize_keys` with only symbol keys' do
5+
expect_offense(<<~RUBY)
6+
{ a: 1, b: 2 }.symbolize_keys
7+
^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type.
8+
RUBY
9+
10+
expect_correction(<<~RUBY)
11+
{ a: 1, b: 2 }
12+
RUBY
13+
end
14+
15+
it 'registers an offense and corrects when using `symbolize_keys` with only symbol and string keys' do
16+
expect_offense(<<~RUBY)
17+
{ a: 1, 'b' => 2 }.symbolize_keys
18+
^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type.
19+
RUBY
20+
21+
expect_correction(<<~RUBY)
22+
{ a: 1, :b => 2 }
23+
RUBY
24+
end
25+
26+
it 'does not register an offense when using `symbolize_keys` with integer keys' do
27+
expect_no_offenses(<<~RUBY)
28+
{ a: 1, 2 => 3 }.symbolize_keys
29+
RUBY
30+
end
31+
32+
it 'does not register an offense when using `symbolize_keys` with non hash literal receiver' do
33+
expect_no_offenses(<<~RUBY)
34+
options.symbolize_keys
35+
RUBY
36+
end
37+
38+
it 'registers an offense and corrects when using `stringify_keys` with only string keys' do
39+
expect_offense(<<~RUBY)
40+
{ 'a' => 1, 'b' => 2 }.stringify_keys
41+
^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type.
42+
RUBY
43+
44+
expect_correction(<<~RUBY)
45+
{ 'a' => 1, 'b' => 2 }
46+
RUBY
47+
end
48+
49+
it 'registers an offense and corrects when using `stringify_keys` with only symbol and string keys' do
50+
expect_offense(<<~RUBY)
51+
{ a: 1, 'b' => 2 }.stringify_keys
52+
^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type.
53+
RUBY
54+
55+
expect_correction(<<~RUBY)
56+
{ 'a'=> 1, 'b' => 2 }
57+
RUBY
58+
end
59+
60+
it 'does not register an offense when using `stringify_keys` with integer keys' do
61+
expect_no_offenses(<<~RUBY)
62+
{ 'a' => 1, 2 => 3 }.stringify_keys
63+
RUBY
64+
end
65+
66+
it 'does not register an offense when using `stringify_keys` with non hash literal receiver' do
67+
expect_no_offenses(<<~RUBY)
68+
options.stringify_keys
69+
RUBY
70+
end
71+
72+
it 'registers an offense and corrects when using `deep_symbolize_keys` with symbol keys' do
73+
expect_offense(<<~RUBY)
74+
{
75+
a: 1,
76+
b: {
77+
c: 1
78+
}
79+
}.deep_symbolize_keys
80+
^^^^^^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type.
81+
RUBY
82+
83+
expect_correction(<<~RUBY)
84+
{
85+
a: 1,
86+
b: {
87+
c: 1
88+
}
89+
}
90+
RUBY
91+
end
92+
93+
it 'registers an offense and corrects when using `deep_symbolize_keys` with symbol and string keys' do
94+
expect_offense(<<~RUBY)
95+
{
96+
'a' => 1,
97+
b: {
98+
c: 1
99+
}
100+
}.deep_symbolize_keys
101+
^^^^^^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type.
102+
RUBY
103+
104+
expect_correction(<<~RUBY)
105+
{
106+
:a => 1,
107+
b: {
108+
c: 1
109+
}
110+
}
111+
RUBY
112+
end
113+
114+
it 'registers an offense and corrects when using `deep_symbolize_keys` with flat and only symbol and string keys' do
115+
expect_offense(<<~RUBY)
116+
{
117+
'a' => 1,
118+
b: 2
119+
}.deep_symbolize_keys
120+
^^^^^^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type.
121+
RUBY
122+
123+
expect_correction(<<~RUBY)
124+
{
125+
:a => 1,
126+
b: 2
127+
}
128+
RUBY
129+
end
130+
131+
it 'does not register an offense when using `deep_symbolize_keys` with integer keys' do
132+
expect_no_offenses(<<~RUBY)
133+
{
134+
'a' => 1,
135+
b: {
136+
2 => 3
137+
}
138+
}.deep_symbolize_keys
139+
RUBY
140+
end
141+
142+
it 'does not register an offense when using `deep_symbolize_keys` with non hash literal receiver' do
143+
expect_no_offenses(<<~RUBY)
144+
options.deep_symbolize_keys
145+
RUBY
146+
end
147+
148+
it 'does not register an offense when using `deep_symbolize_keys` with non literal values' do
149+
expect_no_offenses(<<~RUBY)
150+
{ 'a' => 1, b: foo }.deep_symbolize_keys
151+
RUBY
152+
end
153+
154+
it 'registers an offense and corrects when using `deep_stringify_keys` with only string keys' do
155+
expect_offense(<<~RUBY)
156+
{
157+
'a' => 1,
158+
'b' => {
159+
'c' => 1
160+
}
161+
}.deep_stringify_keys
162+
^^^^^^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type.
163+
RUBY
164+
165+
expect_correction(<<~RUBY)
166+
{
167+
'a' => 1,
168+
'b' => {
169+
'c' => 1
170+
}
171+
}
172+
RUBY
173+
end
174+
175+
it 'registers an offense and corrects when using `deep_stringify_keys` with only symbol and string keys' do
176+
expect_offense(<<~RUBY)
177+
{
178+
'a' => 1,
179+
b: {
180+
c: 1
181+
}
182+
}.deep_stringify_keys
183+
^^^^^^^^^^^^^^^^^^^ Convert hash keys explicitly to the required type.
184+
RUBY
185+
186+
expect_correction(<<~RUBY)
187+
{
188+
'a' => 1,
189+
'b'=> {
190+
'c'=> 1
191+
}
192+
}
193+
RUBY
194+
end
195+
196+
it 'does not register an offense when using `deep_stringify_keys` with integer keys' do
197+
expect_no_offenses(<<~RUBY)
198+
{
199+
'a' => 1,
200+
b: {
201+
2 => 3
202+
}
203+
}.deep_stringify_keys
204+
RUBY
205+
end
206+
207+
it 'does not register an offense when using `deep_stringify_keys` with non hash literal receiver' do
208+
expect_no_offenses(<<~RUBY)
209+
options.deep_stringify_keys
210+
RUBY
211+
end
212+
213+
it 'registers an offense and autocorrects when using `symbolize_keys` with empty hash literal' do
214+
expect_offense(<<~RUBY)
215+
{}.symbolize_keys
216+
^^^^^^^^^^^^^^ Redundant hash keys conversion, all the keys have the required type.
217+
RUBY
218+
219+
expect_correction(<<~RUBY)
220+
{}
221+
RUBY
222+
end
223+
224+
it 'does not register an offense when using `symbolize_keys` with non alphanumeric keys' do
225+
expect_no_offenses(<<~RUBY)
226+
{ 'hello world' => 1 }.symbolize_keys
227+
RUBY
228+
end
229+
230+
context 'Ruby >= 3.1', :ruby31 do
231+
it 'does not register an offense when using hash value omission' do
232+
expect_no_offenses(<<~RUBY)
233+
{ a:, b: 2 }.stringify_keys
234+
RUBY
235+
end
236+
end
237+
end

0 commit comments

Comments
 (0)