diff --git a/lib/ruby_lsp/document.rb b/lib/ruby_lsp/document.rb index e57ad3de8..f17d439a3 100644 --- a/lib/ruby_lsp/document.rb +++ b/lib/ruby_lsp/document.rb @@ -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| diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb index 962c3f2a5..68101d4a5 100644 --- a/lib/ruby_lsp/requests/code_action_resolve.rb +++ b/lib/ruby_lsp/requests/code_action_resolve.rb @@ -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" @@ -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] @@ -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] @@ -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( @@ -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 diff --git a/lib/ruby_lsp/requests/code_actions.rb b/lib/ruby_lsp/requests/code_actions.rb index d710501fd..f60aec7cf 100644 --- a/lib/ruby_lsp/requests/code_actions.rb +++ b/lib/ruby_lsp/requests/code_actions.rb @@ -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 @@ -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 diff --git a/test/expectations/code_action_resolve/aref_call_aref_assign.exp.json b/test/expectations/code_action_resolve/aref_call_aref_assign.exp.json new file mode 100644 index 000000000..d9c668996 --- /dev/null +++ b/test/expectations/code_action_resolve/aref_call_aref_assign.exp.json @@ -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" + } + ] + } + ] + } + } +} diff --git a/test/expectations/code_action_resolve/nested_block_calls.exp.json b/test/expectations/code_action_resolve/nested_block_calls.exp.json new file mode 100644 index 000000000..ef1a59da8 --- /dev/null +++ b/test/expectations/code_action_resolve/nested_block_calls.exp.json @@ -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| } }" + } + ] + } + ] + } + } +} diff --git a/test/expectations/code_action_resolve/nested_oneline_blocks.exp.json b/test/expectations/code_action_resolve/nested_oneline_blocks.exp.json new file mode 100644 index 000000000..dfbe92ded --- /dev/null +++ b/test/expectations/code_action_resolve/nested_oneline_blocks.exp.json @@ -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" + } + ] + } + ] + } + } +} diff --git a/test/expectations/code_actions/aref_field.exp.json b/test/expectations/code_actions/aref_field.exp.json index 77e89b68f..4e7478c73 100644 --- a/test/expectations/code_actions/aref_field.exp.json +++ b/test/expectations/code_actions/aref_field.exp.json @@ -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" + } } ] } diff --git a/test/fixtures/nested_block_calls.rb b/test/fixtures/nested_block_calls.rb new file mode 100644 index 000000000..43de2c3b0 --- /dev/null +++ b/test/fixtures/nested_block_calls.rb @@ -0,0 +1,4 @@ +method_call(other_call).each do |a| + nested_call(fourth_call).each do |b| + end +end diff --git a/test/fixtures/nested_oneline_blocks.rb b/test/fixtures/nested_oneline_blocks.rb new file mode 100644 index 000000000..62caf97a7 --- /dev/null +++ b/test/fixtures/nested_oneline_blocks.rb @@ -0,0 +1 @@ +method_call(other_call).each { |a| nested_call(fourth_call).each { |b| } } diff --git a/test/requests/code_actions_expectations_test.rb b/test/requests/code_actions_expectations_test.rb index 4cf9376b3..f0c6be9c6 100644 --- a/test/requests/code_actions_expectations_test.rb +++ b/test/requests/code_actions_expectations_test.rb @@ -76,7 +76,7 @@ def code_action_for_diagnostic(diagnostic) def code_action_for_refactor(refactor) LanguageServer::Protocol::Interface::CodeAction.new( title: refactor["title"], - kind: LanguageServer::Protocol::Constant::CodeActionKind::REFACTOR_EXTRACT, + kind: refactor["kind"], data: { range: refactor.dig("data", "range"), uri: refactor.dig("data", "uri"), diff --git a/test/ruby_document_test.rb b/test/ruby_document_test.rb index 1aca63b30..7571cd6a0 100644 --- a/test/ruby_document_test.rb +++ b/test/ruby_document_test.rb @@ -716,6 +716,29 @@ def qux assert_equal("qux", node_context.surrounding_method) end + def test_locate_first_within_range + document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: URI("file:///foo/bar.rb")) + method_call(other_call).each do |a| + nested_call(fourth_call).each do |b| + end + end + RUBY + + target = document.locate_first_within_range( + { start: { line: 0, character: 0 }, end: { line: 3, character: 3 } }, + node_types: [Prism::CallNode], + ) + + assert_equal("each", T.cast(target, Prism::CallNode).message) + + target = document.locate_first_within_range( + { start: { line: 1, character: 2 }, end: { line: 2, character: 5 } }, + node_types: [Prism::CallNode], + ) + + assert_equal("each", T.cast(target, Prism::CallNode).message) + end + private def assert_error_edit(actual, error_range)