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.

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

[Fix #6202] RescueEnsureAlignment auto-correct infinite loop #6246

Merged
merged 3 commits into from
Sep 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
* [#6196](https://github.com/rubocop-hq/rubocop/issues/6196): Fix incorrect autocorrect for `Style/EmptyCaseCondition` when using `return` in `when` clause and assigning the return value of `case`. ([@koic][])
* [#6142](https://github.com/rubocop-hq/rubocop/issues/6142): Ignore keyword arguments in `Rails/Delegate`. ([@sunny][])
* [#6240](https://github.com/rubocop-hq/rubocop/issues/6240): Fix an auto-correct error for `Style/WordArray` when setting `EnforcedStyle: brackets` and using string interpolation in `%W` literal. ([@koic][])
* [#6202](https://github.com/rubocop-hq/rubocop/issues/6202): Fix infinite loop when auto-correcting `Lint/RescueEnsureAlignment` when `end` is misaligned. The alignment and message are now based on the beginning position rather than the `end` position. ([@rrosenblum][])
* [#6199](https://github.com/rubocop-hq/rubocop/issues/6199): Don't recommend `Date` usage in `Style/DateTime`. ([@deivid-rodriguez][])

### Changes
Expand Down
43 changes: 32 additions & 11 deletions lib/rubocop/cop/layout/rescue_ensure_alignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ class RescueEnsureAlignment < Cop
include RangeHelp

MSG = '`%<kw_loc>s` at %<kw_loc_line>d, %<kw_loc_column>d is not ' \
'aligned with `end` at %<end_loc_line>d, %<end_loc_column>d.'
.freeze
'aligned with `%<beginning>s` at ' \
'%<begin_loc_line>d, %<begin_loc_column>d.'.freeze
ANCESTOR_TYPES = %i[kwbegin def defs class module].freeze
RUBY_2_5_ANCESTOR_TYPES = (ANCESTOR_TYPES + [:block]).freeze

def on_resbody(node)
check(node) unless modifier?(node)
Expand Down Expand Up @@ -56,24 +58,39 @@ def autocorrect(node)
private

def check(node)
end_loc = ancestor_node(node).loc.end
ancestor_node = ancestor_node(node)
alignment_loc = ancestor_node.loc.expression
kw_loc = node.loc.keyword

return if end_loc.column == kw_loc.column
return if end_loc.line == kw_loc.line
return if alignment_loc.column == kw_loc.column
return if alignment_loc.line == kw_loc.line

add_offense(node,
location: kw_loc,
message: format_message(kw_loc, end_loc))
message: format_message(kw_loc,
alignment_loc,
ancestor_node))
end

def format_message(kw_loc, end_loc)
def format_message(kw_loc, alignment_loc, ancestor_node)
format(MSG,
kw_loc: kw_loc.source,
kw_loc_line: kw_loc.line,
kw_loc_column: kw_loc.column,
end_loc_line: end_loc.line,
end_loc_column: end_loc.column)
beginning: alignment_source(ancestor_node),
begin_loc_line: alignment_loc.line,
begin_loc_column: alignment_loc.column)
end

def alignment_source(node)
ending = case node.type
when :def, :defs, :class, :module
node.loc.name
when :block, :kwbegin
node.loc.begin
end

range_between(node.loc.expression.begin_pos, ending.end_pos).source
end

def modifier?(node)
Expand All @@ -90,8 +107,12 @@ def whitespace_range(node)
end

def ancestor_node(node)
types = %i[kwbegin def defs class module]
types << :block if target_ruby_version >= 2.5
types = if target_ruby_version >= 2.5
RUBY_2_5_ANCESTOR_TYPES
else
ANCESTOR_TYPES
end

node.each_ancestor(*types).first
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -645,8 +645,8 @@ def method
def func
foo
bar
rescue StandardError
baz
rescue StandardError
baz
end

def func
Expand Down