Skip to content

Commit

Permalink
Merge pull request #6792 from koic/fix_infinite_loop_indentation_width
Browse files Browse the repository at this point in the history
[Fix #6699] Fix a false negative for `Layout/IndentationWidth`
  • Loading branch information
koic committed Feb 26, 2019
2 parents 9ae94da + f5b6244 commit 3d9131d
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### Bug fixes

* [#6699](https://github.com/rubocop-hq/rubocop/issues/6699): Fix infinite loop for `Layout/IndentationWidth` and `Layout/IndentationConsistency` when bad modifier indentation before good method definition. ([@koic][])

### Changes

* [#6688](https://github.com/rubocop-hq/rubocop/pull/6688): Add `iterator?` to deprecated methods and prefer `block_given?` instead. ([@tejasbubane][])
Expand Down
28 changes: 23 additions & 5 deletions lib/rubocop/cop/layout/indentation_width.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ class IndentationWidth < Cop # rubocop:disable Metrics/ClassLength

SPECIAL_MODIFIERS = %w[private protected].freeze

def_node_matcher :access_modifier?, <<-PATTERN
[(send ...) access_modifier?]
PATTERN

def on_rescue(node)
_begin_node, *_rescue_nodes, else_node = *node
check_indentation(node.loc.else, else_node)
Expand Down Expand Up @@ -161,15 +165,12 @@ def autocorrect(node)
private

def check_members(base, members)
check_indentation(base, members.first)
check_indentation(base, select_check_member(members.first))

return unless members.any? && members.first.begin_type?

if indentation_consistency_style == 'rails'
each_member(members) do |member, previous_modifier|
check_indentation(previous_modifier, member,
indentation_consistency_style)
end
check_members_for_rails_style(members)
else
members.first.children.each do |member|
next if member.send_type? && member.access_modifier?
Expand All @@ -179,6 +180,23 @@ def check_members(base, members)
end
end

def select_check_member(member)
return unless member

if access_modifier?(member.children.first)
member.children.first
else
member
end
end

def check_members_for_rails_style(members)
each_member(members) do |member, previous_modifier|
check_indentation(previous_modifier, member,
indentation_consistency_style)
end
end

def each_member(members)
previous_modifier = nil
members.first.children.each do |member|
Expand Down
6 changes: 6 additions & 0 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -986,12 +986,18 @@ class Dsl
#{e}:1:1: C: Style/Documentation: Missing top-level class documentation comment.
#{e}:2:1: C: [Corrected] Layout/AccessModifierIndentation: Indent access modifiers like `private`.
#{e}:2:1: C: [Corrected] Layout/EmptyLinesAroundAccessModifier: Keep a blank line after `private`.
#{e}:2:1: C: [Corrected] Layout/IndentationWidth: Use 2 (not 0) spaces for indentation.
#{e}:2:1: C: [Corrected] Layout/IndentationWidth: Use 2 (not 4) spaces for indentation.
#{e}:2:3: W: Lint/UselessAccessModifier: Useless `private` access modifier.
#{e}:2:5: C: [Corrected] Layout/AccessModifierIndentation: Indent access modifiers like `private`.
#{e}:3:7: C: [Corrected] Style/MutableConstant: Freeze mutable objects assigned to constants.
#{e}:3:7: C: [Corrected] Style/WordArray: Use `%w` or `%W` for an array of words.
#{e}:3:8: C: [Corrected] Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
#{e}:3:15: C: [Corrected] Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
#{e}:3:21: C: [Corrected] Style/TrailingCommaInArrayLiteral: Avoid comma after the last item of an array.
#{e}:4:1: C: [Corrected] Layout/IndentationWidth: Use 2 (not 4) spaces for indentation.
#{e}:4:3: C: [Corrected] Layout/IndentationConsistency: Inconsistent indentation detected.
#{e}:4:5: C: [Corrected] Layout/IndentationConsistency: Inconsistent indentation detected.
#{e}:4:7: C: [Corrected] Style/WordArray: Use `%w` or `%W` for an array of words.
RESULT
end
Expand Down
22 changes: 22 additions & 0 deletions spec/rubocop/cop/layout/indentation_width_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1072,6 +1072,28 @@ def f
end
RUBY
end

it 'registers an offense and corrects for bad modifier indentation ' \
'before good method definition' do
expect_offense(<<-RUBY.strip_indent)
class Foo
private
^^^^^^ Use 2 (not 6) spaces for indentation.
def foo
end
end
RUBY

expect_correction(<<-RUBY.strip_indent)
class Foo
private
def foo
end
end
RUBY
end
end

context 'when consistency style is normal' do
Expand Down

0 comments on commit 3d9131d

Please sign in to comment.