diff --git a/changelog/change_recognize_to_sym_in_rails_environment_comparison.md b/changelog/change_recognize_to_sym_in_rails_environment_comparison.md new file mode 100644 index 0000000000..556596a06b --- /dev/null +++ b/changelog/change_recognize_to_sym_in_rails_environment_comparison.md @@ -0,0 +1 @@ +* [#1481](https://github.com/rubocop/rubocop-rails/pull/1481): Recognize `Rails.env.to_sym` in `Rails/EnvironmentComparison`. ([@lovro-bikic][]) diff --git a/lib/rubocop/cop/rails/environment_comparison.rb b/lib/rubocop/cop/rails/environment_comparison.rb index 456bc85dde..6d902f1ad6 100644 --- a/lib/rubocop/cop/rails/environment_comparison.rb +++ b/lib/rubocop/cop/rails/environment_comparison.rb @@ -3,12 +3,13 @@ module RuboCop module Cop module Rails - # Checks that Rails.env is compared using `.production?`-like + # Checks that `Rails.env` is compared using `.production?`-like # methods instead of equality against a string or symbol. # # @example # # bad # Rails.env == 'production' + # Rails.env.to_sym == :production # # # bad, always returns false # Rails.env == :test @@ -18,26 +19,40 @@ module Rails class EnvironmentComparison < Base extend AutoCorrector - MSG = 'Favor `%sRails.env.%s?` over `%s`.' + MSG = 'Favor `%s` over `%s`.' SYM_MSG = 'Do not compare `Rails.env` with a symbol, it will always evaluate to `false`.' RESTRICT_ON_SEND = %i[== !=].freeze - def_node_matcher :comparing_str_env_with_rails_env_on_lhs?, <<~PATTERN - (send - (send (const {nil? cbase} :Rails) :env) - {:== :!=} - $str - ) + def_node_matcher :comparing_env_with_rails_env_on_lhs?, <<~PATTERN + { + (send + (send (const {nil? cbase} :Rails) :env) + {:== :!=} + $str + ) + (send + (send (send (const {nil? cbase} :Rails) :env) :to_sym) + {:== :!=} + $sym + ) + } PATTERN - def_node_matcher :comparing_str_env_with_rails_env_on_rhs?, <<~PATTERN - (send - $str - {:== :!=} - (send (const {nil? cbase} :Rails) :env) - ) + def_node_matcher :comparing_env_with_rails_env_on_rhs?, <<~PATTERN + { + (send + $str + {:== :!=} + (send (const {nil? cbase} :Rails) :env) + ) + (send + $sym + {:== :!=} + (send (send (const {nil? cbase} :Rails) :env) :to_sym) + ) + } PATTERN def_node_matcher :comparing_sym_env_with_rails_env_on_lhs?, <<~PATTERN @@ -56,59 +71,52 @@ class EnvironmentComparison < Base ) PATTERN - def_node_matcher :content, <<~PATTERN - ({str sym} $_) - 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)) - env, = *env_node - bang = node.method?(:!=) ? '!' : '' - message = format(MSG, bang: bang, env: env, source: node.source) - - add_offense(node, message: message) do |corrector| - autocorrect(corrector, node) - end + check_env_comparison_with_rails_env(node) + check_sym_env_comparison_with_rails_env(node) + end + + private + + def check_env_comparison_with_rails_env(node) + return unless comparing_env_with_rails_env_on_lhs?(node) || comparing_env_with_rails_env_on_rhs?(node) + + replacement = build_predicate_method(node) + message = format(MSG, prefer: replacement, source: node.source) + + add_offense(node, message: message) do |corrector| + corrector.replace(node, replacement) end + end + def check_sym_env_comparison_with_rails_env(node) return unless comparing_sym_env_with_rails_env_on_lhs?(node) || comparing_sym_env_with_rails_env_on_rhs?(node) add_offense(node, message: SYM_MSG) do |corrector| - autocorrect(corrector, node) + replacement = build_predicate_method(node) + corrector.replace(node, replacement) end end - private + def build_predicate_method(node) + bang = node.method?(:!=) ? '!' : '' - def autocorrect(corrector, node) - replacement = build_predicate_method(node) + receiver, argument = extract_receiver_and_argument(node) + receiver = receiver.receiver if receiver.method?(:to_sym) - corrector.replace(node, replacement) + "#{bang}#{receiver.source}.#{argument.value}?" end - def build_predicate_method(node) + def extract_receiver_and_argument(node) if rails_env_on_lhs?(node) - build_predicate_method_for_rails_env_on_lhs(node) + [node.receiver, node.first_argument] else - build_predicate_method_for_rails_env_on_rhs(node) + [node.first_argument, node.receiver] end end def rails_env_on_lhs?(node) - comparing_str_env_with_rails_env_on_lhs?(node) || comparing_sym_env_with_rails_env_on_lhs?(node) - end - - def build_predicate_method_for_rails_env_on_lhs(node) - bang = node.method?(:!=) ? '!' : '' - - "#{bang}#{node.receiver.source}.#{content(node.first_argument)}?" - end - - def build_predicate_method_for_rails_env_on_rhs(node) - bang = node.method?(:!=) ? '!' : '' - - "#{bang}#{node.first_argument.source}.#{content(node.receiver)}?" + comparing_env_with_rails_env_on_lhs?(node) || comparing_sym_env_with_rails_env_on_lhs?(node) end end end diff --git a/spec/rubocop/cop/rails/environment_comparison_spec.rb b/spec/rubocop/cop/rails/environment_comparison_spec.rb index 1254ebee8f..0342e1a2f9 100644 --- a/spec/rubocop/cop/rails/environment_comparison_spec.rb +++ b/spec/rubocop/cop/rails/environment_comparison_spec.rb @@ -101,6 +101,70 @@ end end + context 'when comparing `Rails.env.to_sym` to a symbol' do + context 'when using equals' do + it 'registers an offense and corrects when `Rails.env.to_sym` is used on LHS' do + expect_offense(<<~RUBY) + Rails.env.to_sym == :production + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `Rails.env.production?` over `Rails.env.to_sym == :production`. + RUBY + + expect_correction(<<~RUBY) + Rails.env.production? + RUBY + end + + it 'registers an offense and corrects when `Rails.env.to_sym` is used on RHS' do + expect_offense(<<~RUBY) + :production == Rails.env.to_sym + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `Rails.env.production?` over `:production == Rails.env.to_sym`. + RUBY + + expect_correction(<<~RUBY) + Rails.env.production? + RUBY + end + end + + context 'when using not equals' do + it 'registers an offense and corrects when `Rails.env.to_sym` is used on LHS' do + expect_offense(<<~RUBY) + Rails.env.to_sym != :production + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `!Rails.env.production?` over `Rails.env.to_sym != :production`. + RUBY + + expect_correction(<<~RUBY) + !Rails.env.production? + RUBY + end + + it 'registers an offense and corrects when `Rails.env.to_sym` is used on RHS' do + expect_offense(<<~RUBY) + :production != Rails.env.to_sym + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor `!Rails.env.production?` over `:production != Rails.env.to_sym`. + RUBY + + expect_correction(<<~RUBY) + !Rails.env.production? + RUBY + end + end + end + + context 'when comparing `Rails.env.to_sym` to a string' do + it 'does not register when `Rails.env.to_sym` is used on LHS' do + expect_no_offenses(<<~RUBY) + Rails.env.to_sym == 'production' + RUBY + end + + it 'does not register when `Rails.env.to_sym` is used on RHS' do + expect_no_offenses(<<~RUBY) + 'production' == Rails.env.to_sym + RUBY + end + end + it 'does not register an offense when using `#good_method`' do expect_no_offenses(<<~RUBY) Rails.env.production?