Skip to content

Commit

Permalink
[Fix #5956] Make Layout/AccessModifierIndentation work with dynamic…
Browse files Browse the repository at this point in the history
… modules (#6573)

* [Fix #5956] More robust AccessModifierIndentation

Use `end`'s indentation as the base indentation, which is more robust
since it also plays nice with dynamic module or class definitions.

* Move `effective_column` method to `RangeHelp`

It fits better in there an it also make it possible to reuse the method
in places where `ConfigurableEnforcedStyle` has been separatedly mixed
in.

* Extract `column_offset_between` method

And move `effective_colum` under the "private note".
  • Loading branch information
deivid-rodriguez authored and bbatsov committed Dec 23, 2018
1 parent fb05bc5 commit c40e683
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* [#6389](https://github.com/rubocop-hq/rubocop/pull/6389): Fix false negative for `Style/TrailingCommaInHashLitera`/`Style/TrailingCommaInArrayLiteral` when there is a comment in the last line. ([@bayandin][])
* [#6566](https://github.com/rubocop-hq/rubocop/issues/6566): Fix false positive for `Layout/EmptyLinesAroundAccessModifier` when at the end of specifying a superclass is missing blank line. ([@koic][])
* [#6571](https://github.com/rubocop-hq/rubocop/issues/6571): Fix a false positive for `Layout/TrailingCommaInArguments` when a line break before a method call and `EnforcedStyleForMultiline` is set to `consistent_comma`. ([@koic][])
* [#6573](https://github.com/rubocop-hq/rubocop/pull/6573): Make `Layout/AccessModifierIndentation` work for dynamic module or class definitions. ([@deivid-rodriguez][])
* [#6562](https://github.com/rubocop-hq/rubocop/pull/6562): Fix a false positive for `Style/MethodCallWithArgsParentheses` `omit_parentheses` enforced style after safe navigation call. ([@gsamokovarov][])
* [#6570](https://github.com/rubocop-hq/rubocop/pull/6570): Fix a false positive for `Style/MethodCallWithArgsParentheses` `omit_parentheses` enforced style while splatting the result of a method invocation. ([@gsamokovarov][])
* [#6598](https://github.com/rubocop-hq/rubocop/pull/6598): Fix a false positive for `Style/MethodCallWithArgsParentheses` `omit_parentheses` enforced style for calls with regexp slash literals argument. ([@gsamokovarov][])
Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cop/layout/access_modifier_indentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ module Layout
class AccessModifierIndentation < Cop
include Alignment
include ConfigurableEnforcedStyle
include RangeHelp

MSG = '%<style>s access modifiers like `%<node>s`.'.freeze

Expand Down Expand Up @@ -69,14 +70,13 @@ def check_body(body, node)
return unless body.begin_type?

modifiers = body.each_child_node(:send).select(&:access_modifier?)
class_column = node.source_range.column
end_range = node.loc.end

modifiers.each { |modifier| check_modifier(modifier, class_column) }
modifiers.each { |modifier| check_modifier(modifier, end_range) }
end

def check_modifier(send_node, class_start_col)
access_modifier_start_col = send_node.source_range.column
offset = access_modifier_start_col - class_start_col
def check_modifier(send_node, end_range)
offset = column_offset_between(send_node.source_range, end_range)

@column_delta = expected_indent_offset - offset
if @column_delta.zero?
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/else_alignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def check_assignment(node, rhs)
def check_alignment(base_range, else_range)
return unless begins_its_line?(else_range)

@column_delta = effective_column(base_range) - else_range.column
@column_delta = column_offset_between(base_range, else_range)
return if @column_delta.zero?

message = format(
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/indentation_width.rb
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def check_if(node, body, else_clause, base_loc)
def check_indentation(base_loc, body_node, style = 'normal')
return unless indentation_to_check?(base_loc, body_node)

indentation = body_node.loc.column - effective_column(base_loc)
indentation = column_offset_between(body_node.loc, base_loc)
@column_delta = configured_indentation_width - indentation
return if @column_delta.zero?

Expand Down
18 changes: 2 additions & 16 deletions lib/rubocop/cop/mixin/end_keyword_alignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ module Cop
# Functions for checking the alignment of the `end` keyword.
module EndKeywordAlignment
include ConfigurableEnforcedStyle

BYTE_ORDER_MARK = 0xfeff # The Unicode codepoint
include RangeHelp

MSG = '`end` at %<end_line>d, %<end_col>d is not aligned with ' \
'`%<source>s` at %<align_line>d, %<align_col>d.'.freeze
Expand Down Expand Up @@ -36,7 +35,7 @@ def check_end_kw_alignment(node, align_ranges)
def matching_ranges(end_loc, align_ranges)
align_ranges.select do |_, range|
range.line == end_loc.line ||
effective_column(range) == end_loc.column
column_offset_between(range, end_loc).zero?
end
end

Expand Down Expand Up @@ -66,19 +65,6 @@ def variable_alignment?(whole_expression, rhs, end_alignment_style)
def line_break_before_keyword?(whole_expression, rhs)
rhs.first_line > whole_expression.line
end

# Returns the column attribute of the range, except if the range is on
# the first line and there's a byte order mark at the beginning of that
# line, in which case 1 is subtracted from the column value. This gives
# the column as it appears when viewing the file in an editor.
def effective_column(range)
if range.line == 1 &&
@processed_source.raw_source.codepoints.first == BYTE_ORDER_MARK
range.column - 1
else
range.column
end
end
end
end
end
19 changes: 19 additions & 0 deletions lib/rubocop/cop/mixin/range_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ module Cop
module RangeHelp
private

BYTE_ORDER_MARK = 0xfeff # The Unicode codepoint

def source_range(source_buffer, line_number, column, length = 1)
if column.is_a?(Range)
column_index = column.begin
Expand Down Expand Up @@ -72,8 +74,25 @@ def range_by_whole_lines(range, include_final_newline: false)
.intersect(buffer.source_range)
end

def column_offset_between(base_range, range)
effective_column(base_range) - effective_column(range)
end

## Helpers for above range methods. Do not use inside Cops.

# Returns the column attribute of the range, except if the range is on
# the first line and there's a byte order mark at the beginning of that
# line, in which case 1 is subtracted from the column value. This gives
# the column as it appears when viewing the file in an editor.
def effective_column(range)
if range.line == 1 &&
@processed_source.raw_source.codepoints.first == BYTE_ORDER_MARK
range.column - 1
else
range.column
end
end

def directions(side)
if side == :both
[true, true]
Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/layout/access_modifier_indentation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,17 @@ def test; end
RUBY
end

it 'accepts properly indented private in module defined with Module.new' do
expect_no_offenses(<<-RUBY.strip_indent)
Test = Module.new do
private
def test; end
end
RUBY
end

it 'accepts an empty class' do
expect_no_offenses(<<-RUBY.strip_indent)
class Test
Expand Down

0 comments on commit c40e683

Please sign in to comment.