Skip to content

Commit

Permalink
[Fix #6326] Fix alignment in Layout/RescueEnsureAlignment when using …
Browse files Browse the repository at this point in the history
…inline access modifiers (#6328)
  • Loading branch information
andrew-aladev authored and bbatsov committed Oct 22, 2018
1 parent 1f00c77 commit c77387c
Show file tree
Hide file tree
Showing 3 changed files with 332 additions and 44 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
* [#6311](https://github.com/rubocop-hq/rubocop/pull/6311): Prevent `Style/Semicolon` from breaking on single line if-then-else in assignment. ([@drenmi][])
* [#6315](https://github.com/rubocop-hq/rubocop/pull/6315): Fix an error for `Rails/HasManyOrHasOneDependent` when an Active Record model does not have any relations. ([@koic][])
* [#6316](https://github.com/rubocop-hq/rubocop/issues/6316): Fix an auto-correct error for `Style/For` when setting `EnforcedStyle: each` with range provided to the `for` loop without a `do` keyword or semicolon and without enclosing parenthesis. ([@lukasz-wojcik][])
* [#6326](https://github.com/rubocop-hq/rubocop/issues/6326): Fix an alignment source for `Layout/RescueEnsureAlignment` when using inline access modifier. ([@andrew-aladev][])

### Changes

Expand Down Expand Up @@ -3625,5 +3626,6 @@
[@lukasz-wojcik]: https://github.com/lukasz-wojcik
[@albaer]: https://github.com/albaer
[@Kevinrob]: https://github.com/Kevinrob
[@andrew-aladev]: https://github.com/andrew-aladev
[@y-yagi]: https://github.com/y-yagi
[@DiscoStarslayer]: https://github.com/DiscoStarslayer
[@DiscoStarslayer]: https://github.com/DiscoStarslayer
127 changes: 84 additions & 43 deletions lib/rubocop/cop/layout/rescue_ensure_alignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class RescueEnsureAlignment < Cop
'%<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
ANCESTOR_TYPES_WITH_ACCESS_MODIFIERS = %i[def defs].freeze

def on_resbody(node)
check(node) unless modifier?(node)
Expand All @@ -38,6 +39,18 @@ def on_ensure(node)
check(node)
end

def autocorrect(node)
whitespace = whitespace_range(node)
# Some inline node is sitting before current node.
return nil unless whitespace.source.strip.empty?

alignment_node = alignment_node(node)
return false if alignment_node.nil?

new_column = alignment_node.loc.column
->(corrector) { corrector.replace(whitespace, ' ' * new_column) }
end

def investigate(processed_source)
@modifier_locations =
processed_source.tokens.each_with_object([]) do |token, locations|
Expand All @@ -47,50 +60,88 @@ def investigate(processed_source)
end
end

def autocorrect(node)
whitespace = whitespace_range(node)
return false unless whitespace.source.strip.empty?
private

new_column = ancestor_node(node).loc.end.column
->(corrector) { corrector.replace(whitespace, ' ' * new_column) }
# Check alignment of node with rescue or ensure modifiers.

def check(node)
alignment_node = alignment_node(node)
return if alignment_node.nil?

alignment_loc = alignment_node.loc.expression
kw_loc = node.loc.keyword

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

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

private
def format_message(alignment_node, alignment_loc, kw_loc)
format(
MSG,
kw_loc: kw_loc.source,
kw_loc_line: kw_loc.line,
kw_loc_column: kw_loc.column,
beginning: alignment_source(alignment_node, alignment_loc),
begin_loc_line: alignment_loc.line,
begin_loc_column: alignment_loc.column
)
end

def check(node)
def alignment_source(node, starting_loc)
ending_loc =
case node.type
when :block, :kwbegin
node.loc.begin
when :def, :defs, :class, :module
node.loc.name
else
# It is a wrapper with access modifier.
node.child_nodes.first.loc.name
end

range_between(starting_loc.begin_pos, ending_loc.end_pos).source
end

# We will use ancestor or wrapper with access modifier.

def alignment_node(node)
ancestor_node = ancestor_node(node)
alignment_loc = ancestor_node.loc.expression
kw_loc = node.loc.keyword
return nil if ancestor_node.nil?

return if alignment_loc.column == kw_loc.column
return if alignment_loc.line == kw_loc.line
access_modifier_node = access_modifier_node(ancestor_node)
return ancestor_node if access_modifier_node.nil?

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

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,
beginning: alignment_source(ancestor_node),
begin_loc_line: alignment_loc.line,
begin_loc_column: alignment_loc.column)
def ancestor_node(node)
ancestor_types =
if target_ruby_version >= 2.5
RUBY_2_5_ANCESTOR_TYPES
else
ANCESTOR_TYPES
end

node.each_ancestor(*ancestor_types).first
end

def alignment_source(node)
ending = case node.type
when :def, :defs, :class, :module
node.loc.name
when :block, :kwbegin
node.loc.begin
end
def access_modifier_node(node)
return nil unless
ANCESTOR_TYPES_WITH_ACCESS_MODIFIERS.include?(node.type)

access_modifier_node = node.ancestors.first
return nil unless
access_modifier_node.respond_to?(:access_modifier?) &&
access_modifier_node.access_modifier?

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

def modifier?(node)
Expand All @@ -100,21 +151,11 @@ def modifier?(node)
end

def whitespace_range(node)
begin_pos = node.loc.keyword.begin_pos
begin_pos = node.loc.keyword.begin_pos
current_column = node.loc.keyword.column

range_between(begin_pos - current_column, begin_pos)
end

def ancestor_node(node)
types = if target_ruby_version >= 2.5
RUBY_2_5_ANCESTOR_TYPES
else
ANCESTOR_TYPES
end

node.each_ancestor(*types).first
end
end
end
end
Expand Down
Loading

0 comments on commit c77387c

Please sign in to comment.