Skip to content

Commit 9458f95

Browse files
koickoic
authored
Merge pull request #1481 from lovro-bikic/environment-comparison-to-sym
Make `Rails/EnvironmentComparison` aware of `Rails.env.to_sym`
2 parents 37a6760 + d1ef4bb commit 9458f95

File tree

3 files changed

+121
-48
lines changed

3 files changed

+121
-48
lines changed

‎changelog/change_recognize_to_sym_in_rails_environment_comparison.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1481](https://github.com/rubocop/rubocop-rails/pull/1481): Recognize `Rails.env.to_sym` in `Rails/EnvironmentComparison`. ([@lovro-bikic][])

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

Lines changed: 56 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33
module RuboCop
44
module Cop
55
module Rails
6-
# Checks that Rails.env is compared using `.production?`-like
6+
# Checks that `Rails.env` is compared using `.production?`-like
77
# methods instead of equality against a string or symbol.
88
#
99
# @example
1010
# # bad
1111
# Rails.env == 'production'
12+
# Rails.env.to_sym == :production
1213
#
1314
# # bad, always returns false
1415
# Rails.env == :test
@@ -18,26 +19,40 @@ module Rails
1819
class EnvironmentComparison < Base
1920
extend AutoCorrector
2021

21-
MSG = 'Favor `%<bang>sRails.env.%<env>s?` over `%<source>s`.'
22+
MSG = 'Favor `%<prefer>s` over `%<source>s`.'
2223

2324
SYM_MSG = 'Do not compare `Rails.env` with a symbol, it will always evaluate to `false`.'
2425

2526
RESTRICT_ON_SEND = %i[== !=].freeze
2627

27-
def_node_matcher :comparing_str_env_with_rails_env_on_lhs?, <<~PATTERN
28-
(send
29-
(send (const {nil? cbase} :Rails) :env)
30-
{:== :!=}
31-
$str
32-
)
28+
def_node_matcher :comparing_env_with_rails_env_on_lhs?, <<~PATTERN
29+
{
30+
(send
31+
(send (const {nil? cbase} :Rails) :env)
32+
{:== :!=}
33+
$str
34+
)
35+
(send
36+
(send (send (const {nil? cbase} :Rails) :env) :to_sym)
37+
{:== :!=}
38+
$sym
39+
)
40+
}
3341
PATTERN
3442

35-
def_node_matcher :comparing_str_env_with_rails_env_on_rhs?, <<~PATTERN
36-
(send
37-
$str
38-
{:== :!=}
39-
(send (const {nil? cbase} :Rails) :env)
40-
)
43+
def_node_matcher :comparing_env_with_rails_env_on_rhs?, <<~PATTERN
44+
{
45+
(send
46+
$str
47+
{:== :!=}
48+
(send (const {nil? cbase} :Rails) :env)
49+
)
50+
(send
51+
$sym
52+
{:== :!=}
53+
(send (send (const {nil? cbase} :Rails) :env) :to_sym)
54+
)
55+
}
4156
PATTERN
4257

4358
def_node_matcher :comparing_sym_env_with_rails_env_on_lhs?, <<~PATTERN
@@ -56,59 +71,52 @@ class EnvironmentComparison < Base
5671
)
5772
PATTERN
5873

59-
def_node_matcher :content, <<~PATTERN
60-
({str sym} $_)
61-
PATTERN
62-
6374
def on_send(node)
64-
if (env_node = comparing_str_env_with_rails_env_on_lhs?(node) ||
65-
comparing_str_env_with_rails_env_on_rhs?(node))
66-
env, = *env_node
67-
bang = node.method?(:!=) ? '!' : ''
68-
message = format(MSG, bang: bang, env: env, source: node.source)
69-
70-
add_offense(node, message: message) do |corrector|
71-
autocorrect(corrector, node)
72-
end
75+
check_env_comparison_with_rails_env(node)
76+
check_sym_env_comparison_with_rails_env(node)
77+
end
78+
79+
private
80+
81+
def check_env_comparison_with_rails_env(node)
82+
return unless comparing_env_with_rails_env_on_lhs?(node) || comparing_env_with_rails_env_on_rhs?(node)
83+
84+
replacement = build_predicate_method(node)
85+
message = format(MSG, prefer: replacement, source: node.source)
86+
87+
add_offense(node, message: message) do |corrector|
88+
corrector.replace(node, replacement)
7389
end
90+
end
7491

92+
def check_sym_env_comparison_with_rails_env(node)
7593
return unless comparing_sym_env_with_rails_env_on_lhs?(node) || comparing_sym_env_with_rails_env_on_rhs?(node)
7694

