Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Style/AccessorGrouping: Fix sibling detection for methods with type sigs #11675

Merged
merged 1 commit into from
Mar 8, 2023
Merged

Style/AccessorGrouping: Fix sibling detection for methods with type sigs #11675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_style_accessor_grouping_fix_sibling_detection_for.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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)
Copy link
Contributor Author

@issyl0 issyl0 Mar 7, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran this (new) test on RuboCop master (v1.48.0) and the autocorrect is completely bugged in the latest release because of the sibling grouping detection. Sorry for not thinking about this sooner! 😳

  1) RuboCop::Cop::Style::AccessorGrouping when EnforcedStyle is grouped registers an offense for grouped accessors distinct from a typechecked accessor method
     Failure/Error: expect(new_source).to eq(correction)
     
       expected: "class Foo\n  extend T::Sig\n\n  sig { returns(Integer) }\n  attr_reader :one\n\n  attr_reader :two, :three, :four\nend\n"
            got: "class Foo\n  extend T::Sig\n\n  sig { returns(Integer) }\n  attr_reader :one\nend\n"
     
       (compared using ==)
     
       Diff:
       @@ -3,7 +3,5 @@
        
          sig { returns(Integer) }
          attr_reader :one
       -
       -  attr_reader :two, :three, :four
        end

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