Merge pull request #69 from koic/fix_false_negative_for_global_expect…

…ions Fix a false negative for `Minitest/GlobalExpectations` cop
rubocop · Mar 16, 2020 · 468e2fe · 468e2fe
2 parents 757b5e2 + cedbeb3
commit 468e2fe
Show file tree

Hide file tree

Showing 3 changed files with 142 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 ### Bug fixes
 
 * [#60](https://github.com/rubocop-hq/rubocop-minitest/issues/60): Fix `Minitest/GlobalExpectations` autocorrection for chained methods. ([@tejasbubane][])
+* [#69](https://github.com/rubocop-hq/rubocop-minitest/pull/69): Fix a false negative for `Minitest/GlobalExpectations` cop when using a variable or a hash index for receiver. ([@koic][])
 
 ## 0.7.0 (2020-03-09)
 

diff --git a/lib/rubocop/cop/minitest/global_expectations.rb b/lib/rubocop/cop/minitest/global_expectations.rb
@@ -35,7 +35,11 @@ class GlobalExpectations < Cop
         end.join(' ').freeze
 
         def_node_matcher :global_expectation?, <<~PATTERN
-          (send (send _ _) {#{MATCHERS_STR}} ...)
+          (send {
+            (send _ _)
+            ({lvar ivar cvar gvar} _)
+            (send {(send _ _) ({lvar ivar cvar gvar} _)} _ _)
+          } {#{MATCHERS_STR}} ...)
         PATTERN
 
         def on_send(node)

diff --git a/test/rubocop/cop/minitest/global_expectations_test.rb b/test/rubocop/cop/minitest/global_expectations_test.rb
@@ -19,6 +19,142 @@ class GlobalExpectationsTest < Minitest::Test
       RUBY
     end
 
+    define_method(:"test_registers_offense_when_using_global_#{matcher}_for_lvar") do
+      assert_offense(<<~RUBY)
+        it 'does something' do
+          n = do_something
+          n.#{matcher} 42
+          #{'^' * (matcher.length + 5)} Prefer using `_(n).#{matcher} 42`.
+        end
+      RUBY
+
+      assert_correction(<<~RUBY)
+        it 'does something' do
+          n = do_something
+          _(n).#{matcher} 42
+        end
+      RUBY
+    end
+
+    define_method(:"test_registers_offense_when_using_global_#{matcher}_for_ivar") do
+      assert_offense(<<~RUBY)
+        it 'does something' do
+          @n = do_something
+          @n.#{matcher} 42
+          #{'^' * (matcher.length + 6)} Prefer using `_(@n).#{matcher} 42`.
+        end
+      RUBY
+
+      assert_correction(<<~RUBY)
+        it 'does something' do
+          @n = do_something
+          _(@n).#{matcher} 42
+        end
+      RUBY
+    end
+
+    define_method(:"test_registers_offense_when_using_global_#{matcher}_for_cvar") do
+      assert_offense(<<~RUBY)
+        it 'does something' do
+          @@n = do_something
+          @@n.#{matcher} 42
+          #{'^' * (matcher.length + 7)} Prefer using `_(@@n).#{matcher} 42`.
+        end
+      RUBY
+
+      assert_correction(<<~RUBY)
+        it 'does something' do
+          @@n = do_something
+          _(@@n).#{matcher} 42
+        end
+      RUBY
+    end
+
+    define_method(:"test_registers_offense_when_using_global_#{matcher}_for_gvar") do
+      assert_offense(<<~RUBY)
+        it 'does something' do
+          $n = do_something
+          $n.#{matcher} 42
+          #{'^' * (matcher.length + 6)} Prefer using `_($n).#{matcher} 42`.
+        end
+      RUBY
+
+      assert_correction(<<~RUBY)
+        it 'does something' do
+          $n = do_something
+          _($n).#{matcher} 42
+        end
+      RUBY
+    end
+
+    define_method(:"test_registers_offense_when_using_global_#{matcher}_for_hash_as_lvar") do
+      assert_offense(<<~RUBY)
+        it 'does something' do
+          n = do_something
+          n[:foo].#{matcher} 42
+          #{'^' * (matcher.length + 11)} Prefer using `_(n[:foo]).#{matcher} 42`.
+        end
+      RUBY
+
+      assert_correction(<<~RUBY)
+        it 'does something' do
+          n = do_something
+          _(n[:foo]).#{matcher} 42
+        end
+      RUBY
+    end
+
+    define_method(:"test_registers_offense_when_using_global_#{matcher}_for_hash_as_ivar") do
+      assert_offense(<<~RUBY)
+        it 'does something' do
+          @n = do_something
+          @n[:foo].#{matcher} 42
+          #{'^' * (matcher.length + 12)} Prefer using `_(@n[:foo]).#{matcher} 42`.
+        end
+      RUBY
+
+      assert_correction(<<~RUBY)
+        it 'does something' do
+          @n = do_something
+          _(@n[:foo]).#{matcher} 42
+        end
+      RUBY
+    end
+
+    define_method(:"test_registers_offense_when_using_global_#{matcher}_for_hash_as_cvar") do
+      assert_offense(<<~RUBY)
+        it 'does something' do
+          @@n = do_something
+          @@n[:foo].#{matcher} 42
+          #{'^' * (matcher.length + 13)} Prefer using `_(@@n[:foo]).#{matcher} 42`.
+        end
+      RUBY
+
+      assert_correction(<<~RUBY)
+        it 'does something' do
+          @@n = do_something
+          _(@@n[:foo]).#{matcher} 42
+        end
+      RUBY
+    end
+
+    define_method(:"test_registers_offense_when_using_global_#{matcher}_for_hash_as_gvar") do
+      assert_offense(<<~RUBY)
+        it 'does something' do
+          $n = do_something
+          $n[:foo].#{matcher} 42
+          #{'^' * (matcher.length + 12)} Prefer using `_($n[:foo]).#{matcher} 42`.
+        end
+      RUBY
+
+      assert_correction(<<~RUBY)
+        it 'does something' do
+          $n = do_something
+          _($n[:foo]).#{matcher} 42
+        end
+      RUBY
+    end
+
     define_method(:"test_no_offense_when_using_expect_form_of_#{matcher}") do
       assert_no_offenses(<<~RUBY)
         it 'does something' do