Fix some false negatives for Minitest/GlobalExpectations

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### Bug fixes
+
+* [#72](https:/rubocop-hq/rubocop-minitest/pull/72): Fix false negatives for `Minitest/GlobalExpectations`. ([@andrykonchin][])
+
 ## 0.8.0 (2020-03-24)
 
 ### New features

lib/rubocop/cop/minitest/global_expectations.rb

@@ -30,27 +30,31 @@ class GlobalExpectations < Cop
 
  BLOCK_MATCHERS = %i[must_output must_raise must_be_silent must_throw].freeze
 
- MATCHERS_STR = (VALUE_MATCHERS + BLOCK_MATCHERS).map do |m|
+ VALUE_MATCHERS_STR = VALUE_MATCHERS.map do |m|
  ":#{m}"
  end.join(' ').freeze
 
- def_node_matcher :global_expectation?, <<~PATTERN
- (send {
- (send _ _)
- ({lvar ivar cvar gvar} _)
- (send {(send _ _) ({lvar ivar cvar gvar} _)} _ _)
- } {#{MATCHERS_STR}} ...)
+ BLOCK_MATCHERS_STR = BLOCK_MATCHERS.map do |m|
+ ":#{m}"
+ end.join(' ').freeze
+
+ def_node_matcher :value_global_expectation?, <<~PATTERN
+ (send !(send nil? :_ _) {#{VALUE_MATCHERS_STR}} _)
+ PATTERN
+
+ def_node_matcher :block_global_expectation?, <<~PATTERN
+ (send [!(send nil? :_ _) !(block (send nil? :_) _ _)] {#{BLOCK_MATCHERS_STR}} _)
  PATTERN
 
  def on_send(node)
- return unless global_expectation?(node)
+ return unless value_global_expectation?(node) || block_global_expectation?(node)
 
  message = format(MSG, preferred: preferred_receiver(node))
  add_offense(node, location: node.receiver.source_range, message: message)
  end
 
  def autocorrect(node)
- return unless global_expectation?(node)
+ return unless value_global_expectation?(node) || block_global_expectation?(node)
 
  lambda do |corrector|
  receiver = node.receiver.source_range

test/rubocop/cop/minitest/global_expectations_test.rb

@@ -162,6 +162,51 @@ class GlobalExpectationsTest < Minitest::Test
  end
  RUBY
  end
+
+ define_method(:"registers_offense_when_using_global_#{matcher}_for_chained_hash_reference") do
+ assert_offense(<<~RUBY)
+ it 'does something' do
+ options[:a][:b].#{matcher} 0
+ ^^^^^^^^^^^^^^^ Use `_(options[:a][:b])` instead.
+ end
+ RUBY
+
+ assert_correction(<<~RUBY)
+ it 'does something' do
+ _(options[:a][:b]).#{matcher} 0
+ end
+ RUBY
+ end
+
+ define_method(:"registers_offense_when_using_global_#{matcher}_for_method_call_with_params") do
+ assert_offense(<<~RUBY)
+ it 'does something' do
+ foo(a).#{matcher} 0
+ ^^^^^^ Use `_(foo(a))` instead.
+ end
+ RUBY
+
+ assert_correction(<<~RUBY)
+ it 'does something' do
+ _(foo(a)).#{matcher} 0
+ end
+ RUBY
+ end
+
+ define_method(:"registers_offense_when_using_global_#{matcher}_for_constant") do
+ assert_offense(<<~RUBY)
+ it 'does something' do
+ C.#{matcher}(:a)
+ ^ Use `_(C)` instead.
+ end
+ RUBY
+
+ assert_correction(<<~RUBY)
+ it 'does something' do
+ _(C).#{matcher}(:a)
+ end
+ RUBY
+ end
  end
 
  def test_works_with_chained_method_calls