Skip to content

Commit

Permalink
Fix clobbering when consecutive nodes are with wrong order
Browse files Browse the repository at this point in the history
  • Loading branch information
Darhazer committed Jul 17, 2024
1 parent 879a747 commit 57688d6
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 14 deletions.
24 changes: 16 additions & 8 deletions lib/rubocop/cop/layout/ordered_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,33 @@ def self.method_name(node)

def on_begin(node)
start_node = node.children.find(&:class_type?)&.children&.last || node
check(start_node)
end

private

def access_modified?(node, is_class_method_block)
(node.defs_type? && !is_class_method_block) ||
(node.def_type? && is_class_method_block) ||
(node.send_type? && node.bare_access_modifier?)
end

def check(start_node)
consecutive_methods(start_node.children) do |previous, current|
next if ordered?(previous, current)

add_offense(current, message: message) do |corrector|
next if part_of_ignored_node?(previous)

OrderedMethodsCorrector.new(
processed_source.ast_with_comments, start_node.children, cop_config
).correct(current, previous, corrector)

ignore_node(current)
end
end
end

private

def access_modified?(node, is_class_method_block)
(node.defs_type? && !is_class_method_block) ||
(node.def_type? && is_class_method_block) ||
(node.send_type? && node.bare_access_modifier?)
end

# We disable `Style/ExplicitBlockArgument` for performance. See
# https://github.com/shanecav84/rubocop-ordered_methods/pull/5#pullrequestreview-562957146
# rubocop:disable Style/ExplicitBlockArgument
Expand Down
41 changes: 35 additions & 6 deletions spec/rubocop/cop/layout/ordered_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
let(:enforced_style) { 'alphabetical' }

it 'registers an offense when methods are not in alphabetical order' do
expect_offense(<<-RUBY.gsub(/^\s+\|/, ''))
expect_offense(<<~RUBY)
class Foo
def self.class_b; end
def self.class_a; end
Expand Down Expand Up @@ -69,7 +69,7 @@ def instance_c; end
end

it 'registers an offense when require is present at the top of the file (regression)' do
expect_offense(<<-RUBY.gsub(/^\s+\|/, ''))
expect_offense(<<~RUBY)
require "date"
class Foo
Expand All @@ -81,14 +81,14 @@ def self.class_a; end
end

it 'does not register an offense when methods are in alphabetical order' do
expect_no_offenses(<<-RUBY.gsub(/^\s+\|/, ''))
expect_no_offenses(<<~RUBY)
def a; end
def b; end
RUBY
end

it 'does not register an offense when there are nodes in the class but no methods' do
expect_no_offenses(<<-RUBY.gsub(/^\s+\|/, ''))
expect_no_offenses(<<~RUBY)
require "../app/lib/bar"
class Foo
Expand All @@ -97,8 +97,37 @@ class Foo
RUBY
end

it 'auto-corrects consecurive offenses' do
expect_offense(<<~RUBY)
require "date"
class Foo
def self.class_c; end
def self.class_b; end
^^^^^^^^^^^^^^^^^^^^^ Methods should be sorted in alphabetical order.
def self.class_a; end
^^^^^^^^^^^^^^^^^^^^^ Methods should be sorted in alphabetical order.
end
RUBY

# first auto-correction pass
expect_correction(<<~RUBY)
require "date"
class Foo
def self.class_b; end
def self.class_c; end
def self.class_a; end
end
RUBY
end

it 'autocorrects methods that are not in alphabetical order' do
new_source = autocorrect_source_file(<<-RUBY.gsub(/^\s+\|/, ''))
new_source = autocorrect_source_file(<<~RUBY)
class Foo
# Comment class_b
def self.class_b; end
Expand Down Expand Up @@ -150,7 +179,7 @@ def instance_c; end
end
RUBY

expect(new_source).to eq(<<-RUBY.gsub(/^\s+\|/, ''))
expect(new_source).to eq(<<~RUBY)
class Foo
def self.class_a; end
# Comment class_b
Expand Down

0 comments on commit 57688d6

Please sign in to comment.