Skip to content

Commit

Permalink
Limit Layout/ClassStructure constant order autocorrect to literal c…
Browse files Browse the repository at this point in the history
…onstants

Closes #6962
  • Loading branch information
tejasbubane authored and bbatsov committed Nov 16, 2020
1 parent 51fa5bf commit a76a2a4
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@
* [#7991](https://github.com/rubocop-hq/rubocop/issues/7991): Fix an error for `Layout/EmptyLinesAroundAttributeAccessor` when attribute method is method chained. ([@koic][])
* [#7993](https://github.com/rubocop-hq/rubocop/issues/7993): Fix a false positive for `Migration/DepartmentName` when a disable comment contains an unexpected character for department name. ([@koic][])
* [#7983](https://github.com/rubocop-hq/rubocop/pull/7983): Make the config loader Bundler-aware. ([@CvX][])
* [#6962](https://github.com/rubocop-hq/rubocop/issues/6962): Limit `Layout/ClassStructure` constant order autocorrect to literal constants. ([@tejasbubane][])

### Changes

Expand Down
18 changes: 15 additions & 3 deletions lib/rubocop/cop/layout/class_structure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ class ClassStructure < Base
MSG = '`%<category>s` is supposed to appear before ' \
'`%<previous>s`.'

def_node_matcher :dynamic_constant?, <<~PATTERN
(casgn nil? _ (send ...))
PATTERN

# Validates code style on class declaration.
# Add offense when find a node out of expected order.
def on_class(class_node)
Expand All @@ -168,11 +172,10 @@ def on_class(class_node)

# Autocorrect by swapping between two nodes autocorrecting them
def autocorrect(corrector, node)
node_classification = classify(node)
previous = node.left_siblings.find do |sibling|
classification = classify(sibling)
!ignore?(classification) && node_classification != classification
!ignore_for_autocorrect?(node, sibling)
end
return unless previous

current_range = source_range_with_comment(node)
previous_range = source_range_with_comment(previous)
Expand Down Expand Up @@ -241,6 +244,15 @@ def ignore?(classification)
expected_order.index(classification).nil?
end

def ignore_for_autocorrect?(node, sibling)
classification = classify(node)
sibling_class = classify(sibling)

ignore?(sibling_class) ||
classification == sibling_class ||
dynamic_constant?(node)
end

def humanize_node(node)
if node.def_type?
return :initializer if node.method?(:initialize)
Expand Down
36 changes: 36 additions & 0 deletions spec/rubocop/cop/layout/class_structure_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,22 @@ def some_private_method
it { expect_offense(code) }
end

context 'constant is not a literal' do
it 'registers offense but does not autocorrect' do
expect_offense <<~RUBY
class Person
def name; end
foo = 5
LIMIT = foo + 1
^^^^^^^^^^^^^^^ `constants` is supposed to appear before `public_methods`.
end
RUBY

expect_no_corrections
end
end

describe '#autocorrect' do
context 'when there is a comment in the macro method' do
it 'autocorrects the offenses' do
Expand All @@ -213,6 +229,26 @@ class Foo
RUBY
end
end

context 'literal constant is after method definitions' do
it 'autocorrects the offenses' do
new_source = autocorrect_source(<<~RUBY)
class Foo
def name; end
LIMIT = 10
end
RUBY

expect(new_source).to eq(<<~RUBY)
class Foo
LIMIT = 10
def name; end
end
RUBY
end
end
end

it 'registers an offense and corrects when str heredoc constant is defined after public method' do
Expand Down

0 comments on commit a76a2a4

Please sign in to comment.