Skip to content

Commit

Permalink
[Fix #1395] Make Rails/EnvironmentComparison cop detect comparisons…
Browse files Browse the repository at this point in the history
… in case statements

Closes #1395
  • Loading branch information
tejasbubane committed Jan 8, 2025
1 parent 88b9dc5 commit 0969b97
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 4 deletions.
21 changes: 17 additions & 4 deletions lib/rubocop/cop/rails/environment_comparison.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ class EnvironmentComparison < Base

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

CASE_MSG = 'Favor environment check predicate methods over case comparison.'

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

def_node_matcher :comparing_str_env_with_rails_env_on_lhs?, <<~PATTERN
(send
(send (const {nil? cbase} :Rails) :env)
#rails_env_const?
{:== :!=}
$str
)
Expand All @@ -36,13 +38,13 @@ class EnvironmentComparison < Base
(send
$str
{:== :!=}
(send (const {nil? cbase} :Rails) :env)
#rails_env_const?
)
PATTERN

def_node_matcher :comparing_sym_env_with_rails_env_on_lhs?, <<~PATTERN
(send
(send (const {nil? cbase} :Rails) :env)
#rails_env_const?
{:== :!=}
$sym
)
Expand All @@ -52,14 +54,18 @@ class EnvironmentComparison < Base
(send
$sym
{:== :!=}
(send (const {nil? cbase} :Rails) :env)
#rails_env_const?
)
PATTERN

def_node_matcher :content, <<~PATTERN
({str sym} $_)
PATTERN

def_node_matcher :rails_env_const?, <<~PATTERN
(send (const {nil? cbase} :Rails) :env)
PATTERN

def on_send(node)
if (env_node = comparing_str_env_with_rails_env_on_lhs?(node) ||
comparing_str_env_with_rails_env_on_rhs?(node))
Expand All @@ -79,6 +85,13 @@ def on_send(node)
end
end

def on_case(case_node)
condition = case_node.condition
return unless rails_env_const?(condition)

add_offense(condition, message: CASE_MSG)
end

private

def autocorrect(corrector, node)
Expand Down
41 changes: 41 additions & 0 deletions spec/rubocop/cop/rails/environment_comparison_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,51 @@
end
end

context 'when comparing Rails.env using a case statement' do
it 'registers an offense' do
expect_offense(<<~RUBY)
case Rails.env
^^^^^^^^^ Favor environment check predicate methods over case comparison.
when "production"
do_production_thing
when "staging"
do_staging_thing
else
do_other_thing
end
RUBY
end

context 'with pattern matching' do
it 'registers an offense' do
expect_offense(<<~RUBY)
case Rails.env
^^^^^^^^^ Favor environment check predicate methods over case comparison.
when "test" | "development"
do_test_thing
else
do_other_thing
end
RUBY
end
end
end

it 'does not register an offense when using `#good_method`' do
expect_no_offenses(<<~RUBY)
Rails.env.production?
Rails.env.test?
RUBY
end

it 'does not register an offense for other case statements' do
expect_no_offenses(<<~RUBY)
case some_method
when "test" | "development"
do_test_thing
else
do_other_thing
end
RUBY
end
end

0 comments on commit 0969b97

Please sign in to comment.