Repository navigation
Add an inline variable refactor - #4252
Open
IslamElsayed wants to merge 1 commit into
Open
IslamElsayed wants to merge 1 commit into
IslamElsayed wants to merge 1 commit into
Conversation
Offered with the cursor on a local variable's name, at its assignment or a read. It replaces every read with the assigned value and removes the assignment, adding parentheses only where operator precedence or the surrounding syntax needs them. It refuses when inlining could change behaviour: a variable assigned more than once or by a parameter, an assignment inside an expression, reads the assignment may not reach, a variable the value reads being reassigned before the value is read, and values that call methods or build objects unless they are read once, unconditionally, in the next statement. Closes Shopify#2504
Author
|
I have signed the CLA! |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Closes #2504
The Ruby LSP can extract a variable but can't inline one.
Implementation
CodeActionsoffers "Refactor: Inline Variable" (refactor.inline) when the cursor or selection is on a local variable's name, at its assignment or at a read. As with the other refactors, the resolve request does the work.The resolver finds the scope that declares the variable, using Prism's
depthto step out of blocks. It collects every assignment and read of the variable there, skipping block parameters that shadow it. It then replaces each read with the assigned value and removes the assignment: the whole line, ora = 1;when it shares a line.Parentheses follow your suggestion, but are based on operator precedence rather than added every time. The example in the issue stays
b = 5 * 10 - 5, whilea = 5 - 10; b = a * 5becomesb = (5 - 10) * 5. It also handles**grouping to the right, negative literals, receivers ((1..3).to_a),and/or, and hashes passed without parentheses. When unsure, it adds them.It refuses, with a message, when inlining could change behaviour:
+=and multiple assignment) or is a parameter.foo(a = 1)), or might not run before a read (a = 1 if cond; puts a).The fourth rule is stricter than what I described in the issue: moving a call past other statements can change its result too.
The part most worth a careful look is
needs_parentheses?and its precedence table.Automated Tests
;. One more expectation checks that the action is offered.test/requests/inline_variable_test.rbcovers the precedence cases and each refusal.server_test.rb, which expects the checkout directory to be namedruby-lsp.Manual Tests
aina = 5 - 10, or on either read, and open the code actions menu.puts 5 - 10andb = (5 - 10) * 5.5 - 10tocomputeand try again. The action is offered but refuses with a message, becausecomputewould run twice.I use AI assistance when working on patches, and I check and test everything before submitting.