You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
textDocument/prepareRename returns nil (and textDocument/rename silently no-ops) when the cursor is placed on the definition site of a top-level constant. Renaming works only from reference sites. The result is the editor surfaces "current selection cannot be renamed" on what looks like a perfectly valid rename target.
Reproduction
example.rb:
classExampleFOO=1#^ cursor hereend
LSP request:
textDocument/prepareRename
position: { line: 1, character: 2 } # on the F of FOO
Expected: a Range covering FOO, allowing the editor to proceed with rename.
Actual: server returns null. Editor reports "cannot be renamed."
Placing the cursor on a reference of FOO elsewhere works correctly.
The same applies to multi-assignment targets (A, B = 1, 2).
Root cause
PrepareRename and Rename filter RubyDocument.locate to:
Prism::ConstantReadNode
Prism::ConstantPathNode
Prism::ConstantPathTargetNode
A top-level constant assignment LHS (FOO = ...) parses as Prism::ConstantWriteNode, and a multi-assignment LHS parses as Prism::ConstantTargetNode. Neither is in the filter, so locate does not return them and perform exits early.
Even with the filter expanded, RubyIndexer::Index.constant_name also only handles the three read-style nodes and returns nil for write nodes, so the rename path would silently no-op without a matching update there.
Reviewed the introducing PRs and the recent Rubydex migration; none discuss the definition-site case:
Add rename support for constants #2626 "Add rename support for constants" introduced the filter. PR description and review only discuss ConstantReadNode / ConstantPathNode / ConstantPathTargetNode. Tests cursor on class names (class RenameMe), never on FOO = ....
Migrate rename to use Rubydex #4033 "Migrate rename to use Rubydex" carries the filter and Index.constant_name's case statement forward unchanged. New tests still cursor on class names only.
Doc PR Clarify limitations of rename #3113 states "Rename is currently only supported for constants, module names and class names," consistent with the maintainers believing constants are fully covered.
No issue or PR I could find specifically mentions cursoring on ConstantWriteNode / ConstantTargetNode.
Suggested fix scope
Minimal, low-risk:
Add Prism::ConstantWriteNode and Prism::ConstantTargetNode to both node_types: filters.
Extend RubyIndexer::Index.constant_name to return node.name.to_s for those two types.
In PrepareRename#perform, use target.name_loc (not target.location) when target is a ConstantWriteNode, since its location covers the whole FOO = value expression.
Out of scope for a first pass (separate node types, more edge cases): ConstantPathWriteNode, ConstantPathOperatorWriteNode, ConstantOperatorWriteNode, ConstantOrWriteNode, ConstantAndWriteNode. Worth tackling in a follow-up if there is interest.
Local verification
I applied the three-file patch above to a vendored copy of ruby-lsp 0.26.9 and confirmed that:
prepareRename on a top-level constant definition now returns the correct range.
rename from the definition site updates both the definition and all references in the workspace.
Reference-site rename (the previously working case) is unaffected.
Happy to send a PR with tests if useful; opening this as a bug report first to surface whether the constraint was deliberate.
Environment
ruby-lsp 0.26.9 (also confirmed via source review that main post-Migrate rename to use Rubydex #4033 / v0.27.0.beta1+ has the same filters and constant_name case statement)
Summary
textDocument/prepareRenamereturnsnil(andtextDocument/renamesilently no-ops) when the cursor is placed on the definition site of a top-level constant. Renaming works only from reference sites. The result is the editor surfaces "current selection cannot be renamed" on what looks like a perfectly valid rename target.Reproduction
example.rb:LSP request:
Expected: a
RangecoveringFOO, allowing the editor to proceed with rename.Actual: server returns
null. Editor reports "cannot be renamed."Placing the cursor on a reference of
FOOelsewhere works correctly.The same applies to multi-assignment targets (
A, B = 1, 2).Root cause
PrepareRenameandRenamefilterRubyDocument.locateto:Prism::ConstantReadNodePrism::ConstantPathNodePrism::ConstantPathTargetNodeA top-level constant assignment LHS (
FOO = ...) parses asPrism::ConstantWriteNode, and a multi-assignment LHS parses asPrism::ConstantTargetNode. Neither is in the filter, solocatedoes not return them andperformexits early.Even with the filter expanded,
RubyIndexer::Index.constant_namealso only handles the three read-style nodes and returnsnilfor write nodes, so the rename path would silently no-op without a matching update there.Affected files on
main:lib/ruby_lsp/requests/prepare_rename.rb(node_types:filter)lib/ruby_lsp/requests/rename.rb(node_types:filter)lib/ruby_indexer/lib/ruby_indexer/index.rb(Index.constant_name)Why this looks like an oversight
Reviewed the introducing PRs and the recent Rubydex migration; none discuss the definition-site case:
ConstantReadNode/ConstantPathNode/ConstantPathTargetNode. Tests cursor on class names (class RenameMe), never onFOO = ....Rename.Index.constant_name's case statement forward unchanged. New tests still cursor on class names only.No issue or PR I could find specifically mentions cursoring on
ConstantWriteNode/ConstantTargetNode.Suggested fix scope
Minimal, low-risk:
Prism::ConstantWriteNodeandPrism::ConstantTargetNodeto bothnode_types:filters.RubyIndexer::Index.constant_nameto returnnode.name.to_sfor those two types.PrepareRename#perform, usetarget.name_loc(nottarget.location) whentargetis aConstantWriteNode, since itslocationcovers the wholeFOO = valueexpression.Out of scope for a first pass (separate node types, more edge cases):
ConstantPathWriteNode,ConstantPathOperatorWriteNode,ConstantOperatorWriteNode,ConstantOrWriteNode,ConstantAndWriteNode. Worth tackling in a follow-up if there is interest.Local verification
I applied the three-file patch above to a vendored copy of ruby-lsp 0.26.9 and confirmed that:
prepareRenameon a top-level constant definition now returns the correct range.renamefrom the definition site updates both the definition and all references in the workspace.Happy to send a PR with tests if useful; opening this as a bug report first to surface whether the constraint was deliberate.
Environment
mainpost-Migrate rename to use Rubydex #4033 / v0.27.0.beta1+ has the same filters andconstant_namecase statement)