Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add switch block refactor #2372

Merged
merged 3 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
vinistock marked this conversation as resolved.
Show resolved Hide resolved
queue = T.let(@parse_result.value.child_nodes.compact, T::Array[T.nilable(Prism::Node)])
vinistock marked this conversation as resolved.
Show resolved Hide resolved

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
vinistock marked this conversation as resolved.
Show resolved Hide resolved

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"
}
]
}
]
}
}
}
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
Loading