diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e4000483..f840f691c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - 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) diff --git a/config/default.yml b/config/default.yml index 540d1c098..634c883d2 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1068,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: "<>" + Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/TravelAround diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index d86c55c29..04714fa5c 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -123,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 diff --git a/docs/modules/ROOT/pages/cops_rspec_rails.adoc b/docs/modules/ROOT/pages/cops_rspec_rails.adoc index 4cc9bd24b..8d4e11824 100644 --- a/docs/modules/ROOT/pages/cops_rspec_rails.adoc +++ b/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) +| <> +| - +|=== + +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 diff --git a/lib/rubocop/cop/rspec/rails/travel_around.rb b/lib/rubocop/cop/rspec/rails/travel_around.rb new file mode 100644 index 000000000..175879ca2 --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index 1d575ce5c..70519bfd0 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/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' diff --git a/spec/rubocop/cop/rspec/rails/travel_around_spec.rb b/spec/rubocop/cop/rspec/rails/travel_around_spec.rb new file mode 100644 index 000000000..ae1c2944a --- /dev/null +++ b/spec/rubocop/cop/rspec/rails/travel_around_spec.rb @@ -0,0 +1,157 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::Rails::TravelAround do + context 'with `freeze_time` in `before`' do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + before { freeze_time } + RUBY + end + end + + context 'with `freeze_time` in `around(:all)`' do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + around(:all) do |example| + freeze_time do + example.run + end + end + RUBY + end + end + + context 'with `freeze_time` in `around(:suite)`' do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + around(:suite) do |example| + freeze_time do + example.run + end + end + RUBY + end + end + + context 'with another step in `freeze_time`' do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + around do |example| + freeze_time do + do_some_preparation + example.run + end + end + RUBY + end + end + + context 'with `freeze_time` in `around`' do + it 'registers offense' do + expect_offense(<<~RUBY) + around do |example| + freeze_time do + ^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`. + example.run + end + end + RUBY + + expect_correction(<<~RUBY) + before { freeze_time } + + around do |example| + example.run + end + RUBY + end + end + + context 'with `freeze_time` in `around(:each)`' do + it 'registers offense' do + expect_offense(<<~RUBY) + around(:each) do |example| + freeze_time do + ^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`. + example.run + end + end + RUBY + + expect_correction(<<~RUBY) + before { freeze_time } + + around(:each) do |example| + example.run + end + RUBY + end + end + + context 'with `freeze_time` and another node in `around`' do + it 'registers offense' do + expect_offense(<<~RUBY) + around do |example| + foo + + freeze_time do + ^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`. + example.run + end + end + RUBY + + expect_correction(<<~RUBY) + before { freeze_time } + + around do |example| + foo + + example.run + end + RUBY + end + end + + context 'with `travel` in `around`' do + it 'registers offense' do + expect_offense(<<~RUBY) + around do |example| + travel(duration) do + ^^^^^^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`. + example.run + end + end + RUBY + + expect_correction(<<~RUBY) + before { travel(duration) } + + around do |example| + example.run + end + RUBY + end + end + + context 'with `travel_to` in `around`' do + it 'registers offense' do + expect_offense(<<~RUBY) + around do |example| + travel_to(time) do + ^^^^^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`. + example.run + end + end + RUBY + + expect_correction(<<~RUBY) + before { travel_to(time) } + + around do |example| + example.run + end + RUBY + end + end +end