Skip to content

Commit

Permalink
Add switch block refactor (#2372)
Browse files Browse the repository at this point in the history
* Add the ability to locate the first node within a range

* Add the switch block style refactor

* Apply consistent block style to all nested blocks
  • Loading branch information
vinistock authored Aug 8, 2024
1 parent b6ba7da commit b7f033e
Show file tree
Hide file tree
Showing 11 changed files with 331 additions and 7 deletions.
34 changes: 34 additions & 0 deletions lib/ruby_lsp/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,40 @@ def locate(node, char_position, node_types: [])
NodeContext.new(closest, parent, nesting_nodes, call_node)
end

sig do
params(
range: T::Hash[Symbol, T.untyped],
node_types: T::Array[T.class_of(Prism::Node)],
).returns(T.nilable(Prism::Node))
end
def locate_first_within_range(range, node_types: [])
scanner = create_scanner
start_position = scanner.find_char_position(range[:start])
end_position = scanner.find_char_position(range[:end])
desired_range = (start_position...end_position)
queue = T.let(@parse_result.value.child_nodes.compact, T::Array[T.nilable(Prism::Node)])

until queue.empty?
candidate = queue.shift

# Skip nil child nodes
next if candidate.nil?

# Add the next child_nodes to the queue to be processed. The order here is important! We want to move in the
# same order as the visiting mechanism, which means searching the child nodes before moving on to the next
# sibling
T.unsafe(queue).unshift(*candidate.child_nodes)

# Skip if the current node doesn't cover the desired position
loc = candidate.location

if desired_range.cover?(loc.start_offset...loc.end_offset) &&
(node_types.empty? || node_types.any? { |type| candidate.class == type })
return candidate
end
end
end

sig { returns(SorbetLevel) }
def sorbet_level
sigil = parse_result.magic_comments.find do |comment|
Expand Down
110 changes: 104 additions & 6 deletions lib/ruby_lsp/requests/code_action_resolve.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ module Requests
#
class CodeActionResolve < Request
extend T::Sig
include Support::Common

NEW_VARIABLE_NAME = "new_variable"
NEW_METHOD_NAME = "new_method"

Expand All @@ -45,20 +47,62 @@ def initialize(document, code_action)

sig { override.returns(T.any(Interface::CodeAction, Error)) }
def perform
return Error::EmptySelection if @document.source.empty?

case @code_action[:title]
when CodeActions::EXTRACT_TO_VARIABLE_TITLE
refactor_variable
when CodeActions::EXTRACT_TO_METHOD_TITLE
refactor_method
when CodeActions::SWITCH_BLOCK_STYLE_TITLE
switch_block_style
else
Error::UnknownCodeAction
end
end

private

sig { returns(T.any(Interface::CodeAction, Error)) }
def refactor_variable
return Error::EmptySelection if @document.source.empty?
def switch_block_style
source_range = @code_action.dig(:data, :range)
return Error::EmptySelection if source_range[:start] == source_range[:end]

target = @document.locate_first_within_range(
@code_action.dig(:data, :range),
node_types: [Prism::CallNode],
)

return Error::InvalidTargetRange unless target.is_a?(Prism::CallNode)

node = target.block
return Error::InvalidTargetRange unless node.is_a?(Prism::BlockNode)

indentation = " " * target.location.start_column unless node.opening_loc.slice == "do"

Interface::CodeAction.new(
title: CodeActions::SWITCH_BLOCK_STYLE_TITLE,
edit: Interface::WorkspaceEdit.new(
document_changes: [
Interface::TextDocumentEdit.new(
text_document: Interface::OptionalVersionedTextDocumentIdentifier.new(
uri: @code_action.dig(:data, :uri),
version: nil,
),
edits: [
Interface::TextEdit.new(
range: range_from_location(node.location),
new_text: recursively_switch_nested_block_styles(node, indentation),
),
],
),
],
),
)
end

sig { returns(T.any(Interface::CodeAction, Error)) }
def refactor_variable
source_range = @code_action.dig(:data, :range)
return Error::EmptySelection if source_range[:start] == source_range[:end]

Expand Down Expand Up @@ -153,8 +197,6 @@ def refactor_variable

sig { returns(T.any(Interface::CodeAction, Error)) }
def refactor_method
return Error::EmptySelection if @document.source.empty?

source_range = @code_action.dig(:data, :range)
return Error::EmptySelection if source_range[:start] == source_range[:end]

Expand Down Expand Up @@ -206,8 +248,6 @@ def refactor_method
)
end

private

sig { params(range: T::Hash[Symbol, T.untyped], new_text: String).returns(Interface::TextEdit) }
def create_text_edit(range, new_text)
Interface::TextEdit.new(
Expand All @@ -218,6 +258,64 @@ def create_text_edit(range, new_text)
new_text: new_text,
)
end

sig { params(node: Prism::BlockNode, indentation: T.nilable(String)).returns(String) }
def recursively_switch_nested_block_styles(node, indentation)
parameters = node.parameters
body = node.body

