Fix false positives and negatives for SendNode#macro?

Bump minimum compatibility with main gem.

.github/workflows/rubocop.yml

@@ -25,9 +25,9 @@ jobs:
         title: [ null ]
         include:
           - { os: windows, rubocop: master, ruby: mingw }
-          - { rubocop: '0.89.0', ruby: 2.4, os: ubuntu }
-          - { rubocop: '0.89.0', ruby: head, os: ubuntu }
-          - { rubocop: '0.89.0', ruby: 2.4, os: ubuntu, coverage: true, title: 'Coverage' }
+          - { rubocop: '0.91.2', ruby: 2.4, os: ubuntu }
+          - { rubocop: '0.91.2', ruby: head, os: ubuntu }
+          - { rubocop: '0.91.2', ruby: 2.4, os: ubuntu, coverage: true, title: 'Coverage' }
           - { rubocop: master, ruby: 2.7, os: ubuntu, modern: true, title: 'Specs "modern"' }
           - { rubocop: master, ruby: 2.7, os: ubuntu, internal_investigation: true, modern: true, title: 'Coding Style' }
 
@@ -87,7 +87,7 @@ jobs:
       matrix:
         os: [ ubuntu ]
         ruby: [ 2.4, 2.7 ]
-        rubocop: [ '0.89.0', master ]
+        rubocop: [ '0.91.2', master ]
 
     steps:
       - name: checkout

CHANGELOG.md

@@ -12,6 +12,10 @@
 
 * [#121](https://github.com/rubocop-hq/rubocop-ast/pull/121): Update from `Parser::Ruby28` to `Parser::Ruby30` for Ruby 3.0 parser (experimental). ([@koic][])
 
+### Bug fixes
+
+* [#120](https://github.com/rubocop-hq/rubocop-ast/pull/120): **(Potentially breaking)** Fix false positives and negatives for `SendNode#macro?` ([@marcandre][])
+
 ## 0.4.2 (2020-09-18)
 
 ### Bug fixes

lib/rubocop/ast/node/mixin/method_dispatch_node.rb

@@ -42,7 +42,7 @@ def block_node
       #
       # @return [Boolean] whether the dispatched method is a macro method
       def macro?
-        !receiver && macro_scope?
+        !receiver && in_macro_scope?
       end
 
       # Checks whether the dispatched method is an access modifier.
@@ -222,31 +222,21 @@ def binary_operation?
 
       private
 
-      def_node_matcher :macro_scope?, <<~PATTERN
-        {^{({sclass class module block} ...) class_constructor?}
-         ^^{({sclass class module block} ... ({begin if} ...)) class_constructor?}
-         ^#macro_kwbegin_wrapper?
-         #root_node?}
+      def_node_matcher :in_macro_scope?, <<~PATTERN
+        {
+          root?                                    # Either a root node,
+          ^{                                       # or the parent is...
+            sclass class module class_constructor? # a class-like node
+            [ {                                    # or some "wrapper"
+                kwbegin begin block
+                (if _condition <%0 _>)  # note: we're excluding the condition of `if` nodes
+              }
+              #in_macro_scope?                     # that is itself in a macro scope
+            ]
+          }
+        }
       PATTERN
 
-      # Check if a node's parent is a kwbegin wrapper within a macro scope
-      #
-      # @param parent [Node] parent of the node being checked
-      #
-      # @return [Boolean] true if the parent is a kwbegin in a macro scope
-      def macro_kwbegin_wrapper?(parent)
-        parent.kwbegin_type? && macro_scope?(parent)
-      end
-
-      # Check if a node does not have a parent
-      #
-      # @param node [Node]
-      #
-      # @return [Boolean] if the parent is nil
-      def root_node?(node)
-        node.parent.nil?
-      end
-
       def_node_matcher :adjacent_def_modifier?, <<~PATTERN
         (send nil? _ ({def defs} ...))
       PATTERN

lib/rubocop/ast/rubocop_compatibility.rb

@@ -7,7 +7,8 @@ module AST
     # @api private
     module RuboCopCompatibility
       INCOMPATIBLE_COPS = {
-        '0.89.0' => 'Layout/LineLength'
+        '0.89.0' => 'Layout/LineLength',
+        '0.91.2' => 'Style/MixinUsage'
       }.freeze
       def rubocop_loaded
         loaded = Gem::Version.new(RuboCop::Version::STRING)

spec/rubocop/ast/rubocop_compatibility_spec.rb

@@ -16,7 +16,7 @@
   end
 
   context 'when ran from a compatible version of Rubocop' do
-    let(:rubocop_version) { '0.89.0' }
+    let(:rubocop_version) { '0.91.2' }
 
     it 'issues a warning' do
       expect { callback }.not_to output.to_stderr

spec/rubocop/ast/send_node_spec.rb

@@ -253,7 +253,7 @@ module Foo
         it { expect(send_node.macro?).to be_truthy }
       end
 
-      context 'when parent is a block' do
+      context 'when parent is a block in a macro scope' do
         let(:source) do
           ['concern :Auth do',
            '>>bar :baz<<',
@@ -264,6 +264,30 @@ module Foo
         it { expect(send_node.macro?).to be_truthy }
       end
 
+      context 'when parent is a block not in a macro scope' do
+        let(:source) { <<~RUBY }
+          class Foo
+            def bar
+              3.times do
+                >>something :baz<<
+                other
+              end
+            end
+          end
+        RUBY
+
+        it { expect(send_node.macro?).to be_falsey }
+      end
+
+      context 'when in the global scope' do
+        let(:source) { <<~RUBY }
+          >>something :baz<<
+          other
+        RUBY
+
+        it { expect(send_node.macro?).to be_truthy }
+      end
+
       context 'when parent is a keyword begin inside of an class' do
         let(:source) do
           ['class Foo',
@@ -301,6 +325,24 @@ module Foo
 
         it { expect(send_node.macro?).to be_falsey }
       end
+
+      context 'when in an if' do
+        let(:source) { <<~RUBY }
+          >>bar :baz<< if qux
+          other
+        RUBY
+
+        it { expect(send_node.macro?).to be_truthy }
+      end
+
+      context 'when the condition of an if' do
+        let(:source) { <<~RUBY }
+          qux if >>bar :baz<<
+          other
+        RUBY
+
+        it { expect(send_node.macro?).to be_falsey }
+      end
     end
 
     context 'with a receiver' do