diff --git a/CHANGELOG.md b/CHANGELOG.md index 56bb2d8c..1b8eddde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Master (Unreleased) +- Add new `RSpecRails/SettingsOverwritten` cop. ([@ydah]) + ## 2.28.2 (2024-03-31) - Fix a `NameError` by Cross-Referencing. ([@ydah]) diff --git a/config/default.yml b/config/default.yml index bc3fe0e5..0697b306 100644 --- a/config/default.yml +++ b/config/default.yml @@ -75,6 +75,12 @@ RSpecRails/NegationBeValid: VersionAdded: '2.23' Reference: https://www.rubydoc.info/gems/rubocop-rspec_rails/RuboCop/Cop/RSpecRails/NegationBeValid +RSpecRails/SettingsOverwritten: + Description: Do not overwrite Settings directly, use mock instead. + Enabled: pending + VersionAdded: "<>" + Reference: https://www.rubydoc.info/gems/rubocop-rspec_rails/RuboCop/Cop/RSpecRails/SettingsOverwritten + RSpecRails/TravelAround: Description: Prefer to travel in `before` rather than `around`. Enabled: pending diff --git a/lib/rubocop/cop/rspec_rails/settings_overwritten.rb b/lib/rubocop/cop/rspec_rails/settings_overwritten.rb new file mode 100644 index 00000000..f1800aca --- /dev/null +++ b/lib/rubocop/cop/rspec_rails/settings_overwritten.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpecRails + # Do not overwrite Settings directly, use mock instead. + # + # @example + # # bad + # Settings.foo = 'bar' + # Settings[:foo] = 'bar' + # Settings.foo[:bar] = 'baz' + # Settings.foo += 'bar' + # Settings.foo << 'bar' + # + # # good + # allow(Settings).to receive(:foo).and_return('bar') + # allow(Settings).to receive(:[]).with(:foo).and_return('bar') + # allow(Settings.foo).to receive(:[]).with(:bar).and_return('baz') + # allow(Settings).to receive(:foo).and_return(Settings.foo + "bar") + # + class SettingsOverwritten < ::RuboCop::Cop::Base + MSG = 'Do not overwrite Settings directly, use mock instead.' + + # @!method settings?(node) + def_node_matcher :settings?, <<~PATTERN + (const nil? :Settings) + PATTERN + + def on_const(node) + return unless settings?(node) + return unless (assignment = first_assignment(node)) + + add_offense(assignment) + end + + private + + def first_assignment(node) + node.each_ancestor.find(&:assignment_or_similar?) + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_rails_cops.rb b/lib/rubocop/cop/rspec_rails_cops.rb index 1a101b34..96b0f87a 100644 --- a/lib/rubocop/cop/rspec_rails_cops.rb +++ b/lib/rubocop/cop/rspec_rails_cops.rb @@ -6,4 +6,5 @@ require_relative 'rspec_rails/inferred_spec_type' require_relative 'rspec_rails/minitest_assertions' require_relative 'rspec_rails/negation_be_valid' +require_relative 'rspec_rails/settings_overwritten' require_relative 'rspec_rails/travel_around' diff --git a/spec/rubocop/cop/rspec_rails/settings_overwritten_spec.rb b/spec/rubocop/cop/rspec_rails/settings_overwritten_spec.rb new file mode 100644 index 00000000..c5f73416 --- /dev/null +++ b/spec/rubocop/cop/rspec_rails/settings_overwritten_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpecRails::SettingsOverwritten, :config do + it 'registers an offense when using `Settings.foo = "bar"`' do + expect_offense(<<~RUBY) + Settings.foo = "bar" + ^^^^^^^^^^^^^^^^^^^^ Do not overwrite Settings directly, use mock instead. + RUBY + end + + it 'registers an offense when using `Settings[:foo] = "bar"`' do + expect_offense(<<~RUBY) + Settings[:foo] = "bar" + ^^^^^^^^^^^^^^^^^^^^^^ Do not overwrite Settings directly, use mock instead. + RUBY + end + + it 'registers an offense when using `Settings.foo[:bar] = "baz"`' do + expect_offense(<<~RUBY) + Settings.foo[:bar] = "baz" + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not overwrite Settings directly, use mock instead. + RUBY + end + + it 'registers an offense when using `Settings.foo += "bar"`' do + expect_offense(<<~RUBY) + Settings.foo += "bar" + ^^^^^^^^^^^^^^^^^^^^^ Do not overwrite Settings directly, use mock instead. + RUBY + end + + it 'registers an offense when using `Settings.foo << "bar"`' do + expect_offense(<<~RUBY) + Settings.foo << "bar" + ^^^^^^^^^^^^^^^^^^^^^ Do not overwrite Settings directly, use mock instead. + RUBY + end + + it 'does not register an offense when using mock' do + expect_no_offenses(<<~RUBY) + allow(Settings).to receive(:foo).and_return("bar") + RUBY + end +end