Merge pull request #1503 from r7kamura/feature/freeze-time-around

Add `RSpec/Rails/TravelAround` and `RSpec/RedundantAround` cops

.rubocop.yml

@@ -132,6 +132,8 @@ RSpec/NoExpectationExample:
   Enabled: true
 RSpec/PendingWithoutReason:
   Enabled: true
+RSpec/RedundantAround:
+  Enabled: true
 RSpec/SortMetadata:
   Enabled: true
 RSpec/SubjectDeclaration:

CHANGELOG.md

@@ -7,6 +7,8 @@
 - Fix a false positive for `RSpec/VariableName` when inside non-spec code. ([@ydah])
 - Fix a false positive for `RSpec/VariableDefinition` when inside non-spec code. ([@ydah])
 - Add new `RSpec/PendingBlockInsideExample` cop. ([@ydah])
+- Add `RSpec/RedundantAround` cop. ([@r7kamura])
+- Add `RSpec/Rails/TravelAround` cop. ([@r7kamura])
 
 ## 2.18.1 (2023-01-19)
 

config/default.yml

@@ -712,6 +712,12 @@ RSpec/ReceiveNever:
   VersionAdded: '1.28'
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ReceiveNever
 
+RSpec/RedundantAround:
+  Description: Remove redundant `around` hook.
+  Enabled: pending
+  VersionAdded: "<<next>>"
+  Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/RedundantAround
+
 RSpec/RepeatedDescription:
   Description: Check for repeated description strings in example groups.
   Enabled: true
@@ -1062,3 +1068,10 @@ RSpec/Rails/MinitestAssertions:
   Enabled: pending
   VersionAdded: '2.17'
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/MinitestAssertions
+
+RSpec/Rails/TravelAround:
+  Description: Prefer to travel in `before` rather than `around`.
+  Enabled: pending
+  Safe: false
+  VersionAdded: "<<next>>"
+  Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/TravelAround

docs/modules/ROOT/pages/cops.adoc

@@ -72,6 +72,7 @@
 * xref:cops_rspec.adoc#rspecpredicatematcher[RSpec/PredicateMatcher]
 * xref:cops_rspec.adoc#rspecreceivecounts[RSpec/ReceiveCounts]
 * xref:cops_rspec.adoc#rspecreceivenever[RSpec/ReceiveNever]
+* xref:cops_rspec.adoc#rspecredundantaround[RSpec/RedundantAround]
 * xref:cops_rspec.adoc#rspecrepeateddescription[RSpec/RepeatedDescription]
 * xref:cops_rspec.adoc#rspecrepeatedexample[RSpec/RepeatedExample]
 * xref:cops_rspec.adoc#rspecrepeatedexamplegroupbody[RSpec/RepeatedExampleGroupBody]
@@ -122,5 +123,6 @@
 * xref:cops_rspec_rails.adoc#rspecrails/httpstatus[RSpec/Rails/HttpStatus]
 * xref:cops_rspec_rails.adoc#rspecrails/inferredspectype[RSpec/Rails/InferredSpecType]
 * xref:cops_rspec_rails.adoc#rspecrails/minitestassertions[RSpec/Rails/MinitestAssertions]
+* xref:cops_rspec_rails.adoc#rspecrails/travelaround[RSpec/Rails/TravelAround]
 
 // END_COP_LIST

docs/modules/ROOT/pages/cops_rspec.adoc

@@ -4136,6 +4136,36 @@ expect(foo).not_to receive(:bar)
 
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ReceiveNever
 
+== RSpec/RedundantAround
+
+|===
+| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed
+
+| Pending
+| Yes
+| Yes
+| <<next>>
+| -
+|===
+
+Remove redundant `around` hook.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+around do |example|
+  example.run
+end
+
+# good
+----
+
+=== References
+
+* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/RedundantAround
+
 == RSpec/RepeatedDescription
 
 |===

docs/modules/ROOT/pages/cops_rspec_rails.adoc

@@ -237,3 +237,45 @@ expect(a).not_to eq(b)
 === References
 
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/MinitestAssertions
+
+== RSpec/Rails/TravelAround
+
+|===
+| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed
+
+| Pending
+| No
+| Yes (Unsafe)
+| <<next>>
+| -
+|===
+
+Prefer to travel in `before` rather than `around`.
+
+=== Safety
+
+This cop is unsafe because the automatic `travel_back` is only run
+on test cases that are considered as Rails related.
+
+And also, this cop's autocorrection is unsafe because the order of
+execution will change if other steps exist before traveling in
+`around`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+around do |example|
+  freeze_time do
+    example.run
+  end
+end
+
+# good
+before { freeze_time }
+----
+
+=== References
+
+* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/TravelAround

