Skip to content

Commit

Permalink
[Fix #10689] Fix autocorrect for Layout/FirstHashElementIndentation…
Browse files Browse the repository at this point in the history 
… and `Layout/FirstArrayElementIndentation`

Fixes #10689

This PR fixes the autocorrect for `Layout/FirstHashElementIndentation` and `Layout/FirstArrayElementIndentation` each combined with `Layout/HashAlignment`.

The new behavior indents the elements of a hash in a hash (or an array in a hash) based on the parent hash key _when the parent hash is a method argument and has a following other sibling pair_.

Example code to be corrected:
```ruby
func x: {
  a: 1,
       b: 2
},
     y: { # following sibling pair in the parent hash
  c: 1,
       d: 2
}
```

In the original behavior, the code is corrected like
```ruby
func x: {
  a: 1,
  b: 2
},
     y: { # following sibling pair of the parent hash
       c: 1,
       d: 2
     }
```

With this fix, the code is corrected like
```ruby
func x: {
       a: 1,
       b: 2
     },
     y: { # following sibling pair in the parent hash
       c: 1,
       d: 2
     }
```

The autocorrect keeps the original behavior when the parent hash has no other following pairs. (I could change this so that the elements of the hash are always indented based on the parent hash key, but I kept the original behavior this time)

```ruby
func x: {
  a: 1,
       b: 2
}

func x: {
  a: 1,
  b: 2
}
```
  • Loading branch information
@j-miyake @bbatsov
j-miyake authored and bbatsov committed Jun 20, 2022
1 parent 2eb3844 commit 023e0ea
Show file tree
Hide file tree
Showing 7 changed files with 530 additions and 32 deletions.
1 change: 1 addition & 0 deletions changelog/fix_autocorrect_for_first_hash_element_indentation.md
View file
@@ -0,0 +1 @@
* [#10689](https://github.com/rubocop/rubocop/issues/10689): Fix autocorrect for `Layout/FirstHashElementIndentation` and `Layout/FirstArrayElementIndentation`. ([@j-miyake][])
26 changes: 16 additions & 10 deletions lib/rubocop/cop/layout/first_array_element_indentation.rb
View file
Expand Up @@ -127,22 +127,25 @@ def check_right_bracket(right_bracket, left_bracket, left_parenthesis)
# if the right bracket is on the same line as the last value, accept
return if /\S/.match?(right_bracket.source_line[0...right_bracket.column])

expected_column = base_column(left_bracket, left_parenthesis)
expected_column, indent_base_type = indent_base(left_bracket, left_parenthesis)
@column_delta = expected_column - right_bracket.column
return if @column_delta.zero?

msg = msg(left_parenthesis)
msg = message_for_right_bracket(indent_base_type)
add_offense(right_bracket, message: msg) do |corrector|
autocorrect(corrector, right_bracket)
end
end

# Returns the description of what the correct indentation is based on.
def base_description(left_parenthesis)
if style == :align_brackets
def base_description(indent_base_type)
case indent_base_type
when :left_brace_or_bracket
'the position of the opening bracket'
elsif left_parenthesis && style == :special_inside_parentheses
when :first_colmn_after_left_parenthesis
'the first position after the preceding left parenthesis'
when :parent_hash_key
'the parent hash key'
else
'the start of the line where the left square bracket is'
end
Expand All @@ -156,15 +159,18 @@ def message(base_description)
)
end

def msg(left_parenthesis)
if style == :align_brackets
def message_for_right_bracket(indent_base_type)
case indent_base_type
when :left_brace_or_bracket
'Indent the right bracket the same as the left bracket.'
elsif style == :special_inside_parentheses && left_parenthesis
when :first_colmn_after_left_parenthesis
'Indent the right bracket the same as the first position ' \
'after the preceding left parenthesis.'
'after the preceding left parenthesis.'
when :parent_hash_key
'Indent the right bracket the same as the parent hash key.' \
else
'Indent the right bracket the same as the start of the line' \
' where the left bracket is.'
' where the left bracket is.'
end
end
end
Expand Down
59 changes: 49 additions & 10 deletions lib/rubocop/cop/layout/first_hash_element_indentation.rb
View file
Expand Up @@ -32,6 +32,14 @@ module Layout
# and_in_a_method_call({
# no: :difference
# })
# takes_multi_pairs_hash(x: {
# a: 1,
# b: 2
# },
# y: {
# c: 1,
# d: 2
# })
#
# # good
# special_inside_parentheses
Expand All @@ -41,6 +49,14 @@ module Layout
# but_in_a_method_call({
# its_like: :this
# })
# takes_multi_pairs_hash(x: {
# a: 1,
# b: 2
# },
# y: {
# c: 1,
# d: 2
# })
#
# @example EnforcedStyle: consistent
# # The `consistent` style enforces that the first key in a hash
Expand All @@ -64,6 +80,7 @@ module Layout
# no: :difference
# })
#
#
# @example EnforcedStyle: align_braces
# # The `align_brackets` style enforces that the opening and closing
# # braces are indented to the same position.
Expand All @@ -72,11 +89,27 @@ module Layout
# and_now_for_something = {
# completely: :different
# }
# takes_multi_pairs_hash(x: {
# a: 1,
# b: 2
# },
# y: {
# c: 1,
# d: 2
# })
#
# # good
# and_now_for_something = {
# completely: :different
# }
# takes_multi_pairs_hash(x: {
# a: 1,
# b: 2
# },
# y: {
# c: 1,
# d: 2
# })
class FirstHashElementIndentation < Base
include Alignment
include ConfigurableEnforcedStyle
Expand Down Expand Up @@ -132,11 +165,11 @@ def check_right_brace(right_brace, left_brace, left_parenthesis)
# if the right brace is on the same line as the last value, accept
return if /\S/.match?(right_brace.source_line[0...right_brace.column])

expected_column = base_column(left_brace, left_parenthesis)
expected_column, indent_base_type = indent_base(left_brace, left_parenthesis)
@column_delta = expected_column - right_brace.column
return if @column_delta.zero?

message = message_for_right_brace(left_parenthesis)
message = message_for_right_brace(indent_base_type)
add_offense(right_brace, message: message) do |corrector|
autocorrect(corrector, right_brace)
end
Expand All @@ -155,11 +188,14 @@ def check_based_on_longest_key(hash_node, left_brace, left_parenthesis)
end

# Returns the description of what the correct indentation is based on.
def base_description(left_parenthesis)
if style == :align_braces
def base_description(indent_base_type)
case indent_base_type
when :left_brace_or_bracket
'the position of the opening brace'
elsif left_parenthesis && style == :special_inside_parentheses
when :first_colmn_after_left_parenthesis
'the first position after the preceding left parenthesis'
when :parent_hash_key
'the parent hash key'
else
'the start of the line where the left curly brace is'
end
Expand All @@ -173,15 +209,18 @@ def message(base_description)
)
end

