Merge pull request #148 from gi/issue-147/global-expectations/preferr…

…ed-method [Fix #147] Minitest/GlobalExpectations: add EnforcedStyle config value
rubocop · Nov 2, 2021 · 4df4201 · 4df4201
2 parents a52edd3 + be775ec
commit 4df4201
Show file tree

Hide file tree

Showing 6 changed files with 472 additions and 108 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -10,6 +10,8 @@
 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods.
 Metrics/MethodLength:
   Max: 14
+  Exclude:
+    - 'test/rubocop/cop/minitest/global_expectations_test.rb'
 
 # Offense count: 1
 Style/DocumentDynamicEvalDefinition:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### New features
+
+* [#147](https://github.com/rubocop/rubocop-minitest/issues/147): Add `EnforcedStyle` config parameter for `Minitest/GlobalExpectations`. ([@gi][])
+
 ### Bug fixes
 
 * [#142](https://github.com/rubocop/rubocop-minitest/issues/142): Fix `Minitest/GlobalExpectations` autocorrect when receiver is lambda. ([@gi][])

diff --git a/config/default.yml b/config/default.yml
@@ -103,7 +103,19 @@ Minitest/GlobalExpectations:
   Description: 'This cop checks for deprecated global expectations.'
   StyleGuide: 'https://minitest.rubystyle.guide#global-expectations'
   Enabled: true
+  EnforcedStyle: any
+  Include:
+    - '**/test/**/*'
+    - '**/*_test.rb'
+    - '**/spec/**/*'
+    - '**/*_spec.rb'
+  SupportedStyles:
+    - _
+    - any
+    - expect
+    - value
   VersionAdded: '0.7'
+  VersionChanged: '0.16'
 
 Minitest/LiteralAsActualArgument:
   Description: 'This cop enforces correct order of `expected` and `actual` arguments for `assert_equal`.'

diff --git a/docs/modules/ROOT/pages/cops_minitest.adoc b/docs/modules/ROOT/pages/cops_minitest.adoc
@@ -519,27 +519,120 @@ end
 | Yes
 | Yes
 | 0.7
-| -
+| 0.16
 |===
 
 This cop checks for deprecated global expectations
 and autocorrects them to use expect format.
 
 === Examples
 
+==== EnforcedStyle: _
+
+[source,ruby]
+----
+# bad
+musts.must_equal expected_musts
+wonts.wont_match expected_wonts
+musts.must_raise TypeError
+
+expect(musts).must_equal expected_musts
+expect(wonts).wont_match expected_wonts
+expect { musts }.must_raise TypeError
+
+value(musts).must_equal expected_musts
+value(wonts).wont_match expected_wonts
+value { musts }.must_raise TypeError
+
+# good
+_(musts).must_equal expected_musts
+_(wonts).wont_match expected_wonts
+_ { musts }.must_raise TypeError
+----
+
+==== EnforcedStyle: any (default)
+
+[source,ruby]
+----
+# bad
+musts.must_equal expected_musts
+wonts.wont_match expected_wonts
+musts.must_raise TypeError
+
+# good
+_(musts).must_equal expected_musts
+_(wonts).wont_match expected_wonts
+_ { musts }.must_raise TypeError
+
+expect(musts).must_equal expected_musts
+expect(wonts).wont_match expected_wonts
+expect { musts }.must_raise TypeError
+
+value(musts).must_equal expected_musts
+value(wonts).wont_match expected_wonts
+value { musts }.must_raise TypeError
+----
+
+==== EnforcedStyle: expect
+
 [source,ruby]
 ----
 # bad
 musts.must_equal expected_musts
 wonts.wont_match expected_wonts
 musts.must_raise TypeError
 
+_(musts).must_equal expected_musts
+_(wonts).wont_match expected_wonts
+_ { musts }.must_raise TypeError
+
+value(musts).must_equal expected_musts
+value(wonts).wont_match expected_wonts
+value { musts }.must_raise TypeError
+
 # good
+expect(musts).must_equal expected_musts
+expect(wonts).wont_match expected_wonts
+expect { musts }.must_raise TypeError
+----
+
+==== EnforcedStyle: value
+
+[source,ruby]
+----
+# bad
+musts.must_equal expected_musts
+wonts.wont_match expected_wonts
+musts.must_raise TypeError
+
 _(musts).must_equal expected_musts
 _(wonts).wont_match expected_wonts
 _ { musts }.must_raise TypeError
+
+expect(musts).must_equal expected_musts
+expect(wonts).wont_match expected_wonts
+expect { musts }.must_raise TypeError
+
+# good
+value(musts).must_equal expected_musts
+value(wonts).wont_match expected_wonts
+value { musts }.must_raise TypeError
 ----
 
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| EnforcedStyle
+| `any`
+| `_`, `any`, `expect`, `value`
+
+| Include
+| `+**/test/**/*+`, `+**/*_test.rb+`, `+**/spec/**/*+`, `+**/*_spec.rb+`
+| Array
+|===
+
 === References
 
 * https://minitest.rubystyle.guide#global-expectations

diff --git a/lib/rubocop/cop/minitest/global_expectations.rb b/lib/rubocop/cop/minitest/global_expectations.rb
@@ -6,7 +6,26 @@ module Minitest
       # This cop checks for deprecated global expectations
       # and autocorrects them to use expect format.
       #
-      # @example
+      # @example EnforcedStyle: _
+      #   # bad
+      #   musts.must_equal expected_musts
+      #   wonts.wont_match expected_wonts
+      #   musts.must_raise TypeError
+      #
+      #   expect(musts).must_equal expected_musts
+      #   expect(wonts).wont_match expected_wonts
+      #   expect { musts }.must_raise TypeError
+      #
+      #   value(musts).must_equal expected_musts
+      #   value(wonts).wont_match expected_wonts
+      #   value { musts }.must_raise TypeError
+      #
+      #   # good
+      #   _(musts).must_equal expected_musts
+      #   _(wonts).wont_match expected_wonts
+      #   _ { musts }.must_raise TypeError
+      #
+      # @example EnforcedStyle: any (default)
       #   # bad
       #   musts.must_equal expected_musts
       #   wonts.wont_match expected_wonts
@@ -16,7 +35,54 @@ module Minitest
       #   _(musts).must_equal expected_musts
       #   _(wonts).wont_match expected_wonts
       #   _ { musts }.must_raise TypeError
+      #
+      #   expect(musts).must_equal expected_musts
+      #   expect(wonts).wont_match expected_wonts
+      #   expect { musts }.must_raise TypeError
+      #
+      #   value(musts).must_equal expected_musts
+      #   value(wonts).wont_match expected_wonts
+      #   value { musts }.must_raise TypeError
+      #
+      # @example EnforcedStyle: expect
+      #   # bad
+      #   musts.must_equal expected_musts
+      #   wonts.wont_match expected_wonts
+      #   musts.must_raise TypeError
+      #
+      #   _(musts).must_equal expected_musts
+      #   _(wonts).wont_match expected_wonts
+      #   _ { musts }.must_raise TypeError
+      #
+      #   value(musts).must_equal expected_musts
+      #   value(wonts).wont_match expected_wonts
+      #   value { musts }.must_raise TypeError
+      #
+      #   # good
+      #   expect(musts).must_equal expected_musts
+      #   expect(wonts).wont_match expected_wonts
+      #   expect { musts }.must_raise TypeError
+      #
+      # @example EnforcedStyle: value
+      #   # bad
+      #   musts.must_equal expected_musts
+      #   wonts.wont_match expected_wonts
+      #   musts.must_raise TypeError
+      #
+      #   _(musts).must_equal expected_musts
+      #   _(wonts).wont_match expected_wonts
+      #   _ { musts }.must_raise TypeError
+      #
+      #   expect(musts).must_equal expected_musts
+      #   expect(wonts).wont_match expected_wonts
+      #   expect { musts }.must_raise TypeError
+      #
+      #   # good
+      #   value(musts).must_equal expected_musts
+      #   value(wonts).wont_match expected_wonts
+      #   value { musts }.must_raise TypeError
       class GlobalExpectations < Base
+        include ConfigurableEnforcedStyle
         extend AutoCorrector
 
         MSG = 'Use `%<preferred>s` instead.'
@@ -34,50 +100,35 @@ class GlobalExpectations < Base
 
         RESTRICT_ON_SEND = VALUE_MATCHERS + BLOCK_MATCHERS
 
-        VALUE_MATCHERS_STR = VALUE_MATCHERS.map do |m|
-          ":#{m}"
-        end.join(' ').freeze
-
-        BLOCK_MATCHERS_STR = BLOCK_MATCHERS.map do |m|
-          ":#{m}"
-        end.join(' ').freeze
-
         # There are aliases for the `_` method - `expect` and `value`
-        DSL_METHODS_LIST = %w[_ value expect].map do |n|
-          ":#{n}"
-        end.join(' ').freeze
-
-        def_node_matcher :value_global_expectation?, <<~PATTERN
-          (send !(send nil? {#{DSL_METHODS_LIST}} _) {#{VALUE_MATCHERS_STR}} ...)
-        PATTERN
-
-        def_node_matcher :block_global_expectation?, <<~PATTERN
-          (send
-            [
-              !(send nil? {#{DSL_METHODS_LIST}} _)
-              !(block (send nil? {#{DSL_METHODS_LIST}}) _ _)
-            ]
-            {#{BLOCK_MATCHERS_STR}}
-            _
-          )
-        PATTERN
+        DSL_METHODS = %i[_ expect value].freeze
 
         def on_send(node)
-          return unless value_global_expectation?(node) || block_global_expectation?(node)
+          receiver = node.receiver
+          return unless receiver
 
-          message = format(MSG, preferred: preferred_receiver(node))
+          method = block_receiver?(receiver) || value_receiver?(receiver)
+          return if method == preferred_method || (method && style == :any)
 
-          add_offense(node.receiver, message: message) do |corrector|
-            receiver = node.receiver
-            replacement = preferred_receiver(node)
-            corrector.replace(receiver, replacement)
-          end
+          register_offense(node, method)
         end
 
         private
 
+        def_node_matcher :block_receiver?, <<~PATTERN
+          (block (send nil? $#method_allowed?) _ _)
+        PATTERN
+
+        def_node_matcher :value_receiver?, <<~PATTERN
+          (send nil? $#method_allowed? _)
+        PATTERN
+
+        def method_allowed?(method)
+          DSL_METHODS.include?(method)
+        end
+
         def preferred_method
-          :_
+          style == :any ? :_ : style
         end
 
         def preferred_receiver(node)
@@ -90,6 +141,24 @@ def preferred_receiver(node)
             "#{preferred_method}(#{receiver.source})"
           end
         end
+
+        def register_offense(node, method)
+          receiver = node.receiver
+
+          if method
+            preferred = preferred_method
+            replacement = receiver.source.sub(method.to_s, preferred_method.to_s)
+          else
+            preferred = preferred_receiver(node)
+            replacement = preferred
+          end
+
+          message = format(MSG, preferred: preferred)
+
+          add_offense(receiver, message: message) do |corrector|
+            corrector.replace(receiver, replacement)
+          end
+        end
       end
     end
   end