Merge pull request #1372 from r7kamura/feature/implicit

Add `require_implicit` style to `RSpec/ImplicitSubject`

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Master (Unreleased)
 
+* Add `require_implicit` style to `RSpec/ImplicitSubject`. ([@r7kamura][])
+
 ## 2.13.2 (2022-09-23)
 
 * Fix an error for `RSpec/Capybara/SpecificFinders` with no parentheses. ([@ydah][])

config/default.yml

@@ -492,8 +492,9 @@ RSpec/ImplicitSubject:
     - single_line_only
     - single_statement_only
     - disallow
+    - require_implicit
   VersionAdded: '1.29'
-  VersionChanged: '1.30'
+  VersionChanged: '2.13'
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ImplicitSubject
 
 RSpec/InstanceSpy:

docs/modules/ROOT/pages/cops_rspec.adoc

@@ -2232,7 +2232,7 @@ it { should be_truthy }
 | Yes
 | Yes
 | 1.29
-| 1.30
+| 2.13
 |===
 
 Checks for usage of implicit subject (`is_expected` / `should`).
@@ -2288,14 +2288,38 @@ it { is_expected.to be_truthy }
 it { expect(subject).to be_truthy }
 ----
 
+==== `EnforcedStyle: require_implicit`
+
+[source,ruby]
+----
+# bad
+it { expect(subject).to be_truthy }
+
+# good
+it { is_expected.to be_truthy }
+
+# bad
+it do
+  expect(subject).to be_truthy
+end
+
+# good
+it do
+  is_expected.to be_truthy
+end
+
+# good
+it { expect(named_subject).to be_truthy }
+----
+
 === Configurable attributes
 
 |===
 | Name | Default value | Configurable values
 
 | EnforcedStyle
 | `single_line_only`
-| `single_line_only`, `single_statement_only`, `disallow`
+| `single_line_only`, `single_statement_only`, `disallow`, `require_implicit`
 |===
 
 === References

lib/rubocop/cop/rspec/implicit_subject.rb

@@ -42,21 +42,53 @@ module RSpec
       #   # good
       #   it { expect(subject).to be_truthy }
       #
+      # @example `EnforcedStyle: require_implicit`
+      #   # bad
+      #   it { expect(subject).to be_truthy }
+      #
+      #   # good
+      #   it { is_expected.to be_truthy }
+      #
+      #   # bad
+      #   it do
+      #     expect(subject).to be_truthy
+      #   end
+      #
+      #   # good
+      #   it do
+      #     is_expected.to be_truthy
+      #   end
+      #
+      #   # good
+      #   it { expect(named_subject).to be_truthy }
+      #
       class ImplicitSubject < Base
         extend AutoCorrector
         include ConfigurableEnforcedStyle
 
-        MSG = "Don't use implicit subject."
-        RESTRICT_ON_SEND = %i[is_expected should should_not].freeze
+        MSG_REQUIRE_EXPLICIT = "Don't use implicit subject."
+
+        MSG_REQUIRE_IMPLICIT = "Don't use explicit subject."
+
+        RESTRICT_ON_SEND = %i[
+          expect
+          is_expected
+          should
+          should_not
+        ].freeze
+
+        # @!method explicit_unnamed_subject?(node)
+        def_node_matcher :explicit_unnamed_subject?, <<-PATTERN
+          (send nil? :expect (send nil? :subject))
+        PATTERN
 
         # @!method implicit_subject?(node)
         def_node_matcher :implicit_subject?, <<-PATTERN
           (send nil? {:should :should_not :is_expected} ...)
         PATTERN
 
         def on_send(node)
-          return unless implicit_subject?(node)
-          return if valid_usage?(node)
+          return unless invalid?(node)
 
           add_offense(node) do |corrector|
             autocorrect(corrector, node)
@@ -66,32 +98,67 @@ def on_send(node)
         private
 
         def autocorrect(corrector, node)
-          replacement = 'expect(subject)'
           case node.method_name
