Skip to content

Commit ab544c3

Browse files
rolandcrosby-columntaxrolandcrosby-columntax
authored
Enable "toggle block style" refactor when there's no selection (#3818)
* Enable "toggle block style" without a selection * Add tests for "toggle block style" with no selection
1 parent e3da5af commit ab544c3

16 files changed

+363
-16
lines changed

‎lib/ruby_lsp/requests/code_action_resolve.rb‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,42 @@ def perform
5151
#: -> (Interface::CodeAction)
5252
def switch_block_style
5353
source_range = @code_action.dig(:data, :range)
54-
raise EmptySelectionError, "Invalid selection for refactor" if source_range[:start] == source_range[:end]
54+
if source_range[:start] == source_range[:end]
55+
block_context = @document.locate_node(
56+
source_range[:start],
57+
node_types: [Prism::BlockNode],
58+
)
59+
node = block_context.node
60+
unless node.is_a?(Prism::BlockNode)
61+
raise InvalidTargetRangeError, "Cursor is not inside a block"
62+
end
5563

56-
target = @document.locate_first_within_range(
57-
@code_action.dig(:data, :range),
58-
node_types: [Prism::CallNode],
59-
)
64+
# Find the call node at the block node's start position.
65+
# This should be the call node whose block the cursor is inside of.
66+
call_context = RubyDocument.locate(
67+
@document.ast,
68+
node.location.cached_start_code_units_offset(@document.code_units_cache),
69+
node_types: [Prism::CallNode],
70+
code_units_cache: @document.code_units_cache,
71+
)
72+
target = call_context.node
73+
unless target.is_a?(Prism::CallNode) && target.block == node
74+
raise InvalidTargetRangeError, "Couldn't find an appropriate location to place extracted refactor"
75+
end
76+
else
77+
target = @document.locate_first_within_range(
78+
@code_action.dig(:data, :range),
79+
node_types: [Prism::CallNode],
80+
)
6081

61-
unless target.is_a?(Prism::CallNode)
62-
raise InvalidTargetRangeError, "Couldn't find an appropriate location to place extracted refactor"
63-
end
82+
unless target.is_a?(Prism::CallNode)
83+
raise InvalidTargetRangeError, "Couldn't find an appropriate location to place extracted refactor"
84+
end
6485

65-
node = target.block
66-
unless node.is_a?(Prism::BlockNode)
67-
raise InvalidTargetRangeError, "Couldn't find an appropriate location to place extracted refactor"
86+
node = target.block
87+
unless node.is_a?(Prism::BlockNode)
88+
raise InvalidTargetRangeError, "Couldn't find an appropriate location to place extracted refactor"
89+
end
6890
end
6991

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

‎lib/ruby_lsp/requests/code_actions.rb‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,8 @@ def perform
6363
kind: Constant::CodeActionKind::REFACTOR_EXTRACT,
6464
data: { range: @range, uri: @uri.to_s },
6565
)
66-
code_actions << Interface::CodeAction.new(
67-
title: TOGGLE_BLOCK_STYLE_TITLE,
68-
kind: Constant::CodeActionKind::REFACTOR_REWRITE,
69-
data: { range: @range, uri: @uri.to_s },
70-
)
7166
end
67+
code_actions.concat(toggle_block_style_action)
7268
code_actions.concat(attribute_actions)
7369

