Skip to content

Commit

Permalink
[Fix rubocop#10764] Fix performance issue for Layout/FirstHashElement…
Browse files Browse the repository at this point in the history
…Indentation and Layout/FirstArrayElementIndentation

Fixes rubocop#10764.

In rubocop v1.31.0, `MultilineElementIndentation` that is used by `Layout/FirstHashElementIndentation` and `Layout/FirstArrayElementIndentation` walks all nodes through in the code to find a hash pair node where its value part begins with the given left-brace; That computation is expensive, so I made a change so that the hash pair is passed by each class that includes `MultilineElementIndentation`.
  • Loading branch information
j-miyake committed Jun 30, 2022
1 parent 8fa156c commit d5c49f9
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 19 deletions.
@@ -0,0 +1 @@
* [#10764](https://github.com/rubocop/rubocop/issues/10764): Fix performance issue for Layout/FirstHashElementIndentation and Layout/FirstArrayElementIndentation. ([@j-miyake][])
7 changes: 4 additions & 3 deletions lib/rubocop/cop/layout/first_array_element_indentation.rb
Expand Up @@ -120,14 +120,15 @@ def check(array_node, left_parenthesis)
check_first(first_elem, left_bracket, left_parenthesis, 0)
end

check_right_bracket(array_node.loc.end, left_bracket, left_parenthesis)
check_right_bracket(array_node.loc.end, first_elem, left_bracket, left_parenthesis)
end

def check_right_bracket(right_bracket, left_bracket, left_parenthesis)
def check_right_bracket(right_bracket, first_elem, 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, indent_base_type = indent_base(left_bracket, left_parenthesis)
expected_column, indent_base_type = indent_base(left_bracket, first_elem,
left_parenthesis)
@column_delta = expected_column - right_bracket.column
return if @column_delta.zero?

Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/layout/first_hash_element_indentation.rb
Expand Up @@ -158,14 +158,14 @@ def check(hash_node, left_parenthesis)
end
end

check_right_brace(hash_node.loc.end, left_brace, left_parenthesis)
check_right_brace(hash_node.loc.end, first_pair, left_brace, left_parenthesis)
end

def check_right_brace(right_brace, left_brace, left_parenthesis)
def check_right_brace(right_brace, first_pair, 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, indent_base_type = indent_base(left_brace, left_parenthesis)
expected_column, indent_base_type = indent_base(left_brace, first_pair, left_parenthesis)
@column_delta = expected_column - right_brace.column
return if @column_delta.zero?

Expand Down
19 changes: 6 additions & 13 deletions lib/rubocop/cop/mixin/multiline_element_indentation.rb
Expand Up @@ -26,7 +26,7 @@ def each_argument_node(node, type)
def check_first(first, left_brace, left_parenthesis, offset)
actual_column = first.source_range.column

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

@column_delta = expected_column - actual_column
Expand All @@ -47,10 +47,10 @@ def check_expected_style(styles)
end
end

def indent_base(left_brace, left_parenthesis)
def indent_base(left_brace, first, left_parenthesis)
return [left_brace.column, :left_brace_or_bracket] if style == brace_alignment_style

pair = hash_pair_where_value_beginning_with(left_brace)
pair = hash_pair_where_value_beginning_with(left_brace, first)
if pair && key_and_value_begin_on_same_line?(pair) &&
right_sibling_begins_on_subsequent_line?(pair)
return [pair.loc.column, :parent_hash_key]
Expand All @@ -63,17 +63,10 @@ def indent_base(left_brace, left_parenthesis)
[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 hash_pair_where_value_beginning_with(left_brace, first)
return unless first && first.parent.loc.begin == left_brace

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
first.parent&.parent&.pair_type? ? first.parent.parent : nil
end

def key_and_value_begin_on_same_line?(pair)
Expand Down

0 comments on commit d5c49f9

Please sign in to comment.