# We use the indentation to differentiate between do...end and brace style blocks because only the do...end
# style requires the indentation to build the edit.
#
# If the block is using `do...end` style, we change it to a single line brace block. Newlines are turned into
# semi colons, so that the result is valid Ruby code and still a one liner. If the block is using brace style,
# we do the opposite and turn it into a `do...end` block, making all semi colons into newlines.
source = +""

if indentation
source << "do"
source << " #{parameters.slice}" if parameters
source << "\n#{indentation} "
source << switch_block_body(body, indentation) if body
source << "\n#{indentation}end"
else
source << "{ "
source << "#{parameters.slice} " if parameters
source << switch_block_body(body, nil) if body
source << "}"
end

source
end

sig { params(body: Prism::Node, indentation: T.nilable(String)).returns(String) }
def switch_block_body(body, indentation)
# Check if there are any nested blocks inside of the current block
body_loc = body.location
nested_block = @document.locate_first_within_range(
{
start: { line: body_loc.start_line - 1, character: body_loc.start_column },
end: { line: body_loc.end_line - 1, character: body_loc.end_column },
},
node_types: [Prism::BlockNode],
)

body_content = body.slice.dup

# If there are nested blocks, then we change their style too and we have to mutate the string using the
# relative position in respect to the beginning of the body
if nested_block.is_a?(Prism::BlockNode)
location = nested_block.location
correction_start = location.start_offset - body_loc.start_offset
correction_end = location.end_offset - body_loc.start_offset
next_indentation = indentation ? "#{indentation} " : nil

body_content[correction_start...correction_end] =
recursively_switch_nested_block_styles(nested_block, next_indentation)
end

indentation ? body_content.gsub(";", "\n") : "#{body_content.gsub("\n", ";")} "
end
end
end
end
6 changes: 6 additions & 0 deletions lib/ruby_lsp/requests/code_actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class CodeActions < Request

EXTRACT_TO_VARIABLE_TITLE = "Refactor: Extract Variable"
EXTRACT_TO_METHOD_TITLE = "Refactor: Extract Method"
SWITCH_BLOCK_STYLE_TITLE = "Refactor: Switch block style"

class << self
extend T::Sig
Expand Down Expand Up @@ -69,6 +70,11 @@ def perform
kind: Constant::CodeActionKind::REFACTOR_EXTRACT,
data: { range: @range, uri: @uri.to_s },
)
code_actions << Interface::CodeAction.new(
title: SWITCH_BLOCK_STYLE_TITLE,
kind: Constant::CodeActionKind::REFACTOR_REWRITE,
data: { range: @range, uri: @uri.to_s },
)
end

code_actions
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"params": {
"kind": "refactor.rewrite",
"title": "Refactor: Switch block style",
"data": {
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 0,
"character": 58
}
},
"uri": "file:///fake"
}
},
"result": {
"title": "Refactor: Switch block style",
"edit": {
"documentChanges": [
{
"textDocument": {
"uri": "file:///fake",
"version": null
},
"edits": [
{
"range": {
"start": {
"line": 0,
"character": 26
},
"end": {
"line": 0,
"character": 58
}
},
"newText": "do |a|\n a[\"field\"] == \"expected\"\nend"
}
]
}
]
}
}
}
47 changes: 47 additions & 0 deletions test/expectations/code_action_resolve/nested_block_calls.exp.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"params": {
"kind": "refactor.rewrite",
"title": "Refactor: Switch block style",
"data": {
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 3,
"character": 3
}
},
"uri": "file:///fake"
}
},
"result": {
"title": "Refactor: Switch block style",
"edit": {
"documentChanges": [
{
"textDocument": {
"uri": "file:///fake",
"version": null
},
"edits": [
{
"range": {
"start": {
"line": 0,
"character": 29
},
"end": {
"line": 3,
"character": 3
}
},
"newText": "{ |a| nested_call(fourth_call).each { |b| } }"
}
]
}
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"params": {
"kind": "refactor.rewrite",
"title": "Refactor: Switch block style",
"data": {
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 0,
"character": 74
}
},
"uri": "file:///fake"
}
},
"result": {
"title": "Refactor: Switch block style",
"edit": {
"documentChanges": [
{
"textDocument": {
"uri": "file:///fake",
"version": null
},
"edits": [
{
"range": {
"start": {
"line": 0,
"character": 29
},
"end": {
"line": 0,
"character": 74
}
},
"newText": "do |a|\n nested_call(fourth_call).each do |b|\n \n end\nend"
}
]
}
]
}
}
}
17 changes: 17 additions & 0 deletions test/expectations/code_actions/aref_field.exp.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,23 @@
},
"uri": "file:///fake"
}
},
{
"title": "Refactor: Switch block style",
"kind": "refactor.rewrite",
"data": {
"range": {
"start": {
"line": 2,
"character": 9
},
"end": {
"line": 2,
"character": 13
}
},
"uri": "file:///fake"
}
}
]
}
4 changes: 4 additions & 0 deletions test/fixtures/nested_block_calls.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
method_call(other_call).each do |a|
nested_call(fourth_call).each do |b|
end
end
1 change: 1 addition & 0 deletions test/fixtures/nested_oneline_blocks.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
method_call(other_call).each { |a| nested_call(fourth_call).each { |b| } }
Loading

0 comments on commit b7f033e

Please sign in to comment.