Skip to content

Commit

Permalink
Fix an incorrect autocorrect for Layout/ClassStructure when definit…
Browse files Browse the repository at this point in the history
…ions that need to be sorted are defined alternately

This PR is fix an incorrect autocorrect for `Layout/ClassStructure`
when definitions that need to be sorted are defined alternately.

## Code to reproduce

```ruby
class A
  private def foo; end

  def bar; end

  private def baz; end

  def qux; end
end
```

## Expected behavior

Automatically corrected as follows:

```ruby
class A
  def bar; end

  def qux; end

  private def foo; end

  private def baz; end
end
```

## Actual behavior

Automatically corrected as follows:

```ruby
class A
  def qux; end

  def bar; end

  def qux; end # <------ Duplicate

  private def foo; end

  private def baz; end
end
```
  • Loading branch information
ydah committed Feb 1, 2023
1 parent 1b6aa94 commit 1a1c4ff
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11529](https://github.com/rubocop/rubocop/pull/11529): Fix an incorrect autocorrect for `Layout/ClassStructure` when definitions that need to be sorted are defined alternately. ([@ydah][])
18 changes: 2 additions & 16 deletions lib/rubocop/cop/layout/class_structure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ module Layout
#
class ClassStructure < Base
include VisibilityHelp
include CommentsHelp
extend AutoCorrector

HUMANIZED_NODE_TYPE = {
Expand Down Expand Up @@ -163,7 +164,7 @@ def on_class(class_node)

# Autocorrect by swapping between two nodes autocorrecting them
def autocorrect(corrector, node)
previous = node.left_siblings.find do |sibling|
previous = node.left_siblings.reverse.find do |sibling|
!ignore_for_autocorrect?(node, sibling)
end
return unless previous
Expand Down Expand Up @@ -283,21 +284,6 @@ def marked_as_private_constant?(node, name)
node.arguments.any? { |arg| (arg.sym_type? || arg.str_type?) && arg.value == name }
end

def source_range_with_comment(node)
begin_pos, end_pos =
if (node.def_type? && !node.method?(:initialize)) ||
(node.send_type? && node.def_modifier?)
start_node = find_visibility_start(node) || node
end_node = find_visibility_end(node) || node
[begin_pos_with_comment(start_node),
end_position_for(end_node) + 1]
else
[begin_pos_with_comment(node), end_position_for(node)]
end

Parser::Source::Range.new(buffer, begin_pos, end_pos)
end

def end_position_for(node)
heredoc = find_heredoc(node)
return heredoc.location.heredoc_end.end_pos + 1 if heredoc
Expand Down
34 changes: 31 additions & 3 deletions spec/rubocop/cop/layout/class_structure_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,9 @@ class A
class A
public def bar
end
private def foo
end
end
RUBY
end
Expand All @@ -427,9 +427,37 @@ def bar
class A
def bar
end
private def foo
end
end
RUBY
end

it 'registers an offense and corrects when definitions that need to be sorted are defined alternately' do
expect_offense(<<~RUBY)
class A
private def foo; end
def bar; end
^^^^^^^^^^^^ `public_methods` is supposed to appear before `private_methods`.
private def baz; end
def qux; end
^^^^^^^^^^^^ `public_methods` is supposed to appear before `private_methods`.
end
RUBY

expect_correction(<<~RUBY)
class A
def bar; end
def qux; end
private def foo; end
private def baz; end
end
RUBY
end
Expand All @@ -451,10 +479,10 @@ def bar
class A
def bar
end
def foo
end
private :foo
end
RUBY
end
Expand Down

0 comments on commit 1a1c4ff

Please sign in to comment.