Skip to content

Commit b7f033e

Browse files
vinistockvinistock
authored
Add switch block refactor (#2372)
* 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
1 parent b6ba7da commit b7f033e

11 files changed

+331
-7
lines changed

‎lib/ruby_lsp/document.rb

+34
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,40 @@ def locate(node, char_position, node_types: [])
223223
NodeContext.new(closest, parent, nesting_nodes, call_node)
224224
end
225225

226+
sig do
227+
params(
228+
range: T::Hash[Symbol, T.untyped],
229+
node_types: T::Array[T.class_of(Prism::Node)],
230+
).returns(T.nilable(Prism::Node))
231+
end
232+
def locate_first_within_range(range, node_types: [])
233+
scanner = create_scanner
234+
start_position = scanner.find_char_position(range[:start])
235+
end_position = scanner.find_char_position(range[:end])
236+
desired_range = (start_position...end_position)
237+
queue = T.let(@parse_result.value.child_nodes.compact, T::Array[T.nilable(Prism::Node)])
238+
239+
until queue.empty?
240+
candidate = queue.shift
241+
242+
# Skip nil child nodes
243+
next if candidate.nil?
244+
245+
# Add the next child_nodes to the queue to be processed. The order here is important! We want to move in the
246+
# same order as the visiting mechanism, which means searching the child nodes before moving on to the next
247+
# sibling
248+
T.unsafe(queue).unshift(*candidate.child_nodes)
249+
250+
# Skip if the current node doesn't cover the desired position
251+
loc = candidate.location
252+
253+
if desired_range.cover?(loc.start_offset...loc.end_offset) &&
254+
(node_types.empty? || node_types.any? { |type| candidate.class == type })
255+
return candidate
256+
end
257+
end
258+
end
259+
226260
sig { returns(SorbetLevel) }
227261
def sorbet_level
228262
sigil = parse_result.magic_comments.find do |comment|

‎lib/ruby_lsp/requests/code_action_resolve.rb

+104-6
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ module Requests
2323
#
2424
class CodeActionResolve < Request
2525
extend T::Sig
26+
include Support::Common
27+
2628
NEW_VARIABLE_NAME = "new_variable"
2729
NEW_METHOD_NAME = "new_method"
2830

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

4648
sig { override.returns(T.any(Interface::CodeAction, Error)) }
4749
def perform
50+
return Error::EmptySelection if @document.source.empty?
51+
4852
case @code_action[:title]
4953
when CodeActions::EXTRACT_TO_VARIABLE_TITLE
5054
refactor_variable
5155
when CodeActions::EXTRACT_TO_METHOD_TITLE
5256
refactor_method
57+
when CodeActions::SWITCH_BLOCK_STYLE_TITLE
58+
switch_block_style
5359
else
5460
Error::UnknownCodeAction
5561
end
5662
end
5763

64+
private
65+
5866
sig { returns(T.any(Interface::CodeAction, Error)) }
59-
def refactor_variable
60-
return Error::EmptySelection if @document.source.empty?
67+
def switch_block_style
68+
source_range = @code_action.dig(:data, :range)
69+
return Error::EmptySelection if source_range[:start] == source_range[:end]
70+
71+
target = @document.locate_first_within_range(
72+
@code_action.dig(:data, :range),
73+
node_types: [Prism::CallNode],
74+
)
75+
76+
return Error::InvalidTargetRange unless target.is_a?(Prism::CallNode)
77+
78+
node = target.block
79+
return Error::InvalidTargetRange unless node.is_a?(Prism::BlockNode)
6180

81+
indentation = " " * target.location.start_column unless node.opening_loc.slice == "do"
82+
83+
Interface::CodeAction.new(
84+
title: CodeActions::SWITCH_BLOCK_STYLE_TITLE,
85+
edit: Interface::WorkspaceEdit.new(
86+
document_changes: [
87+
Interface::TextDocumentEdit.new(
88+
text_document: Interface::OptionalVersionedTextDocumentIdentifier.new(
89+
uri: @code_action.dig(:data, :uri),
90+
version: nil,
91+
),
92+
edits: [
93+
Interface::TextEdit.new(
94+
range: range_from_location(node.location),
95+
new_text: recursively_switch_nested_block_styles(node, indentation),
96+
),
97+
],
98+
),
99+
],
100+
),
101+
)
102+
end
103+
104+
sig { returns(T.any(Interface::CodeAction, Error)) }
105+
def refactor_variable
62106
source_range = @code_action.dig(:data, :range)
63107
return Error::EmptySelection if source_range[:start] == source_range[:end]
64108

@@ -153,8 +197,6 @@ def refactor_variable
153197

154198
sig { returns(T.any(Interface::CodeAction, Error)) }
155199
def refactor_method
156-
return Error::EmptySelection if @document.source.empty?
157-
158200
source_range = @code_action.dig(:data, :range)
159201
return Error::EmptySelection if source_range[:start] == source_range[:end]
160202

@@ -206,8 +248,6 @@ def refactor_method
206248
)
207249
end
208250

