diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml index 35927de4c..3828c2c0c 100644 --- a/.github/workflows/rubocop.yml +++ b/.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.92.0', ruby: 2.4, os: ubuntu } + - { rubocop: '0.92.0', ruby: head, os: ubuntu } + - { rubocop: '0.92.0', 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.92.0', master ] steps: - name: checkout diff --git a/CHANGELOG.md b/CHANGELOG.md index ece1622e0..64148a55e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### Bug fixes + +* [#120](https://github.com/rubocop-hq/rubocop-ast/pull/120): **(Potentially breaking)** Fix false positives and negatives for `SendNode#macro?`. This impacts `{non_}bare_access_modifier?` and `special_access_modifier?`. ([@marcandre][]) + ## 0.5.0 (2020-09-24) ### New features diff --git a/lib/rubocop/ast/node/mixin/method_dispatch_node.rb b/lib/rubocop/ast/node/mixin/method_dispatch_node.rb index 2ccd568dc..4e963d9d6 100644 --- a/lib/rubocop/ast/node/mixin/method_dispatch_node.rb +++ b/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 diff --git a/lib/rubocop/ast/rubocop_compatibility.rb b/lib/rubocop/ast/rubocop_compatibility.rb index 2ece4518c..2bd603edc 100644 --- a/lib/rubocop/ast/rubocop_compatibility.rb +++ b/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.92.0' => 'Style/MixinUsage' }.freeze def rubocop_loaded loaded = Gem::Version.new(RuboCop::Version::STRING) diff --git a/spec/rubocop/ast/rubocop_compatibility_spec.rb b/spec/rubocop/ast/rubocop_compatibility_spec.rb index f8ce8d7df..ecc57ec3f 100644 --- a/spec/rubocop/ast/rubocop_compatibility_spec.rb +++ b/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.92.0' } it 'issues a warning' do expect { callback }.not_to output.to_stderr diff --git a/spec/rubocop/ast/send_node_spec.rb b/spec/rubocop/ast/send_node_spec.rb index 6a56229a0..162a86fcd 100644 --- a/spec/rubocop/ast/send_node_spec.rb +++ b/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