def message_for_right_brace(left_parenthesis)
if style == :align_braces
def message_for_right_brace(indent_base_type)
case indent_base_type
when :left_brace_or_bracket
'Indent the right brace the same as the left brace.'
elsif style == :special_inside_parentheses && left_parenthesis
when :first_colmn_after_left_parenthesis
'Indent the right brace the same as the first position ' \
'after the preceding left parenthesis.'
'after the preceding left parenthesis.'
when :parent_hash_key
'Indent the right brace the same as the parent hash key.'
else
'Indent the right brace the same as the start of the line ' \
'where the left brace is.'
'where the left brace is.'
end
end

Expand Down
48 changes: 36 additions & 12 deletions lib/rubocop/cop/mixin/multiline_element_indentation.rb
View file
Expand Up @@ -25,15 +25,17 @@ def each_argument_node(node, type)

def check_first(first, left_brace, left_parenthesis, offset)
actual_column = first.source_range.column
expected_column = base_column(left_brace, left_parenthesis) +
configured_indentation_width + offset

indent_base_column, indent_base_type = indent_base(left_brace, left_parenthesis)
expected_column = indent_base_column + configured_indentation_width + offset

@column_delta = expected_column - actual_column
styles = detected_styles(actual_column, offset, left_parenthesis, left_brace)