209-
private
210-
211251
sig { params(range: T::Hash[Symbol, T.untyped], new_text: String).returns(Interface::TextEdit) }
212252
def create_text_edit(range, new_text)
213253
Interface::TextEdit.new(
@@ -218,6 +258,64 @@ def create_text_edit(range, new_text)
218258
new_text: new_text,
219259
)
220260
end
261+
262+
sig { params(node: Prism::BlockNode, indentation: T.nilable(String)).returns(String) }
263+
def recursively_switch_nested_block_styles(node, indentation)
264+
parameters = node.parameters
265+
body = node.body
266+
267+
# We use the indentation to differentiate between do...end and brace style blocks because only the do...end
268+
# style requires the indentation to build the edit.
269+
#
270+
# If the block is using `do...end` style, we change it to a single line brace block. Newlines are turned into
271+
# semi colons, so that the result is valid Ruby code and still a one liner. If the block is using brace style,
272+
# we do the opposite and turn it into a `do...end` block, making all semi colons into newlines.
273+
source = +""
274+
275+
if indentation
276+
source << "do"
277+
source << " #{parameters.slice}" if parameters
278+
source << "\n#{indentation} "
279+
source << switch_block_body(body, indentation) if body
280+
source << "\n#{indentation}end"
281+
else
282+
source << "{ "
283+
source << "#{parameters.slice} " if parameters
284+
source << switch_block_body(body, nil) if body
285+
source << "}"
286+
end
287+
288+
source
289+
end
290+
291+
sig { params(body: Prism::Node, indentation: T.nilable(String)).returns(String) }
292+
def switch_block_body(body, indentation)
293+
# Check if there are any nested blocks inside of the current block
294+
body_loc = body.location
295+
nested_block = @document.locate_first_within_range(
296+
{
297+
start: { line: body_loc.start_line - 1, character: body_loc.start_column },
298+
end: { line: body_loc.end_line - 1, character: body_loc.end_column },
299+
},
300+
node_types: [Prism::BlockNode],
301+
)
302+
303+
body_content = body.slice.dup
304+
305+
# If there are nested blocks, then we change their style too and we have to mutate the string using the
306+
# relative position in respect to the beginning of the body
307+
if nested_block.is_a?(Prism::BlockNode)
308+
location = nested_block.location
309+
correction_start = location.start_offset - body_loc.start_offset
310+
correction_end = location.end_offset - body_loc.start_offset
311+
next_indentation = indentation ? "#{indentation} " : nil
312+
313+
body_content[correction_start...correction_end] =
314+
recursively_switch_nested_block_styles(nested_block, next_indentation)
315+
end
316+
317+
indentation ? body_content.gsub(";", "\n") : "#{body_content.gsub("\n", ";")} "
318+
end
221319
end
222320
end
223321
end

‎lib/ruby_lsp/requests/code_actions.rb

+6
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class CodeActions < Request
2121

2222
EXTRACT_TO_VARIABLE_TITLE = "Refactor: Extract Variable"
2323
EXTRACT_TO_METHOD_TITLE = "Refactor: Extract Method"
24+
SWITCH_BLOCK_STYLE_TITLE = "Refactor: Switch block style"
2425

2526
class << self
2627
extend T::Sig
@@ -69,6 +70,11 @@ def perform
6970
kind: Constant::CodeActionKind::REFACTOR_EXTRACT,
7071
data: { range: @range, uri: @uri.to_s },
7172
)
73+
code_actions << Interface::CodeAction.new(
74+
title: SWITCH_BLOCK_STYLE_TITLE,
75+
kind: Constant::CodeActionKind::REFACTOR_REWRITE,
76+
data: { range: @range, uri: @uri.to_s },
77+
)
7278
end
7379

