Skip to content

Commit

Permalink
Style/AccessorGrouping: Fix sibling detection for methods with type sigs
Browse files Browse the repository at this point in the history
- Over in `Homebrew/brew`, trying to enable this cop after
  008506d was released, we were seeing
  the following offenses which were incorrect since we can't combine
  attrs that follow on from typed ones.
- This commit ensures that sibling accessors are groupable before
  raising an offense that they should be grouped.
- Also add more tests to ensure the offense detection and subsequent
  autocorrection still works.

```
  sig { returns(Pathname) }
  attr_reader :cached_location

  attr_reader :cache, :meta, :name, :version
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Group together all `attr_reader` attributes
```
  • Loading branch information
issyl0 committed Mar 7, 2023
1 parent 6136ffd commit 1fd778b
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11675](https://github.com/rubocop/rubocop/pull/11675): `Style/AccessorGrouping`: Fix sibling detection for methods with type sigs. ([@issyl0][])
8 changes: 4 additions & 4 deletions lib/rubocop/cop/style/accessor_grouping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def on_class(node)

def check(send_node)
return if previous_line_comment?(send_node) || !groupable_accessor?(send_node)
return unless (grouped_style? && sibling_accessors(send_node).size > 1) ||
return unless (grouped_style? && groupable_sibling_accessors(send_node).size > 1) ||
(separated_style? && send_node.arguments.size > 1)

message = message(send_node)
Expand Down Expand Up @@ -127,12 +127,12 @@ def separated_style?
style == :separated
end

def sibling_accessors(send_node)
def groupable_sibling_accessors(send_node)
send_node.parent.each_child_node(:send).select do |sibling|
sibling.attribute_accessor? &&
sibling.method?(send_node.method_name) &&
node_visibility(sibling) == node_visibility(send_node) &&
!previous_line_comment?(sibling)
groupable_accessor?(sibling) && !previous_line_comment?(sibling)
end
end

Expand All @@ -143,7 +143,7 @@ def message(send_node)

def preferred_accessors(node)
if grouped_style?
accessors = sibling_accessors(node)
accessors = groupable_sibling_accessors(node)
group_accessors(node, accessors) if node.loc == accessors.first.loc
else
separate_accessors(node)
Expand Down
41 changes: 41 additions & 0 deletions spec/rubocop/cop/style/accessor_grouping_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,47 @@ class Foo
RUBY
end

it 'does not register an offense for grouped accessors below a typechecked accessor method' do
expect_no_offenses(<<~RUBY)
class Foo
extend T::Sig
sig { returns(Integer) }
attr_reader :one
attr_reader :two, :three
end
RUBY
end

it 'registers an offense for grouped accessors distinct from a typechecked accessor method' do
expect_offense(<<~RUBY)
class Foo
extend T::Sig
sig { returns(Integer) }
attr_reader :one
attr_reader :two, :three
^^^^^^^^^^^^^^^^^^^^^^^^ Group together all `attr_reader` attributes.
attr_reader :four
^^^^^^^^^^^^^^^^^ Group together all `attr_reader` attributes.
end
RUBY

expect_correction(<<~RUBY)
class Foo
extend T::Sig
sig { returns(Integer) }
attr_reader :one
attr_reader :two, :three, :four
end
RUBY
end

it 'registers an offense for accessors with method definitions' do
expect_offense(<<~RUBY)
class Foo
Expand Down

0 comments on commit 1fd778b

Please sign in to comment.