Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Rails/EnvironmentComparison cop detect comparisons in case statements #1408

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+58 −4
Open

Make Rails/EnvironmentComparison cop detect comparisons in case statements #1408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions lib/rubocop/cop/rails/environment_comparison.rb
View file
Open in desktop
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
View file
Open in desktop
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
Loading