7470
code_actions
@@ -113,6 +109,25 @@ def attribute_actions
113109
),
114110
]
115111
end
112+
113+
#: -> Array[Interface::CodeAction]
114+
def toggle_block_style_action
115+
if @range[:start] == @range[:end]
116+
block_context = @document.locate_node(
117+
@range[:start],
118+
node_types: [Prism::BlockNode],
119+
)
120+
return [] unless block_context.node.is_a?(Prism::BlockNode)
121+
end
122+
123+
[
124+
Interface::CodeAction.new(
125+
title: TOGGLE_BLOCK_STYLE_TITLE,
126+
kind: Constant::CodeActionKind::REFACTOR_REWRITE,
127+
data: { range: @range, uri: @uri.to_s },
128+
),
129+
]
130+
end
116131
end
117132
end
118133
end

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"params": {
3+
"kind": "refactor.rewrite",
4+
"title": "Refactor: Toggle block style",
5+
"data": {
6+
"range": {
7+
"start": {
8+
"line": 0,
9+
"character": 29
10+
},
11+
"end": {
12+
"line": 0,
13+
"character": 29
14+
}
15+
},
16+
"uri": "file:///fake"
17+
}
18+
},
19+
"result": {
20+
"title": "Refactor: Toggle 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_without_selection.exp.json‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"params": {
3+
"kind": "refactor.rewrite",
4+
"title": "Refactor: Toggle block style",
5+
"data": {
6+
"range": {
7+
"start": {
8+
"line": 1,
9+
"character": 30
10+
},
11+
"end": {
12+
"line": 1,
13+
"character": 30
14+
}
15+
},
16+
"uri": "file:///fake"
17+
}
18+
},
19+
"result": {
20+
"title": "Refactor: Toggle 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_block_calls_without_selection_inner.exp.json‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"params": {
3+
"kind": "refactor.rewrite",
4+
"title": "Refactor: Toggle block style",
5+
"data": {
6+
"range": {
7+
"start": {
8+
"line": 1,
9+
"character": 37
10+
},
11+
"end": {
12+
"line": 1,
13+
"character": 37
14+
}
15+
},
16+
"uri": "file:///fake"
17+
}
18+
},
19+
"result": {
20+
"title": "Refactor: Toggle 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": 1,
33+
"character": 32
34+
},
35+
"end": {
36+
"line": 2,
37+
"character": 5
38+
}
39+
},
40+
"newText": "{ |b| }"
41+
}
42+
]
43+
}
44+
]
45+
}
46+
}
47+
}

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"params": {
3+
"kind": "refactor.rewrite",
4+
"title": "Refactor: Toggle block style",
5+
"data": {
6+
"range": {
7+
"start": {
8+
"line": 0,
9+
"character": 31
10+
},
11+
"end": {
12+
"line": 0,
13+
"character": 31
14+
}
15+
},
16+
"uri": "file:///fake"
17+
}
18+
},
19+
"result": {
20+
"title": "Refactor: Toggle 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_action_resolve/nested_oneline_blocks_without_selection_inner.exp.json‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"params": {
3+
"kind": "refactor.rewrite",
4+
"title": "Refactor: Toggle block style",
5+
"data": {
6+
"range": {
7+
"start": {
8+
"line": 0,
9+
"character": 68
10+
},
11+
"end": {
12+
"line": 0,
13+
"character": 68
14+
}
15+
},
16+
"uri": "file:///fake"
17+
}
18+
},
19+
"result": {
20+
"title": "Refactor: Toggle 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": 65
34+
},
35+
"end": {
36+
"line": 0,
37+
"character": 72
38+
}
39+
},
40+
"newText": "do |b|\n \n end"
41+
}
42+
]
43+
}
44+
]
45+
}
46+
}
47+
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
{
2+
"params": {
3+
"range": {
4+
"start": {
5+
"line": 0,
6+
"character": 0
7+
},
8+
"end": {
9+
"line": 0,
10+
"character": 0
11+
}
12+
},
13+
"textDocument": {
14+
"uri": "file:///fake",
15+
"version": null
16+
},
17+
"context": {
18+
"diagnostics": []
19+
}
20+
},
21+
"result": []
22+
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
{
2+
"params": {
3+
"range": {
4+
"start": {
5+
"line": 0,
6+
"character": 22
7+
},
8+
"end": {
9+
"line": 0,
10+
"character": 22
11+
}
12+
},
13+
"textDocument": {
14+
"uri": "file:///fake",
15+
"version": null
16+
},
17+
"context": {
18+
"diagnostics": []
19+
}
20+
},
21+
"result": [
22+
{
23+
"title": "Refactor: Toggle block style",
24+
"kind": "refactor.rewrite",
25+
"data": {
26+
"range": {
27+
"start": {
28+
"line": 0,
29+
"character": 22
30+
},
31+
"end": {
32+
"line": 0,
33+
"character": 22
34+
}
35+
},
36+
"uri": "file:///fake"
37+
}
38+
}
39+
]
40+
}

‎test/fixtures/aref_call_aref_assign_without_selection.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
object["attributes"].find { |a| a["field"] == "expected" }["value"] = "changed"

0 commit comments

Comments
 (0)