if @column_delta.zero?
check_expected_style(styles)
else
incorrect_style_detected(styles, first, left_parenthesis)
incorrect_style_detected(styles, first, indent_base_type)
end
end

Expand All @@ -45,14 +47,36 @@ def check_expected_style(styles)
end
end

def base_column(left_brace, left_parenthesis)
if style == brace_alignment_style
left_brace.column
elsif left_parenthesis && style == :special_inside_parentheses
left_parenthesis.column + 1
else
left_brace.source_line =~ /\S/
def indent_base(left_brace, left_parenthesis)
return [left_brace.column, :left_brace_or_bracket] if style == brace_alignment_style

pair = hash_pair_where_value_beginning_with(left_brace)
if pair && key_and_value_begin_on_same_line?(pair) && pair.right_sibling
return [pair.loc.column, :parent_hash_key]
end

if left_parenthesis && style == :special_inside_parentheses
return [left_parenthesis.column + 1, :first_colmn_after_left_parenthesis]
end

[left_brace.source_line =~ /\S/, :start_of_line]
end

def hash_pair_where_value_beginning_with(left_brace)
node = node_beginning_with(left_brace)
node.parent&.pair_type? ? node.parent : nil
end

def node_beginning_with(left_brace)
processed_source.ast.each_descendant do |node|
if node.loc.is_a?(Parser::Source::Map::Collection) && (node.loc.begin == left_brace)
break node
end
end
end

def key_and_value_begin_on_same_line?(pair)
same_line?(pair.key, pair.value)
end

def detected_styles(actual_column, offset, left_parenthesis, left_brace)
Expand All @@ -73,8 +97,8 @@ def detected_styles_for_column(column, left_parenthesis, left_brace)
styles
end

def incorrect_style_detected(styles, first, left_parenthesis)
msg = message(base_description(left_parenthesis))
def incorrect_style_detected(styles, first, base_column_type)
msg = message(base_description(base_column_type))

add_offense(first, message: msg) do |corrector|
autocorrect(corrector, first)
Expand Down
55 changes: 55 additions & 0 deletions spec/rubocop/cli/autocorrect_spec.rb
View file
Expand Up @@ -2544,4 +2544,59 @@ module Foo#{trailing_whitespace}
end
RUBY
end

it 'indents the elements of a hash in hash based on the parent hash key ' \
'when the parent hash is a method argument and has following other sibling pairs' do
source_file = Pathname('example.rb')
create_file(source_file, <<~RUBY)
desc 'Returns your public timeline.' do
headers XAuthToken: {
required: true,
description: 'Validates your identity'
},
XOptionalHeader: {
required: false,
description: 'Not really needed'
}
end
func x: [
:a,
:b
],
y: [
:c,
:d
]
RUBY

status = cli.run(
%w[--autocorrect --only] << %w[
Layout/FirstHashElementIndentation
Layout/FirstArrayElementIndentation
Layout/HashAlignment
Layout/ArrayAlignment
].join(',')
)
expect(status).to eq(0)
expect(source_file.read).to eq(<<~RUBY)
desc 'Returns your public timeline.' do
headers XAuthToken: {
required: true,
description: 'Validates your identity'
},
XOptionalHeader: {
required: false,
description: 'Not really needed'
}
end
func x: [
:a,
:b
],
y: [
:c,
:d
]
RUBY
end
end

0 comments on commit 023e0ea

Please sign in to comment.