From e3fa25b6aed8b7339b5c00b41643ea6b3c7d5228 Mon Sep 17 00:00:00 2001 From: Ali Ismayilov <993934+aliismayilov@users.noreply.github.com> Date: Mon, 17 Feb 2025 15:01:46 +0100 Subject: [PATCH] Add new cop `BangPersistence` --- CHANGELOG.md | 2 + config/default.yml | 7 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rspecrails.adoc | 45 +++++++++++++ .../cop/rspec_rails/bang_persistence.rb | 67 +++++++++++++++++++ lib/rubocop/cop/rspec_rails_cops.rb | 1 + .../cop/rspec_rails/bang_persistence_spec.rb | 49 ++++++++++++++ 7 files changed, 172 insertions(+) create mode 100644 lib/rubocop/cop/rspec_rails/bang_persistence.rb create mode 100644 spec/rubocop/cop/rspec_rails/bang_persistence_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 661b9868..9dcd2421 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Handle unknown HTTP status codes for `RSpecRails/HttpStatus` cop. ([@viralpraxis]) - Fix a false negative for `RSpecRails/TravelAround` cop when passed as a proc to a travel method. ([@ydah]) +- Introduce new cop `RSpecRails/BangPersistence` cop. ([@aliismayilov]) ## 2.30.0 (2024-06-12) @@ -72,6 +73,7 @@ [@akiomik]: https://github.com/akiomik +[@aliismayilov]: https://github.com/aliismayilov [@anthony-robin]: https://github.com/anthony-robin [@bquorning]: https://github.com/bquorning [@corydiamand]: https://github.com/corydiamand diff --git a/config/default.yml b/config/default.yml index 51ec8889..5f528987 100644 --- a/config/default.yml +++ b/config/default.yml @@ -12,6 +12,13 @@ RSpecRails/AvoidSetupHook: VersionAdded: '2.4' Reference: https://www.rubydoc.info/gems/rubocop-rspec_rails/RuboCop/Cop/RSpecRails/AvoidSetupHook +RSpecRails/BangPersistence: + Description: Prefer bang methods for persistence. + Enabled: pending + SafeAutoCorrect: false + VersionAdded: "<>" + Reference: https://www.rubydoc.info/gems/rubocop-rspec_rails/RuboCop/Cop/RSpecRails/BangPersistence + RSpecRails/HaveHttpStatus: Description: Checks that tests use `have_http_status` instead of equality matchers. Enabled: pending diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index b4fc11c6..63f7c9c1 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -3,6 +3,7 @@ === Department xref:cops_rspecrails.adoc[RSpecRails] * xref:cops_rspecrails.adoc#rspecrailsavoidsetuphook[RSpecRails/AvoidSetupHook] +* xref:cops_rspecrails.adoc#rspecrailsbangpersistence[RSpecRails/BangPersistence] * xref:cops_rspecrails.adoc#rspecrailshavehttpstatus[RSpecRails/HaveHttpStatus] * xref:cops_rspecrails.adoc#rspecrailshttpstatus[RSpecRails/HttpStatus] * xref:cops_rspecrails.adoc#rspecrailsinferredspectype[RSpecRails/InferredSpecType] diff --git a/docs/modules/ROOT/pages/cops_rspecrails.adoc b/docs/modules/ROOT/pages/cops_rspecrails.adoc index 07650f96..3d6b3f19 100644 --- a/docs/modules/ROOT/pages/cops_rspecrails.adoc +++ b/docs/modules/ROOT/pages/cops_rspecrails.adoc @@ -42,6 +42,51 @@ end * https://www.rubydoc.info/gems/rubocop-rspec_rails/RuboCop/Cop/RSpecRails/AvoidSetupHook +[#rspecrailsbangpersistence] +== RSpecRails/BangPersistence + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| Always (Unsafe) +| <> +| - +|=== + +Prefer bang methods for persistence. + +[#safety-rspecrailsbangpersistence] +=== Safety + +This cop is unsafe because by replacing non-bang versions +of the persistence methods, you might: +1. Catch persistence failures. This is what you want to catch. +2. Might replace wrong object calls. + The cop is unaware if the object is ActiveRecord instance + +[#examples-rspecrailsbangpersistence] +=== Examples + +[source,ruby] +---- +# bad +before do + post.update(title: "new title") +end + +# good +before do + post.update!(type: "new title") +end +---- + +[#references-rspecrailsbangpersistence] +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec_rails/RuboCop/Cop/RSpecRails/BangPersistence + [#rspecrailshavehttpstatus] == RSpecRails/HaveHttpStatus diff --git a/lib/rubocop/cop/rspec_rails/bang_persistence.rb b/lib/rubocop/cop/rspec_rails/bang_persistence.rb new file mode 100644 index 00000000..61fba6de --- /dev/null +++ b/lib/rubocop/cop/rspec_rails/bang_persistence.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpecRails + # Prefer bang methods for persistence. + # + # @safety + # This cop is unsafe because by replacing non-bang versions + # of the persistence methods, you might: + # 1. Catch persistence failures. This is what you want to catch. + # 2. Might replace wrong object calls. + # The cop is unaware if the object is ActiveRecord instance + # + # + # @example + # # bad + # before do + # post.update(title: "new title") + # end + # + # # good + # before do + # post.update!(type: "new title") + # end + # + class BangPersistence < ::RuboCop::Cop::Base + extend AutoCorrector + + MSG = 'Prefer bang versions of the persistence methods.' + + PERSISTENCE_METHODS = %i[ + create + destroy + save + update + update_attribute + ].to_set.freeze + + # Match `before` blocks + # @!method before_block?(node) + def_node_matcher :before_block?, <<-PATTERN + (block (send nil? :before) ...) + PATTERN + + # Match `save` and `update` method calls + # @!method peristence_calls(node) + def_node_search :peristence_calls, <<-PATTERN + (send _ {#{PERSISTENCE_METHODS.map { |method| ":#{method}" }.join(' ')}} ...) + PATTERN + + def on_block(node) + return unless before_block?(node) + + peristence_calls(node) do |method_call| + add_offense(method_call) do |corrector| + corrector.replace(method_call.loc.selector, + "#{method_call.method_name}!") + end + end + end + + alias on_numblock on_block + end + end + end +end diff --git a/lib/rubocop/cop/rspec_rails_cops.rb b/lib/rubocop/cop/rspec_rails_cops.rb index 1a101b34..8c327f4c 100644 --- a/lib/rubocop/cop/rspec_rails_cops.rb +++ b/lib/rubocop/cop/rspec_rails_cops.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative 'rspec_rails/avoid_setup_hook' +require_relative 'rspec_rails/bang_persistence' require_relative 'rspec_rails/have_http_status' require_relative 'rspec_rails/http_status' require_relative 'rspec_rails/inferred_spec_type' diff --git a/spec/rubocop/cop/rspec_rails/bang_persistence_spec.rb b/spec/rubocop/cop/rspec_rails/bang_persistence_spec.rb new file mode 100644 index 00000000..27696efb --- /dev/null +++ b/spec/rubocop/cop/rspec_rails/bang_persistence_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpecRails::BangPersistence do + context 'with `save!` in `before`' do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + before { post.save! } + RUBY + end + end + + context 'with `update!` in `before`' do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + before { post.update!(title: "new title") } + RUBY + end + end + + context 'with `save` in `before`' do + it 'registers offense' do + expect_offense(<<~RUBY) + before { post.save } + ^^^^^^^^^ Prefer bang versions of the persistence methods. + RUBY + + expect_correction(<<~RUBY) + before { post.save! } + RUBY + end + end + + context 'with `update_attribute` in `before`' do + it 'registers offense' do + expect_offense(<<~RUBY) + before do + post.update_attribute(title: "new title") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer bang versions of the persistence methods. + end + RUBY + + expect_correction(<<~RUBY) + before do + post.update_attribute!(title: "new title") + end + RUBY + end + end +end