lib/rubocop/cop/rspec/rails/travel_around.rb

@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      module Rails
+        # Prefer to travel in `before` rather than `around`.
+        #
+        # @safety
+        #   This cop is unsafe because the automatic `travel_back` is only run
+        #   on test cases that are considered as Rails related.
+        #
+        #   And also, this cop's autocorrection is unsafe because the order of
+        #   execution will change if other steps exist before traveling in
+        #   `around`.
+        #
+        # @example
+        #   # bad
+        #   around do |example|
+        #     freeze_time do
+        #       example.run
+        #     end
+        #   end
+        #
+        #   # good
+        #   before { freeze_time }
+        class TravelAround < Base
+          extend AutoCorrector
+
+          MSG = 'Prefer to travel in `before` rather than `around`.'
+
+          TRAVEL_METHOD_NAMES = %i[
+            freeze_time
+            travel
+            travel_to
+          ].to_set.freeze
+
+          # @!method extract_run_in_travel(node)
+          def_node_matcher :extract_run_in_travel, <<~PATTERN
+            (block
+              $(send nil? TRAVEL_METHOD_NAMES ...)
+              (args ...)
+              (send _ :run)
+            )
+          PATTERN
+
+          # @!method match_around_each?(node)
+          def_node_matcher :match_around_each?, <<~PATTERN
+            (block
+              (send _ :around (sym :each)?)
+              ...
+            )
+          PATTERN
+
+          def on_block(node)
+            run_node = extract_run_in_travel(node)
+            return unless run_node
+
+            around_node = extract_surrounding_around_block(run_node)
+            return unless around_node
+
+            add_offense(node) do |corrector|
+              autocorrect(corrector, node, run_node, around_node)
+            end
+          end
+          alias on_numblock on_block
+
+          private
+
+          def autocorrect(corrector, node, run_node, around_node)
+            corrector.replace(
+              node,
+              node.body.source
+            )
+            corrector.insert_before(
+              around_node,
+              "before { #{run_node.source} }\n\n"
+            )
+          end
+
+          # @param node [RuboCop::AST::BlockNode]
+          # @return [RuboCop::AST::BlockNode, nil]
+          def extract_surrounding_around_block(node)
+            node.each_ancestor(:block).find do |ancestor|
+              match_around_each?(ancestor)
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rspec/redundant_around.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Remove redundant `around` hook.
+      #
+      # @example
+      #   # bad
+      #   around do |example|
+      #     example.run
+      #   end
+      #
+      #   # good
+      #
+      class RedundantAround < Base
+        extend AutoCorrector
+
+        MSG = 'Remove redundant `around` hook.'
+
+        RESTRICT_ON_SEND = %i[around].freeze
+
+        def on_block(node)
+          return unless match_redundant_around_hook_block?(node)
+
+          add_offense(node) do |corrector|
+            autocorrect(corrector, node)
+          end
+        end
+        alias on_numblock on_block
+
+        def on_send(node)
+          return unless match_redundant_around_hook_send?(node)
+
+          add_offense(node) do |corrector|
+            autocorrect(corrector, node)
+          end
+        end
+
+        private
+
+        # @!method match_redundant_around_hook_block?(node)
+        def_node_matcher :match_redundant_around_hook_block?, <<~PATTERN
+          (block
+            (send _ :around ...)
+            (args _?)
+            (send _ :run)
+          )
+        PATTERN
+
+        # @!method match_redundant_around_hook_send?(node)
+        def_node_matcher :match_redundant_around_hook_send?, <<~PATTERN
+          (send
+            _
+            :around
+            ...
+            (block-pass
+              (sym :run)
+            )
+          )
+        PATTERN
+
+        def autocorrect(corrector, node)
+          corrector.remove(node)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rspec_cops.rb

@@ -25,6 +25,7 @@
 end
 require_relative 'rspec/rails/inferred_spec_type'
 require_relative 'rspec/rails/minitest_assertions'
+require_relative 'rspec/rails/travel_around'
 
 require_relative 'rspec/align_left_let_brace'
 require_relative 'rspec/align_right_let_brace'
@@ -96,6 +97,7 @@
 require_relative 'rspec/predicate_matcher'
 require_relative 'rspec/receive_counts'
 require_relative 'rspec/receive_never'
+require_relative 'rspec/redundant_around'
 require_relative 'rspec/repeated_description'
 require_relative 'rspec/repeated_example'
 require_relative 'rspec/repeated_example_group_body'