7480
code_actions

‎test/expectations/code_action_resolve/aref_call_aref_assign.exp.json

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"params": {
3+
"kind": "refactor.rewrite",
4+
"title": "Refactor: Switch block style",
5+
"data": {
6+
"range": {
7+
"start": {
8+
"line": 0,
9+
"character": 0
10+
},
11+
"end": {
12+
"line": 0,
13+
"character": 58
14+
}
15+
},
16+
"uri": "file:///fake"
17+
}
18+
},
19+
"result": {
20+
"title": "Refactor: Switch block style",
21+
"edit": {
22+
"documentChanges": [
23+
{
24+
"textDocument": {
25+
"uri": "file:///fake",
26+
"version": null
27+
},
28+
"edits": [
29+
{
30+
"range": {
31+
"start": {
32+
"line": 0,
33+
"character": 26
34+
},
35+
"end": {
36+
"line": 0,
37+
"character": 58
38+
}
39+
},
40+
"newText": "do |a|\n a[\"field\"] == \"expected\"\nend"
41+
}
42+
]
43+
}
44+
]
45+
}
46+
}
47+
}

‎test/expectations/code_action_resolve/nested_block_calls.exp.json

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"params": {
3+
"kind": "refactor.rewrite",
4+
"title": "Refactor: Switch block style",
5+
"data": {
6+
"range": {
7+
"start": {
8+
"line": 0,
9+
"character": 0
10+
},
11+
"end": {
12+
"line": 3,
13+
"character": 3
14+
}
15+
},
16+
"uri": "file:///fake"
17+
}
18+
},
19+
"result": {
20+
"title": "Refactor: Switch block style",
21+
"edit": {
22+
"documentChanges": [
23+
{
24+
"textDocument": {
25+
"uri": "file:///fake",
26+
"version": null
27+
},
28+
"edits": [
29+
{
30+
"range": {
31+
"start": {
32+
"line": 0,
33+
"character": 29
34+
},
35+
"end": {
36+
"line": 3,
37+
"character": 3
38+
}
39+
},
40+
"newText": "{ |a| nested_call(fourth_call).each { |b| } }"
41+
}
42+
]
43+
}
44+
]
45+
}
46+
}
47+
}

‎test/expectations/code_action_resolve/nested_oneline_blocks.exp.json

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"params": {
3+
"kind": "refactor.rewrite",
4+
"title": "Refactor: Switch block style",
5+
"data": {
6+
"range": {
7+
"start": {
8+
"line": 0,
9+
"character": 0
10+
},
11+
"end": {
12+
"line": 0,
13+
"character": 74
14+
}
15+
},
16+
"uri": "file:///fake"
17+
}
18+
},
19+
"result": {
20+
"title": "Refactor: Switch block style",
21+
"edit": {
22+
"documentChanges": [
23+
{
24+
"textDocument": {
25+
"uri": "file:///fake",
26+
"version": null
27+
},
28+
"edits": [
29+
{
30+
"range": {
31+
"start": {
32+
"line": 0,
33+
"character": 29
34+
},
35+
"end": {
36+
"line": 0,
37+
"character": 74
38+
}
39+
},
40+
"newText": "do |a|\n nested_call(fourth_call).each do |b|\n \n end\nend"
41+
}
42+
]
43+
}
44+
]
45+
}
46+
}
47+
}

‎test/expectations/code_actions/aref_field.exp.json

+17
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,23 @@
5252
},
5353
"uri": "file:///fake"
5454
}
55+
},
56+
{
57+
"title": "Refactor: Switch block style",
58+
"kind": "refactor.rewrite",
59+
"data": {
60+
"range": {
61+
"start": {
62+
"line": 2,
63+
"character": 9
64+
},
65+
"end": {
66+
"line": 2,
67+
"character": 13
68+
}
69+
},
70+
"uri": "file:///fake"
71+
}
5572
}
5673
]
5774
}

‎test/fixtures/nested_block_calls.rb

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
method_call(other_call).each do |a|
2+
nested_call(fourth_call).each do |b|
3+
end
4+
end

‎test/fixtures/nested_oneline_blocks.rb

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
method_call(other_call).each { |a| nested_call(fourth_call).each { |b| } }

0 commit comments

Comments
 (0)