7795
add_offense(node, message: SYM_MSG) do |corrector|
78-
autocorrect(corrector, node)
96+
replacement = build_predicate_method(node)
97+
corrector.replace(node, replacement)
7998
end
8099
end
81100

82-
private
101+
def build_predicate_method(node)
102+
bang = node.method?(:!=) ? '!' : ''
83103

84-
def autocorrect(corrector, node)
85-
replacement = build_predicate_method(node)
104+
receiver, argument = extract_receiver_and_argument(node)
105+
receiver = receiver.receiver if receiver.method?(:to_sym)
86106

87-
corrector.replace(node, replacement)
107+
"#{bang}#{receiver.source}.#{argument.value}?"
88108
end
89109

90-
def build_predicate_method(node)
110+
def extract_receiver_and_argument(node)
91111
if rails_env_on_lhs?(node)
92-
build_predicate_method_for_rails_env_on_lhs(node)
112+
[node.receiver, node.first_argument]
93113
else
94-
build_predicate_method_for_rails_env_on_rhs(node)
114+
[node.first_argument, node.receiver]
95115
end
96116
end
97117

98118
def rails_env_on_lhs?(node)
99-
comparing_str_env_with_rails_env_on_lhs?(node) || comparing_sym_env_with_rails_env_on_lhs?(node)
100-
end
101-
102-
def build_predicate_method_for_rails_env_on_lhs(node)
103-
bang = node.method?(:!=) ? '!' : ''
104-
105-
"#{bang}#{node.receiver.source}.#{content(node.first_argument)}?"
106-
end
107-
108-
def build_predicate_method_for_rails_env_on_rhs(node)
109-
bang = node.method?(:!=) ? '!' : ''
110-
111-
"#{bang}#{node.first_argument.source}.#{content(node.receiver)}?"
119+
comparing_env_with_rails_env_on_lhs?(node) || comparing_sym_env_with_rails_env_on_lhs?(node)
112120
end
113121
end
114122
end

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,70 @@
101101
end
102102
end
103103

104+
context 'when comparing `Rails.env.to_sym` to a symbol' do
105+
context 'when using equals' do
106+
it 'registers an offense and corrects when `Rails.env.to_sym` is used on LHS' do
107+
expect_offense(<<~RUBY)
108+
Rails.env.to_sym == :production
109+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `Rails.env.production?` over `Rails.env.to_sym == :production`.
110+
RUBY
111+
112+
expect_correction(<<~RUBY)
113+
Rails.env.production?
114+
RUBY
115+
end
116+
117+
it 'registers an offense and corrects when `Rails.env.to_sym` is used on RHS' do
118+
expect_offense(<<~RUBY)
119+
:production == Rails.env.to_sym
120+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `Rails.env.production?` over `:production == Rails.env.to_sym`.
121+
RUBY
122+
123+
expect_correction(<<~RUBY)
124+
Rails.env.production?
125+
RUBY
126+
end
127+
end
128+
129+
context 'when using not equals' do
130+
it 'registers an offense and corrects when `Rails.env.to_sym` is used on LHS' do
131+
expect_offense(<<~RUBY)
132+
Rails.env.to_sym != :production
133+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `!Rails.env.production?` over `Rails.env.to_sym != :production`.
134+
RUBY
135+
136+
expect_correction(<<~RUBY)
137+
!Rails.env.production?
138+
RUBY
139+
end
140+
141+
it 'registers an offense and corrects when `Rails.env.to_sym` is used on RHS' do
142+
expect_offense(<<~RUBY)
143+
:production != Rails.env.to_sym
144+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `!Rails.env.production?` over `:production != Rails.env.to_sym`.
145+
RUBY
146+
147+
expect_correction(<<~RUBY)
148+
!Rails.env.production?
149+
RUBY
150+
end
151+
end
152+
end
153+
154+
context 'when comparing `Rails.env.to_sym` to a string' do
155+
it 'does not register when `Rails.env.to_sym` is used on LHS' do
156+
expect_no_offenses(<<~RUBY)
157+
Rails.env.to_sym == 'production'
158+
RUBY
159+
end
160+
161+
it 'does not register when `Rails.env.to_sym` is used on RHS' do
162+
expect_no_offenses(<<~RUBY)
163+
'production' == Rails.env.to_sym
164+
RUBY
165+
end
166+
end
167+
104168
it 'does not register an offense when using `#good_method`' do
105169
expect_no_offenses(<<~RUBY)
106170
Rails.env.production?

0 commit comments

Comments
 (0)