+          when :expect
+            corrector.replace(node, 'is_expected')
+          when :is_expected
+            corrector.replace(node.location.selector, 'expect(subject)')
           when :should
-            replacement += '.to'
+            corrector.replace(node.location.selector, 'expect(subject).to')
           when :should_not
-            replacement += '.not_to'
+            corrector.replace(node.location.selector, 'expect(subject).not_to')
           end
-
-          corrector.replace(node.loc.selector, replacement)
         end
 
-        def valid_usage?(node)
-          example = node.ancestors.find { |parent| example?(parent) }
-          return false if example.nil?
-
-          example.method?(:its) || allowed_by_style?(example)
+        def message(_node)
+          case style
+          when :require_implicit
+            MSG_REQUIRE_IMPLICIT
+          else
+            MSG_REQUIRE_EXPLICIT
+          end
         end
 
-        def allowed_by_style?(example)
+        def invalid?(node)
           case style
+          when :require_implicit
+            explicit_unnamed_subject?(node)
+          when :disallow
+            implicit_subject_in_non_its?(node)
           when :single_line_only
-            example.single_line?
+            implicit_subject_in_non_its_and_non_single_line?(node)
           when :single_statement_only
-            !example.body.begin_type?
-          else
-            false
+            implicit_subject_in_non_its_and_non_single_statement?(node)
+          end
+        end
+
+        def implicit_subject_in_non_its?(node)
+          implicit_subject?(node) && !its?(node)
+        end
+
+        def implicit_subject_in_non_its_and_non_single_line?(node)
+          implicit_subject_in_non_its?(node) && !single_line?(node)
+        end
+
+        def implicit_subject_in_non_its_and_non_single_statement?(node)
+          implicit_subject_in_non_its?(node) && !single_statement?(node)
+        end
+
+        def its?(node)
+          example_of(node)&.method?(:its)
+        end
+
+        def single_line?(node)
+          example_of(node)&.single_line?
+        end
+
+        def single_statement?(node)
+          !example_of(node)&.body&.begin_type?
+        end
+
+        def example_of(node)
+          node.each_ancestor.find do |ancestor|
+            example?(ancestor)
           end
         end
       end

spec/rubocop/cop/rspec/implicit_subject_spec.rb

@@ -183,4 +183,72 @@ def permits(actions)
       RUBY
     end
   end
+
+  context 'with EnforcedStyle `require_implicit`' do
+    let(:enforced_style) { 'require_implicit' }
+
+    context 'with `is_expected`' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<-RUBY)
+          it { is_expected.to be_good }
+        RUBY
+      end
+    end
+
+    context 'with `expect { subject }`' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<-RUBY)
+          it { expect { subject }.to change(goodness, :count) }
+        RUBY
+      end
+    end
+
+    context 'with `its`' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<-RUBY)
+          its(:quality) { is_expected.to be(:high) }
+        RUBY
+      end
+    end
+
+    context 'with named subject' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<-RUBY)
+          subject(:instance) { described_class.new }
+
+          it { expect(instance).to be_good }
+        RUBY
+      end
+    end
+
+    context 'with `expect(subject)` in one-line' do
+      it 'registers and autocorrects an offense' do
+        expect_offense(<<-RUBY)
+          it { expect(subject).to be_good }
+               ^^^^^^^^^^^^^^^ Don't use explicit subject.
+        RUBY
+
+        expect_correction(<<-RUBY)
+          it { is_expected.to be_good }
+        RUBY
+      end
+    end
+
+    context 'with `expect(subject)` in multi-lines' do
+      it 'registers and autocorrects an offense' do
+        expect_offense(<<-RUBY)
+          it do
+            expect(subject).to be_good
+            ^^^^^^^^^^^^^^^ Don't use explicit subject.
+          end
+        RUBY
+
+        expect_correction(<<-RUBY)
+          it do
+            is_expected.to be_good
+          end
+        RUBY
+      end
+    end
+  end
 end