diff --git a/rubocop/cop/rspec/env_assignment.rb b/rubocop/cop/rspec/env_assignment.rb new file mode 100644 index 00000000000..257454af0e1 --- /dev/null +++ b/rubocop/cop/rspec/env_assignment.rb @@ -0,0 +1,58 @@ +require 'rubocop-rspec' +require_relative '../../spec_helpers' + +module RuboCop + module Cop + module RSpec + # This cop checks for ENV assignment in specs + # + # @example + # + # # bad + # before do + # ENV['FOO'] = 'bar' + # end + # + # # good + # before do + # stub_env('FOO', 'bar') + # end + class EnvAssignment < Cop + include SpecHelpers + + MESSAGE = "Don't assign to ENV, use `stub_env` instead.".freeze + + def_node_search :env_assignment?, <<~PATTERN + (send (const nil? :ENV) :[]= ...) + PATTERN + + # Following is what node.children looks like on a match + # [s(:const, nil, :ENV), :[]=, s(:str, "key"), s(:str, "value")] + def on_send(node) + return unless in_spec?(node) + return unless env_assignment?(node) + + add_offense(node, :expression, MESSAGE) + end + + def autocorrect(node) + lambda do |corrector| + corrector.replace(node.loc.expression, stub_env(env_key(node), env_value(node))) + end + end + + def env_key(node) + node.children[2].source + end + + def env_value(node) + node.children[3].source + end + + def stub_env(key, value) + "stub_env(#{key}, #{value})" + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 1df23899efb..4ebbe010e90 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,11 +1,11 @@ +require_relative 'cop/active_record_dependent' +require_relative 'cop/active_record_serialize' require_relative 'cop/custom_error_class' require_relative 'cop/gem_fetcher' -require_relative 'cop/active_record_serialize' -require_relative 'cop/redirect_with_status' +require_relative 'cop/in_batches' require_relative 'cop/polymorphic_associations' require_relative 'cop/project_path_helper' -require_relative 'cop/active_record_dependent' -require_relative 'cop/in_batches' +require_relative 'cop/redirect_with_status' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_column_with_default_to_large_table' require_relative 'cop/migration/add_concurrent_foreign_key' @@ -13,12 +13,13 @@ require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' require_relative 'cop/migration/add_timestamps' require_relative 'cop/migration/datetime' -require_relative 'cop/migration/safer_boolean_column' require_relative 'cop/migration/hash_index' require_relative 'cop/migration/remove_concurrent_index' require_relative 'cop/migration/remove_index' require_relative 'cop/migration/reversible_add_column_with_default' +require_relative 'cop/migration/safer_boolean_column' require_relative 'cop/migration/timestamps' require_relative 'cop/migration/update_column_in_batches' +require_relative 'cop/rspec/env_assignment' require_relative 'cop/rspec/single_line_hook' require_relative 'cop/rspec/verbose_include_metadata' diff --git a/rubocop/spec_helpers.rb b/rubocop/spec_helpers.rb new file mode 100644 index 00000000000..a702a083958 --- /dev/null +++ b/rubocop/spec_helpers.rb @@ -0,0 +1,12 @@ +module RuboCop + module SpecHelpers + SPEC_HELPERS = %w[spec_helper.rb rails_helper.rb].freeze + + # Returns true if the given node originated from the spec directory. + def in_spec?(node) + path = node.location.expression.source_buffer.name + + !SPEC_HELPERS.include?(File.basename(path)) && path.start_with?(File.join(Dir.pwd, 'spec')) + end + end +end diff --git a/spec/rubocop/cop/rspec/env_assignment_spec.rb b/spec/rubocop/cop/rspec/env_assignment_spec.rb new file mode 100644 index 00000000000..4e859b6f6fa --- /dev/null +++ b/spec/rubocop/cop/rspec/env_assignment_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/rspec/env_assignment' + +describe RuboCop::Cop::RSpec::EnvAssignment do + include CopHelper + + OFFENSE_CALL_SINGLE_QUOTES_KEY = %(ENV['FOO'] = 'bar').freeze + OFFENSE_CALL_DOUBLE_QUOTES_KEY = %(ENV["FOO"] = 'bar').freeze + + let(:source_file) { 'spec/foo_spec.rb' } + + subject(:cop) { described_class.new } + + shared_examples 'an offensive ENV#[]= call' do |content| + it "registers an offense for `#{content}`" do + inspect_source(cop, content, source_file) + + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + expect(cop.highlights).to eq([content]) + end + end + + shared_examples 'an autocorrected ENV#[]= call' do |content, autocorrected_content| + it "registers an offense for `#{content}` and autocorrects it to `#{autocorrected_content}`" do + autocorrected = autocorrect_source(cop, content, source_file) + + expect(autocorrected).to eql(autocorrected_content) + end + end + + context 'in a spec file' do + before do + allow(cop).to receive(:in_spec?).and_return(true) + end + + context 'with a key using single quotes' do + it_behaves_like 'an offensive ENV#[]= call', OFFENSE_CALL_SINGLE_QUOTES_KEY + it_behaves_like 'an autocorrected ENV#[]= call', OFFENSE_CALL_SINGLE_QUOTES_KEY, %(stub_env('FOO', 'bar')) + end + + context 'with a key using double quotes' do + it_behaves_like 'an offensive ENV#[]= call', OFFENSE_CALL_DOUBLE_QUOTES_KEY + it_behaves_like 'an autocorrected ENV#[]= call', OFFENSE_CALL_DOUBLE_QUOTES_KEY, %(stub_env("FOO", 'bar')) + end + end + + context 'outside of a spec file' do + it "does not register an offense for `#{OFFENSE_CALL_SINGLE_QUOTES_KEY}` in a non-spec file" do + inspect_source(cop, OFFENSE_CALL_SINGLE_QUOTES_KEY) + + expect(cop.offenses.size).to eq(0) + end + end +end