From 5b981de5e951699bba574fe5f5c09a2a65dac2b8 Mon Sep 17 00:00:00 2001 From: r7kamura Date: Fri, 2 Dec 2022 09:19:59 +0900 Subject: [PATCH] Add `RSpec/Rails/TravelAround` cop --- CHANGELOG.md | 1 + config/default.yml | 7 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rspec_rails.adoc | 42 ++++++++ lib/rubocop/cop/rspec/rails/travel_around.rb | 81 +++++++++++++++ lib/rubocop/cop/rspec_cops.rb | 1 + .../cop/rspec/rails/travel_around_spec.rb | 99 +++++++++++++++++++ 7 files changed, 232 insertions(+) create mode 100644 lib/rubocop/cop/rspec/rails/travel_around.rb create mode 100644 spec/rubocop/cop/rspec/rails/travel_around_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fd401cb51..3bdba72563 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Mark `RSpec/BeEql` as `Safe: false`. ([@r7kamura]) - Add `RSpec/PendingWithoutReason` cop. ([@r7kamura]) - Add `RSpec/RedundantAround` cop. ([@r7kamura]) +- Add `RSpec/Rails/TravelAround` cop. ([@r7kamura]) ## 2.15.0 (2022-11-03) diff --git a/config/default.yml b/config/default.yml index 56245715ef..3b76c9cbb6 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1050,3 +1050,10 @@ RSpec/Rails/InferredSpecType: routing: routing system: system views: view + +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 861501db14..8bcf520fb6 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -120,5 +120,6 @@ * xref:cops_rspec_rails.adoc#rspecrails/havehttpstatus[RSpec/Rails/HaveHttpStatus] * 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/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 e19483188e..26e437e3bc 100644 --- a/docs/modules/ROOT/pages/cops_rspec_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rspec_rails.adoc @@ -204,3 +204,45 @@ end === References * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/InferredSpecType + +== 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 0000000000..b5cb3d7910 --- /dev/null +++ b/lib/rubocop/cop/rspec/rails/travel_around.rb @@ -0,0 +1,81 @@ +# 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 + + def on_block(node) + send_node = match_run_in_travel(node) + return unless send_node + + add_offense(node) do |corrector| + autocorrect(corrector, node, send_node) + end + end + alias on_numblock on_block + + private + + # @!method match_run_in_travel(node) + def_node_matcher :match_run_in_travel, <<~PATTERN + (block + $(send nil? TRAVEL_METHOD_NAMES ...) + (args ...) + (send _ :run) + ) + PATTERN + + def autocorrect(corrector, node, send_node) + corrector.replace( + node, + node.body.source + ) + corrector.insert_before( + extract_surrounding_around_block(node), + "before { #{send_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| + ancestor.method?(:around) + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index a5ae1eb421..3569a71bb3 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -23,6 +23,7 @@ # Rails/HttpStatus cannot be loaded if rack/utils is unavailable. end require_relative 'rspec/rails/inferred_spec_type' +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 0000000000..4915049c69 --- /dev/null +++ b/spec/rubocop/cop/rspec/rails/travel_around_spec.rb @@ -0,0 +1,99 @@ +# 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`' 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` 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