From 8adc3cd4e9d5c2b35b20ca2524ce38d26c777568 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Sun, 20 Oct 2024 18:20:19 +0530 Subject: [PATCH 1/9] code actions for attribute accessor creation --- lib/ruby_lsp/requests/code_action_resolve.rb | 91 +++++++++++++++++++ lib/ruby_lsp/requests/code_actions.rb | 18 ++++ .../create_attribute_accessor.json | 47 ++++++++++ .../create_attribute_reader.exp.json | 47 ++++++++++ .../create_attributer_writer.exp.json | 47 ++++++++++ .../code_actions/aref_field.exp.json | 51 +++++++++++ test/fixtures/create_attribute_accessor.rb | 5 + test/fixtures/create_attribute_reader.rb | 5 + test/fixtures/create_attribute_writer.rb | 7 ++ .../code_actions_expectations_test.rb | 16 +++- 10 files changed, 333 insertions(+), 1 deletion(-) create mode 100644 test/expectations/code_action_resolve/create_attribute_accessor.json create mode 100644 test/expectations/code_action_resolve/create_attribute_reader.exp.json create mode 100644 test/expectations/code_action_resolve/create_attributer_writer.exp.json create mode 100644 test/fixtures/create_attribute_accessor.rb create mode 100644 test/fixtures/create_attribute_reader.rb create mode 100644 test/fixtures/create_attribute_writer.rb diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb index 9420d832e8..ead16e55b6 100644 --- a/lib/ruby_lsp/requests/code_action_resolve.rb +++ b/lib/ruby_lsp/requests/code_action_resolve.rb @@ -42,6 +42,10 @@ def perform refactor_method when CodeActions::TOGGLE_BLOCK_STYLE_TITLE switch_block_style + when CodeActions::CREATE_ATTRIBUTE_READER, + CodeActions::CREATE_ATTRIBUTE_WRITER, + CodeActions::CREATE_ATTRIBUTE_ACCESSOR + create_attribute_accessor else Error::UnknownCodeAction end @@ -325,6 +329,93 @@ def switch_block_body(body, indentation) indentation ? body_content.gsub(";", "\n") : "#{body_content.gsub("\n", ";")} " end + + sig { returns(T.any(Interface::CodeAction, Error)) } + def create_attribute_accessor + source_range = @code_action.dig(:data, :range) + return Error::EmptySelection if source_range[:start] == source_range[:end] + + node = @document.locate_first_within_range( + @code_action.dig(:data, :range), + node_types: [ + Prism::InstanceVariableAndWriteNode, + Prism::InstanceVariableOperatorWriteNode, + Prism::InstanceVariableOrWriteNode, + Prism::InstanceVariableReadNode, + Prism::InstanceVariableTargetNode, + Prism::InstanceVariableWriteNode, + ], + ) + return Error::EmptySelection if node.nil? + + node = T.cast( + node, + T.any( + Prism::InstanceVariableAndWriteNode, + Prism::InstanceVariableOperatorWriteNode, + Prism::InstanceVariableOrWriteNode, + Prism::InstanceVariableReadNode, + Prism::InstanceVariableTargetNode, + Prism::InstanceVariableWriteNode, + ), + ) + + scanner = @document.create_scanner + start_index = scanner.find_char_position( + line: node.location.start_line, + character: node.location.start_character_column, + ) + node_context = RubyDocument.locate( + @document.parse_result.value, + start_index, + node_types: [ + Prism::ClassNode, + Prism::ModuleNode, + Prism::SingletonClassNode, + ], + code_units_cache: @document.code_units_cache, + ) + closest_node = node_context.node + return Error::InvalidTargetRange if closest_node.nil? + + attribute_name = node.name[1..] + indentation = " " * (closest_node.location.start_column + 2) + attribute_accessor_source = T.must( + ( + case @code_action[:title] + when CodeActions::CREATE_ATTRIBUTE_READER + "#{indentation}attr_reader :#{attribute_name}\n\n" + when CodeActions::CREATE_ATTRIBUTE_WRITER + "#{indentation}attr_writer :#{attribute_name}\n\n" + when CodeActions::CREATE_ATTRIBUTE_ACCESSOR + "#{indentation}attr_accessor :#{attribute_name}\n\n" + end + ), + ) + + target_start_line = closest_node.location.start_line + target_range = { + start: { line: target_start_line, character: 0 }, + end: { line: target_start_line, character: 0 }, + } + + Interface::CodeAction.new( + title: @code_action[:title], + edit: Interface::WorkspaceEdit.new( + document_changes: [ + Interface::TextDocumentEdit.new( + text_document: Interface::OptionalVersionedTextDocumentIdentifier.new( + uri: @code_action.dig(:data, :uri), + version: nil, + ), + edits: [ + create_text_edit(target_range, attribute_accessor_source), + ], + ), + ], + ), + ) + end end end end diff --git a/lib/ruby_lsp/requests/code_actions.rb b/lib/ruby_lsp/requests/code_actions.rb index 66fc7d3e98..c50a61e465 100644 --- a/lib/ruby_lsp/requests/code_actions.rb +++ b/lib/ruby_lsp/requests/code_actions.rb @@ -12,6 +12,9 @@ class CodeActions < Request EXTRACT_TO_VARIABLE_TITLE = "Refactor: Extract Variable" EXTRACT_TO_METHOD_TITLE = "Refactor: Extract Method" TOGGLE_BLOCK_STYLE_TITLE = "Refactor: Toggle block style" + CREATE_ATTRIBUTE_READER = "Create Attribute Reader" + CREATE_ATTRIBUTE_WRITER = "Create Attribute Writer" + CREATE_ATTRIBUTE_ACCESSOR = "Create Attribute Accessor" class << self extend T::Sig @@ -65,6 +68,21 @@ def perform kind: Constant::CodeActionKind::REFACTOR_REWRITE, data: { range: @range, uri: @uri.to_s }, ) + code_actions << Interface::CodeAction.new( + title: CREATE_ATTRIBUTE_READER, + kind: Constant::CodeActionKind::EMPTY, + data: { range: @range, uri: @uri.to_s }, + ) + code_actions << Interface::CodeAction.new( + title: CREATE_ATTRIBUTE_WRITER, + kind: Constant::CodeActionKind::EMPTY, + data: { range: @range, uri: @uri.to_s }, + ) + code_actions << Interface::CodeAction.new( + title: CREATE_ATTRIBUTE_ACCESSOR, + kind: Constant::CodeActionKind::EMPTY, + data: { range: @range, uri: @uri.to_s }, + ) end code_actions diff --git a/test/expectations/code_action_resolve/create_attribute_accessor.json b/test/expectations/code_action_resolve/create_attribute_accessor.json new file mode 100644 index 0000000000..5848dd3d82 --- /dev/null +++ b/test/expectations/code_action_resolve/create_attribute_accessor.json @@ -0,0 +1,47 @@ +{ + "params": { + "kind": "", + "title": "Create Attribute Accessor", + "data": { + "range": { + "start": { + "line": 2, + "character": 0 + }, + "end": { + "line": 2, + "character": 16 + } + }, + "uri": "file:///fake" + } + }, + "result": { + "title": "Create Attribute Accessor", + "edit": { + "documentChanges": [ + { + "textDocument": { + "uri": "file:///fake", + "version": null + }, + "edits": [ + { + "range": { + "start": { + "line": 1, + "character": 0 + }, + "end": { + "line": 1, + "character": 0 + } + }, + "newText": " attr_accessor :foo\n\n" + } + ] + } + ] + } + } +} diff --git a/test/expectations/code_action_resolve/create_attribute_reader.exp.json b/test/expectations/code_action_resolve/create_attribute_reader.exp.json new file mode 100644 index 0000000000..7a19c7f241 --- /dev/null +++ b/test/expectations/code_action_resolve/create_attribute_reader.exp.json @@ -0,0 +1,47 @@ +{ + "params": { + "kind": "", + "title": "Create Attribute Reader", + "data": { + "range": { + "start": { + "line": 2, + "character": 0 + }, + "end": { + "line": 2, + "character": 16 + } + }, + "uri": "file:///fake" + } + }, + "result": { + "title": "Create Attribute Reader", + "edit": { + "documentChanges": [ + { + "textDocument": { + "uri": "file:///fake", + "version": null + }, + "edits": [ + { + "range": { + "start": { + "line": 1, + "character": 0 + }, + "end": { + "line": 1, + "character": 0 + } + }, + "newText": " attr_reader :foo\n\n" + } + ] + } + ] + } + } +} diff --git a/test/expectations/code_action_resolve/create_attributer_writer.exp.json b/test/expectations/code_action_resolve/create_attributer_writer.exp.json new file mode 100644 index 0000000000..cc6f78f05d --- /dev/null +++ b/test/expectations/code_action_resolve/create_attributer_writer.exp.json @@ -0,0 +1,47 @@ +{ + "params": { + "kind": "", + "title": "Create Attribute Writer", + "data": { + "range": { + "start": { + "line": 3, + "character": 0 + }, + "end": { + "line": 4, + "character": 0 + } + }, + "uri": "file:///fake" + } + }, + "result": { + "title": "Create Attribute Writer", + "edit": { + "documentChanges": [ + { + "textDocument": { + "uri": "file:///fake", + "version": null + }, + "edits": [ + { + "range": { + "start": { + "line": 2, + "character": 0 + }, + "end": { + "line": 2, + "character": 0 + } + }, + "newText": " attr_accessor :foo\n\n" + } + ] + } + ] + } + } +} diff --git a/test/expectations/code_actions/aref_field.exp.json b/test/expectations/code_actions/aref_field.exp.json index 22fbe7a4b7..38f36d8739 100644 --- a/test/expectations/code_actions/aref_field.exp.json +++ b/test/expectations/code_actions/aref_field.exp.json @@ -69,6 +69,57 @@ }, "uri": "file:///fake" } + }, + { + "title": "Create Attribute Reader", + "kind": "", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } + }, + { + "title": "Create Attribute Writer", + "kind": "", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } + }, + { + "title": "Create Attribute Accessor", + "kind": "", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } } ] } diff --git a/test/fixtures/create_attribute_accessor.rb b/test/fixtures/create_attribute_accessor.rb new file mode 100644 index 0000000000..8859cec27c --- /dev/null +++ b/test/fixtures/create_attribute_accessor.rb @@ -0,0 +1,5 @@ +module Foo + def bar + @foo = "foo" + end +end diff --git a/test/fixtures/create_attribute_reader.rb b/test/fixtures/create_attribute_reader.rb new file mode 100644 index 0000000000..5833bef116 --- /dev/null +++ b/test/fixtures/create_attribute_reader.rb @@ -0,0 +1,5 @@ +class Foo + def bar + @foo = "foo" + end +end diff --git a/test/fixtures/create_attribute_writer.rb b/test/fixtures/create_attribute_writer.rb new file mode 100644 index 0000000000..3798ad029f --- /dev/null +++ b/test/fixtures/create_attribute_writer.rb @@ -0,0 +1,7 @@ +class Foo + class << self + def bar + @foo = "foo" + end + end +end diff --git a/test/requests/code_actions_expectations_test.rb b/test/requests/code_actions_expectations_test.rb index b53ba74c1e..be198d6e59 100644 --- a/test/requests/code_actions_expectations_test.rb +++ b/test/requests/code_actions_expectations_test.rb @@ -54,7 +54,10 @@ def map_actions(expectation) refactors = expectation .select { |action| action["kind"].start_with?("refactor") } .map { |action| code_action_for_refactor(action) } - result = [*quickfixes, *refactors] + empty_kind = expectation + .select { |action| action["kind"] == "" } + .map { |action| code_action_for_refactor(action) } + result = [*quickfixes, *refactors, *empty_kind] JSON.parse(result.to_json) end @@ -88,4 +91,15 @@ def code_action_for_refactor(refactor) }, ) end + + def code_action_for_empty_kind(expectations) + LanguageServer::Protocol::Interface::CodeAction.new( + title: expectations["title"], + kind: expectations["kind"], + data: { + range: expectations.dig("data", "range"), + uri: expectations.dig("data", "uri"), + }, + ) + end end From ef3b70330c420c2998211e4051e765b2d101eaff Mon Sep 17 00:00:00 2001 From: rogancodes Date: Mon, 20 Jan 2025 23:09:02 +0530 Subject: [PATCH 2/9] Handle partial selection based on range start postion --- lib/ruby_lsp/requests/code_action_resolve.rb | 32 +++-- lib/ruby_lsp/requests/code_actions.rb | 57 ++++++-- .../code_actions/aref_field.exp.json | 51 ------- .../create_attribute_accessor.exp.json | 125 ++++++++++++++++++ 4 files changed, 192 insertions(+), 73 deletions(-) create mode 100644 test/expectations/code_actions/create_attribute_accessor.exp.json diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb index ead16e55b6..9cd19862cf 100644 --- a/lib/ruby_lsp/requests/code_action_resolve.rb +++ b/lib/ruby_lsp/requests/code_action_resolve.rb @@ -337,16 +337,21 @@ def create_attribute_accessor node = @document.locate_first_within_range( @code_action.dig(:data, :range), - node_types: [ - Prism::InstanceVariableAndWriteNode, - Prism::InstanceVariableOperatorWriteNode, - Prism::InstanceVariableOrWriteNode, - Prism::InstanceVariableReadNode, - Prism::InstanceVariableTargetNode, - Prism::InstanceVariableWriteNode, - ], + node_types: CodeActions::INSTANCE_VARIABLE_NODES, ) - return Error::EmptySelection if node.nil? + + if node.nil? + start_index, _ = @document.find_index_by_position(source_range[:start], source_range[:end]) + node_context = RubyDocument.locate( + @document.parse_result.value, + start_index, + node_types: CodeActions::INSTANCE_VARIABLE_NODES, + code_units_cache: @document.code_units_cache, + ) + node = node_context.node + + return Error::EmptySelection unless CodeActions::INSTANCE_VARIABLE_NODES.include?(node.class) + end node = T.cast( node, @@ -360,10 +365,11 @@ def create_attribute_accessor ), ) - scanner = @document.create_scanner - start_index = scanner.find_char_position( - line: node.location.start_line, - character: node.location.start_character_column, + start_index, _ = @document.find_index_by_position( + { + line: node.location.start_line, + character: node.location.start_character_column, + }, ) node_context = RubyDocument.locate( @document.parse_result.value, diff --git a/lib/ruby_lsp/requests/code_actions.rb b/lib/ruby_lsp/requests/code_actions.rb index c50a61e465..184ec59c57 100644 --- a/lib/ruby_lsp/requests/code_actions.rb +++ b/lib/ruby_lsp/requests/code_actions.rb @@ -16,6 +16,18 @@ class CodeActions < Request CREATE_ATTRIBUTE_WRITER = "Create Attribute Writer" CREATE_ATTRIBUTE_ACCESSOR = "Create Attribute Accessor" + INSTANCE_VARIABLE_NODES = T.let( + [ + Prism::InstanceVariableAndWriteNode, + Prism::InstanceVariableOperatorWriteNode, + Prism::InstanceVariableOrWriteNode, + Prism::InstanceVariableReadNode, + Prism::InstanceVariableTargetNode, + Prism::InstanceVariableWriteNode, + ], + T::Array[T.class_of(Prism::Node)], + ) + class << self extend T::Sig @@ -68,24 +80,51 @@ def perform kind: Constant::CodeActionKind::REFACTOR_REWRITE, data: { range: @range, uri: @uri.to_s }, ) - code_actions << Interface::CodeAction.new( + code_actions.concat(attribute_actions) + end + + code_actions + end + + private + + sig { returns(T::Array[Interface::CodeAction]) } + def attribute_actions + return [] unless @document.is_a?(RubyDocument) + + node = @document.locate_first_within_range( + @range, + node_types: INSTANCE_VARIABLE_NODES, + ) + + if node.nil? + start_index, _ = @document.find_index_by_position(@range[:start], @range[:end]) + node_context = RubyDocument.locate( + @document.parse_result.value, + start_index, + node_types: INSTANCE_VARIABLE_NODES, + code_units_cache: @document.code_units_cache, + ) + return [] unless INSTANCE_VARIABLE_NODES.include?(node_context.node.class) + end + + [ + Interface::CodeAction.new( title: CREATE_ATTRIBUTE_READER, kind: Constant::CodeActionKind::EMPTY, data: { range: @range, uri: @uri.to_s }, - ) - code_actions << Interface::CodeAction.new( + ), + Interface::CodeAction.new( title: CREATE_ATTRIBUTE_WRITER, kind: Constant::CodeActionKind::EMPTY, data: { range: @range, uri: @uri.to_s }, - ) - code_actions << Interface::CodeAction.new( + ), + Interface::CodeAction.new( title: CREATE_ATTRIBUTE_ACCESSOR, kind: Constant::CodeActionKind::EMPTY, data: { range: @range, uri: @uri.to_s }, - ) - end - - code_actions + ), + ] end end end diff --git a/test/expectations/code_actions/aref_field.exp.json b/test/expectations/code_actions/aref_field.exp.json index 38f36d8739..22fbe7a4b7 100644 --- a/test/expectations/code_actions/aref_field.exp.json +++ b/test/expectations/code_actions/aref_field.exp.json @@ -69,57 +69,6 @@ }, "uri": "file:///fake" } - }, - { - "title": "Create Attribute Reader", - "kind": "", - "data": { - "range": { - "start": { - "line": 2, - "character": 9 - }, - "end": { - "line": 2, - "character": 13 - } - }, - "uri": "file:///fake" - } - }, - { - "title": "Create Attribute Writer", - "kind": "", - "data": { - "range": { - "start": { - "line": 2, - "character": 9 - }, - "end": { - "line": 2, - "character": 13 - } - }, - "uri": "file:///fake" - } - }, - { - "title": "Create Attribute Accessor", - "kind": "", - "data": { - "range": { - "start": { - "line": 2, - "character": 9 - }, - "end": { - "line": 2, - "character": 13 - } - }, - "uri": "file:///fake" - } } ] } diff --git a/test/expectations/code_actions/create_attribute_accessor.exp.json b/test/expectations/code_actions/create_attribute_accessor.exp.json new file mode 100644 index 0000000000..38f36d8739 --- /dev/null +++ b/test/expectations/code_actions/create_attribute_accessor.exp.json @@ -0,0 +1,125 @@ +{ + "params": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "textDocument": { + "uri": "file:///fake", + "version": null + }, + "context": { + "diagnostics": [] + } + }, + "result": [ + { + "title": "Refactor: Extract Variable", + "kind": "refactor.extract", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } + }, + { + "title": "Refactor: Extract Method", + "kind": "refactor.extract", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } + }, + { + "title": "Refactor: Toggle block style", + "kind": "refactor.rewrite", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } + }, + { + "title": "Create Attribute Reader", + "kind": "", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } + }, + { + "title": "Create Attribute Writer", + "kind": "", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } + }, + { + "title": "Create Attribute Accessor", + "kind": "", + "data": { + "range": { + "start": { + "line": 2, + "character": 9 + }, + "end": { + "line": 2, + "character": 13 + } + }, + "uri": "file:///fake" + } + } + ] +} From 379f5a3a3d50fbdd3e80cbbf4925c5366eaa1154 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Tue, 21 Jan 2025 00:12:53 +0530 Subject: [PATCH 3/9] removed the redundant method added in the expectation test --- .../requests/code_actions_expectations_test.rb | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/test/requests/code_actions_expectations_test.rb b/test/requests/code_actions_expectations_test.rb index be198d6e59..35ab594758 100644 --- a/test/requests/code_actions_expectations_test.rb +++ b/test/requests/code_actions_expectations_test.rb @@ -52,12 +52,9 @@ def map_actions(expectation) .select { |action| action["kind"] == "quickfix" } .map { |action| code_action_for_diagnostic(action) } refactors = expectation - .select { |action| action["kind"].start_with?("refactor") } + .select { |action| action["kind"].start_with?("refactor") || action["kind"] == "" } .map { |action| code_action_for_refactor(action) } - empty_kind = expectation - .select { |action| action["kind"] == "" } - .map { |action| code_action_for_refactor(action) } - result = [*quickfixes, *refactors, *empty_kind] + result = [*quickfixes, *refactors] JSON.parse(result.to_json) end @@ -91,15 +88,4 @@ def code_action_for_refactor(refactor) }, ) end - - def code_action_for_empty_kind(expectations) - LanguageServer::Protocol::Interface::CodeAction.new( - title: expectations["title"], - kind: expectations["kind"], - data: { - range: expectations.dig("data", "range"), - uri: expectations.dig("data", "uri"), - }, - ) - end end From 78a6edf8afbaaa562efb24821da12ead6c28cac0 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Fri, 24 Jan 2025 19:03:52 +0530 Subject: [PATCH 4/9] Handle default args end position based on source --- test/requests/code_actions_expectations_test.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/requests/code_actions_expectations_test.rb b/test/requests/code_actions_expectations_test.rb index 35ab594758..8ea10affe8 100644 --- a/test/requests/code_actions_expectations_test.rb +++ b/test/requests/code_actions_expectations_test.rb @@ -8,7 +8,7 @@ class CodeActionsExpectationsTest < ExpectationsTestRunner expectations_tests RubyLsp::Requests::CodeActions, "code_actions" def run_expectations(source) - params = @__params&.any? ? @__params : default_args + params = @__params&.any? ? @__params : default_args(source) document = RubyLsp::RubyDocument.new( source: source, version: 1, @@ -36,10 +36,11 @@ def assert_expectations(source, expected) private - def default_args + def default_args(source) + end_position = source.lines.count > 1 ? { line: 1, character: 1 } : { line: 0, character: 1 } { range: { - start: { line: 0, character: 0 }, end: { line: 1, character: 1 }, + start: { line: 0, character: 0 }, end: end_position, }, context: { diagnostics: [], From 466dfac4a6c69d211111a3971195ab4c03b3510c Mon Sep 17 00:00:00 2001 From: rogancodes Date: Sat, 25 Jan 2025 09:29:17 +0530 Subject: [PATCH 5/9] Fixed the range parameter in code actions test --- vscode/src/test/suite/client.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vscode/src/test/suite/client.test.ts b/vscode/src/test/suite/client.test.ts index c9a0866b19..3f1331da31 100644 --- a/vscode/src/test/suite/client.test.ts +++ b/vscode/src/test/suite/client.test.ts @@ -545,13 +545,13 @@ suite("Client", () => { textDocument: { uri: documentUri.toString(), }, - range: { start: { line: 2 }, end: { line: 4 } }, + range: { start: { line: 0 }, end: { line: 0 } }, context: { diagnostics: [ { range: { - start: { line: 2, character: 0 }, - end: { line: 2, character: 0 }, + start: { line: 0, character: 0 }, + end: { line: 1, character: 2 }, }, message: "Layout/EmptyLines: Extra blank line detected.", data: { From 353e6a32070c74147d5ab4b4160159bfb4d5f00c Mon Sep 17 00:00:00 2001 From: rogancodes Date: Thu, 30 Jan 2025 12:16:37 +0530 Subject: [PATCH 6/9] addressed review comments --- lib/ruby_lsp/requests/code_action_resolve.rb | 11 +++-- lib/ruby_lsp/requests/code_actions.rb | 12 +++-- ...son => create_attribute_accessor.exp.json} | 2 +- ...ccessor_without_surrounding_class.exp.json | 47 +++++++++++++++++++ ....json => create_attribute_writer.exp.json} | 2 +- ...bute_accessor_without_surrounding_class.rb | 1 + 6 files changed, 65 insertions(+), 10 deletions(-) rename test/expectations/code_action_resolve/{create_attribute_accessor.json => create_attribute_accessor.exp.json} (94%) create mode 100644 test/expectations/code_action_resolve/create_attribute_accessor_without_surrounding_class.exp.json rename test/expectations/code_action_resolve/{create_attributer_writer.exp.json => create_attribute_writer.exp.json} (94%) create mode 100644 test/fixtures/create_attribute_accessor_without_surrounding_class.rb diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb index 9cd19862cf..15cfd95e7c 100644 --- a/lib/ruby_lsp/requests/code_action_resolve.rb +++ b/lib/ruby_lsp/requests/code_action_resolve.rb @@ -333,11 +333,14 @@ def switch_block_body(body, indentation) sig { returns(T.any(Interface::CodeAction, Error)) } def create_attribute_accessor source_range = @code_action.dig(:data, :range) - return Error::EmptySelection if source_range[:start] == source_range[:end] - node = @document.locate_first_within_range( - @code_action.dig(:data, :range), - node_types: CodeActions::INSTANCE_VARIABLE_NODES, + node = ( + if source_range[:start] != source_range[:end] + @document.locate_first_within_range( + @code_action.dig(:data, :range), + node_types: CodeActions::INSTANCE_VARIABLE_NODES, + ) + end ) if node.nil? diff --git a/lib/ruby_lsp/requests/code_actions.rb b/lib/ruby_lsp/requests/code_actions.rb index 184ec59c57..d1b15ab2a8 100644 --- a/lib/ruby_lsp/requests/code_actions.rb +++ b/lib/ruby_lsp/requests/code_actions.rb @@ -80,8 +80,8 @@ def perform kind: Constant::CodeActionKind::REFACTOR_REWRITE, data: { range: @range, uri: @uri.to_s }, ) - code_actions.concat(attribute_actions) end + code_actions.concat(attribute_actions) code_actions end @@ -92,9 +92,13 @@ def perform def attribute_actions return [] unless @document.is_a?(RubyDocument) - node = @document.locate_first_within_range( - @range, - node_types: INSTANCE_VARIABLE_NODES, + node = ( + if @range.dig(:start) != @range.dig(:end) + @document.locate_first_within_range( + @range, + node_types: INSTANCE_VARIABLE_NODES, + ) + end ) if node.nil? diff --git a/test/expectations/code_action_resolve/create_attribute_accessor.json b/test/expectations/code_action_resolve/create_attribute_accessor.exp.json similarity index 94% rename from test/expectations/code_action_resolve/create_attribute_accessor.json rename to test/expectations/code_action_resolve/create_attribute_accessor.exp.json index 5848dd3d82..f7e830d89c 100644 --- a/test/expectations/code_action_resolve/create_attribute_accessor.json +++ b/test/expectations/code_action_resolve/create_attribute_accessor.exp.json @@ -37,7 +37,7 @@ "character": 0 } }, - "newText": " attr_accessor :foo\n\n" + "newText": " attr_accessor :foo\n\n" } ] } diff --git a/test/expectations/code_action_resolve/create_attribute_accessor_without_surrounding_class.exp.json b/test/expectations/code_action_resolve/create_attribute_accessor_without_surrounding_class.exp.json new file mode 100644 index 0000000000..1fa7287dec --- /dev/null +++ b/test/expectations/code_action_resolve/create_attribute_accessor_without_surrounding_class.exp.json @@ -0,0 +1,47 @@ +{ + "params": { + "kind": "", + "title": "Create Attribute Accessor", + "data": { + "range": { + "start": { + "line": 0, + "character": 1 + }, + "end": { + "line": 0, + "character": 1 + } + }, + "uri": "file:///fake" + } + }, + "result": { + "title": "Create Attribute Accessor", + "edit": { + "documentChanges": [ + { + "textDocument": { + "uri": "file:///fake", + "version": null + }, + "edits": [ + { + "range": { + "start": { + "line": 1, + "character": 0 + }, + "end": { + "line": 1, + "character": 0 + } + }, + "newText": " attr_accessor :foo\n\n" + } + ] + } + ] + } + } +} diff --git a/test/expectations/code_action_resolve/create_attributer_writer.exp.json b/test/expectations/code_action_resolve/create_attribute_writer.exp.json similarity index 94% rename from test/expectations/code_action_resolve/create_attributer_writer.exp.json rename to test/expectations/code_action_resolve/create_attribute_writer.exp.json index cc6f78f05d..c0840bc08f 100644 --- a/test/expectations/code_action_resolve/create_attributer_writer.exp.json +++ b/test/expectations/code_action_resolve/create_attribute_writer.exp.json @@ -37,7 +37,7 @@ "character": 0 } }, - "newText": " attr_accessor :foo\n\n" + "newText": " attr_writer :foo\n\n" } ] } diff --git a/test/fixtures/create_attribute_accessor_without_surrounding_class.rb b/test/fixtures/create_attribute_accessor_without_surrounding_class.rb new file mode 100644 index 0000000000..97d58fb8c9 --- /dev/null +++ b/test/fixtures/create_attribute_accessor_without_surrounding_class.rb @@ -0,0 +1 @@ +@foo = 1 From 067af82c56044bb0162f6428f56a2fba348174bf Mon Sep 17 00:00:00 2001 From: rogancodes Date: Thu, 30 Jan 2025 20:02:42 +0530 Subject: [PATCH 7/9] addressed styling comments --- lib/ruby_lsp/requests/code_action_resolve.rb | 39 ++++++++------------ lib/ruby_lsp/requests/code_actions.rb | 14 +++---- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb index 15cfd95e7c..91e3ed575c 100644 --- a/lib/ruby_lsp/requests/code_action_resolve.rb +++ b/lib/ruby_lsp/requests/code_action_resolve.rb @@ -334,14 +334,12 @@ def switch_block_body(body, indentation) def create_attribute_accessor source_range = @code_action.dig(:data, :range) - node = ( - if source_range[:start] != source_range[:end] - @document.locate_first_within_range( - @code_action.dig(:data, :range), - node_types: CodeActions::INSTANCE_VARIABLE_NODES, - ) - end - ) + node = if source_range[:start] != source_range[:end] + @document.locate_first_within_range( + @code_action.dig(:data, :range), + node_types: CodeActions::INSTANCE_VARIABLE_NODES, + ) + end if node.nil? start_index, _ = @document.find_index_by_position(source_range[:start], source_range[:end]) @@ -368,21 +366,16 @@ def create_attribute_accessor ), ) - start_index, _ = @document.find_index_by_position( + node_context = @document.locate_node( { line: node.location.start_line, character: node.location.start_character_column, }, - ) - node_context = RubyDocument.locate( - @document.parse_result.value, - start_index, node_types: [ Prism::ClassNode, Prism::ModuleNode, Prism::SingletonClassNode, ], - code_units_cache: @document.code_units_cache, ) closest_node = node_context.node return Error::InvalidTargetRange if closest_node.nil? @@ -390,16 +383,14 @@ def create_attribute_accessor attribute_name = node.name[1..] indentation = " " * (closest_node.location.start_column + 2) attribute_accessor_source = T.must( - ( - case @code_action[:title] - when CodeActions::CREATE_ATTRIBUTE_READER - "#{indentation}attr_reader :#{attribute_name}\n\n" - when CodeActions::CREATE_ATTRIBUTE_WRITER - "#{indentation}attr_writer :#{attribute_name}\n\n" - when CodeActions::CREATE_ATTRIBUTE_ACCESSOR - "#{indentation}attr_accessor :#{attribute_name}\n\n" - end - ), + case @code_action[:title] + when CodeActions::CREATE_ATTRIBUTE_READER + "#{indentation}attr_reader :#{attribute_name}\n\n" + when CodeActions::CREATE_ATTRIBUTE_WRITER + "#{indentation}attr_writer :#{attribute_name}\n\n" + when CodeActions::CREATE_ATTRIBUTE_ACCESSOR + "#{indentation}attr_accessor :#{attribute_name}\n\n" + end, ) target_start_line = closest_node.location.start_line diff --git a/lib/ruby_lsp/requests/code_actions.rb b/lib/ruby_lsp/requests/code_actions.rb index d1b15ab2a8..05e3fb095b 100644 --- a/lib/ruby_lsp/requests/code_actions.rb +++ b/lib/ruby_lsp/requests/code_actions.rb @@ -92,14 +92,12 @@ def perform def attribute_actions return [] unless @document.is_a?(RubyDocument) - node = ( - if @range.dig(:start) != @range.dig(:end) - @document.locate_first_within_range( - @range, - node_types: INSTANCE_VARIABLE_NODES, - ) - end - ) + node = if @range.dig(:start) != @range.dig(:end) + @document.locate_first_within_range( + @range, + node_types: INSTANCE_VARIABLE_NODES, + ) + end if node.nil? start_index, _ = @document.find_index_by_position(@range[:start], @range[:end]) From 197c417a33513db37e03d54807501b9022e35830 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Mon, 3 Feb 2025 21:47:51 +0530 Subject: [PATCH 8/9] converted doc.locate to doc.locate_node --- lib/ruby_lsp/requests/code_action_resolve.rb | 7 ++----- lib/ruby_lsp/requests/code_actions.rb | 9 +++------ 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb index 91e3ed575c..30c9562dcd 100644 --- a/lib/ruby_lsp/requests/code_action_resolve.rb +++ b/lib/ruby_lsp/requests/code_action_resolve.rb @@ -342,12 +342,9 @@ def create_attribute_accessor end if node.nil? - start_index, _ = @document.find_index_by_position(source_range[:start], source_range[:end]) - node_context = RubyDocument.locate( - @document.parse_result.value, - start_index, + node_context = @document.locate_node( + source_range[:start], node_types: CodeActions::INSTANCE_VARIABLE_NODES, - code_units_cache: @document.code_units_cache, ) node = node_context.node diff --git a/lib/ruby_lsp/requests/code_actions.rb b/lib/ruby_lsp/requests/code_actions.rb index 05e3fb095b..4c698c9bce 100644 --- a/lib/ruby_lsp/requests/code_actions.rb +++ b/lib/ruby_lsp/requests/code_actions.rb @@ -100,12 +100,9 @@ def attribute_actions end if node.nil? - start_index, _ = @document.find_index_by_position(@range[:start], @range[:end]) - node_context = RubyDocument.locate( - @document.parse_result.value, - start_index, - node_types: INSTANCE_VARIABLE_NODES, - code_units_cache: @document.code_units_cache, + node_context = @document.locate_node( + @range[:start], + node_types: CodeActions::INSTANCE_VARIABLE_NODES, ) return [] unless INSTANCE_VARIABLE_NODES.include?(node_context.node.class) end From fc4eb89b0dd70f938a60119bcb5b2d4d381ad244 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Tue, 4 Feb 2025 11:26:13 +0530 Subject: [PATCH 9/9] Add character to the range --- vscode/src/test/suite/client.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/vscode/src/test/suite/client.test.ts b/vscode/src/test/suite/client.test.ts index 3f1331da31..58d236e95c 100644 --- a/vscode/src/test/suite/client.test.ts +++ b/vscode/src/test/suite/client.test.ts @@ -545,7 +545,10 @@ suite("Client", () => { textDocument: { uri: documentUri.toString(), }, - range: { start: { line: 0 }, end: { line: 0 } }, + range: { + start: { line: 0, character: 1 }, + end: { line: 0, character: 2 }, + }, context: { diagnostics: [ {