"Smart" variable rename

[adbertram]

When I rename a variable in one place, I want to ensure all references to that variable are renamed that are in the same scope. Basically, I want to ensure when a rename a variable that anything that might be depending on that name is renamed as well in the same script. Here's an example:

http://www.screencast.com/t/ggUIoKqSC

[Bill-Stewart]

I think you can do this already by 1) selecting the variable, 2) use the command editor.action.selectHighlights (default key mapping is Ctrl+Shift+L), then 3) type the replacement name.

[sgtoj]

@Bill-Stewart You're somewhat correct. @adbertram is referring to the Rename Symbol (F2) command. It will smartly update the symbol (e.g. variable, class, functions, etc.) versus a blind change of all occurrence of a word.

Example:

# get some objects to work with
$something    = (Get-Something);
$anotherthing = (Get-AnotherThing);

# set some value from the first object
$someValue1  = $something.SomeValue;
$someValue2  = $something.SomeValue;

# set another one from the other object
$somevalue3  = $anotherthing.SomeValue;

Basic Explanations: With Symbol Rename, the editor will understand that both occurrence SomeValue from $something.SomeValue are the same but the one from $anotherthing.SomeValue is not. So the Symbol Rename command of SomeValue property on the $something object will only make two changes even though there are at least 3 occurrence of "somevalue" (actually there are 6).

[Bill-Stewart]

Well, I interpreted the OP's request as variables rather than object members. I wouldn't mind this if performance is good. (As noted, we can already change variables pretty easily.)

[mattmcnabb]

Plus one on the request from @adbertram. The key here is that the token rename should be scope-aware in PowerShell. We've been doing this in ISESteroids for a while:

[rkeithhill]

Because of PowerShell's dynamic scoping that is hard to get right i.e. infer scripter intent. In the example above, I'd argue that renaming $Var1 on line 2 shouldn't rename the variable on line 5. Line 2 should be considered a "new" $Var1 variable because of PowerShell's "copy on write" behavior.

Now if line 2 was $script:Var1 = ... then it would be obvious that line 5 should be renamed.

However, with dynamic scopes you don't know until runtime which scope a variable is defined in if it isn't defined in the local scope. That makes it hard to ensure correctness with this feature request IMO.

[mattmcnabb]

@rkeithhill you're right about the intention of the example above. Sounds like it's difficult to implement - still would be really nice to have. Wonder how it's achieved in ISE Steroids?

[adbertram]

PowerShell Studio does it as well and I used it religiously.

On Tue, Dec 6, 2016 at 6:22 AM, Matt McNabb notifications@github.com wrote:

@rkeithhill https://github.com/rkeithhill you're right about the intention of the example above. Sounds like it's difficult to implement - still would be really nice to have. Wonder how it's achieved in ISE Steroids?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PowerShell/vscode-powershell/issues/261#issuecomment-265137172, or mute the thread https://github.com/notifications/unsubscribe-auth/AEG3-kvlvLvsAcgdC_PWAXft0mynxmnmks5rFVOXgaJpZM4JtI2E .

[daviwil]

The question isn't whether it's possible (it is), it's whether the change we make is correct in terms of how PowerShell's variable scoping works. Right now our symbol renaming is primitive but once we start doing it the "right" way, it will adhere as well as it can to PowerShell's scoping rules.

[adbertram]

Great! Can we get that next week? ;)

On Tue, Dec 6, 2016 at 8:54 AM, David Wilson notifications@github.com wrote:

The question isn't whether it's possible (it is), it's whether the change we make is correct in terms of how PowerShell's variable scoping works. Right now our symbol renaming is primitive but once we start doing it the "right" way, it will adhere as well as it can to PowerShell's scoping rules.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PowerShell/vscode-powershell/issues/261#issuecomment-265169692, or mute the thread https://github.com/notifications/unsubscribe-auth/AEG3-nH5Z8DLHCGTTs5RFqVOrV2DkBxZks5rFXcWgaJpZM4JtI2E .

[daviwil]

Nope ;) You will get a new release with some other shiny features, though.

[Clijsters]

Would be a great feature!

[ajansveld]

Already a fan of vscode but this would really take it to the next level.

[johntiger1]

Also would like this feature

[smk89]

I'm a bit surprised that it's not in already. The hard bit should be correctly finding all of the references to a variable, which we can already do by hitting Shift+F12. What's left is a way to replace the highlighted strings with a new string.

[Stephanevg]

I am also verry interested in this feature. is there any idea when this will be added?( I don't want to be a negative Nancy, but this issue has been open for two years now.)

[EnderSyth]

I'd like to chime in on this as well. VSCode is great for many languages but for powershell it's lacking some feature you get used to in other languages or even other editors (the ISE integrated terminal with intelisense). I hope this get's added as it would improve the workflow for myself and many others.

[TylerLeonhardt]

(the ISE integrated terminal with intelisense)

@gyro2death have you tried the PowerShell Preview extension with PSReadLine support in the integrated console?

[rjmholt]

I wrote up a quick note in a reddit post a while ago.

PowerShell's dynamic (i.e. determined at runtime) scope makes this impossible to do in general. Take this example:

$x = 1

function Write-X
{
    $x
}

function Write-XInScope
{
    $x = 7
    Write-X
}

Write-X
Write-XInScope

If you rename the $x in Write-X, which $xs do you rename?

Even if you only rename the single $x occurrence in Write-X, you've still broken Write-XInScope.

I think this issue has been blocked by the hope that it might be possible to do something clever here, but it's not; this is runtime-only information, so the only way to always know the scope of a variable is to execute the script.

My thinking is that we should instead take a conservative approach:

• Renaming a normal variable only changes the name of it in its immediate scope
• Renaming a script: variable is lexical, so we can safely rename all occurrences there
• Renaming a global: or env: is slightly more work, but just requires renaming the variable everywhere in a workspace.

This policy may still cause unexpected effects, but it's simpler than almost all other ways of doing things (the other simple one being to rename all occurrences in a script file -- but that file could still be called within another scope).

I have the feeling that many people write PowerShell scripts as though PowerShell has lexical scope, but unfortunately that's not the case. We'll do our best to balance expectations with not breaking PowerShell, but this is one of those edge areas.

[AndersRask]

I wrote up a quick note in a reddit post a while ago.

PowerShell's dynamic (i.e. determined at runtime) scope makes this impossible to do in general. Take this example:

$x = 1

function Write-X
{
    $x
}

function Write-XInScope
{
    $x = 7
    Write-X
}

Write-X
Write-XInScope

But then again, calling a variable inside a function that was defined outside is pretty nasty! Having read a ton of other peoples PowerShell, I agree though, that we cannot expect lexical scope to be used always, however I think people who would do this, are not even aware of the implications or problems with the code ;-)

I would be fine with that the refactoring only worked inside the advanced function you were currently working, as this would probably be the case in most instances anway

[SeidChr]

open since 2016

is there some light at the end of the tunnel? is there a tunnel?

[JustinGrote]

@SeidChr as @rjmholt correctly, noted, there's no way to implement this in a way that users of other languages would expect, e.g. to correctly rename all references of a variable in all scopes. However, if we limit the expectation to only rename all occurances of a variable within a single function scope (e.g. you update a parameter and all the references to that parameter within a function will be updated), and maybe have a popup that a user has to acknowledge "this doesn't work like other languages", then this may be feasible.

Trust me, you're not alone, it annoys me all the time when F2 doesnt work, and I may take a stab at it if I can fit it into my priority deck.

Here's some prior art that is sort of along the lines of an approach that might work, mine the AST and then rebuild it: https://www.powershellgallery.com/packages/PSModuleDevelopment/2.2.6.72/Content/functions%5Crefactor%5CRename-PSMDParameter.ps1, @FriedrichWeinmann may have some thoughts.

[SeidChr]

I can't believe it's impossible to figure out the actual occurrences of a symbol. I'm not asking to rename values in Set-Variable or similar. Although i believe even this is possible. But i have no choice than to believe it.

[JustinGrote]

@SeidChr it's not that it's hard to find all occurances of the variable. The problem is that the same variable name can have different contexts. Take this example:

$x = 5

function test ($x, $y) {
  return $x + $y
}

Renaming all occurances of $x will break this script.

[SeeminglyScience]

Edit(ish): I see Justin already beat me to it but hopefully this still clears some stuff up 😁

It's not that it's impossible to find all occurrences, it's that it's impossible to tell which occurrences actually refer to the same variable.

Take this for instance:

$someVar = 10
function DoSomething {
    $someVar = 40
}

Is this the same instance? No not typically. But if you do . DoSomething now it is. Most folks don't know you can dot source a function by name so maybe that's not a huge problem by itself, but here are a few more examples:

# Not same
$var = 10
0..10 | Select-Object @{n='SomeProperty';e={ $var = 30 * $_; $var }}

# Not same
$var = 10
Get-ChildItem | Rename-Item -NewName { $var = $_.FullName + (Get-Random); $var }

# Same
$var = 10
0..10 | ForEach-Object {
    $var += 5
}

# Not same
$var = 10
. (Get-Module Pester) { $var = 30 }

# Same
$var = 10
$sb = { $var = 30 }
. $sb

# ???
$var = 10
$sb = { $var = 30 }
$shouldDotSource = Get-Random -Minimum 0 -Maximum 2
if ($shouldDotSource) {
    . $sb
} else {
    & $sb
}

There's no way to know statically whether any of these scriptblocks will even be invoked, much less what context they save their state to.

Now, all that might not matter to you and I get that - and I also think it would be useful regardless. It's a problem of how we can articulate the limitations to the user. If we just implement the feature with all of the problems that it will inevitably have, then folks will assume we cracked it somehow and we will potentially end up causing a lot of very difficult to troubleshoot bugs.

That said, we do provide extension points that make this pretty easy to put into a profile function. In your $profile, put this:

if ($psEditor) {
    function Profile.RenameVariable {
        [CmdletBinding()]
        param($Context)
        end {
            $variable = Find-Ast -AtCursor
            if ($variable -isnot [System.Management.Automation.Language.VariableExpressionAst]) {
                $psEditor.Window.ShowWarningMessage('No variable found at current cursor location.')
                return
            }

            $esp = [Microsoft.PowerShell.EditorServices.Extensions.EditorObjectExtensions, Microsoft.PowerShell.EditorServices]::GetExtensionServiceProvider(
                $psEditor)
            $task = $esp.EditorUI.PromptInputAsync('New variable name?')

            while (-not $task.AsyncWaitHandle.WaitOne(200)) { }
            $newName = $task.GetAwaiter().GetResult()

            if ([string]::IsNullOrWhiteSpace($newName)) {
                return
            }

            Find-Ast { $_.VariablePath.UserPath -eq $variable.VariablePath.UserPath } |
                Sort-Object { $_.Extent.StartOffset } -Descending |
                ForEach-Object {
                    $Context.CurrentFile.InsertText(
                        "`$$newName",
                        $_.Extent.StartLineNumber,
                        $_.Extent.StartColumnNumber,
                        $_.Extent.EndLineNumber,
                        $_.Extent.EndColumnNumber)
                }
        }
    }

    $splat = @{
        Name = 'Profile.RenameVariable'
        ScriptBlock = ${function:Profile.RenameVariable}
        DisplayName = 'Rename variable in current document'
    }

    Register-EditorCommand @splat
}

And in your VSCode keybindings.json include this:

    {
        "key": "F2",
        "command": "PowerShell.InvokeRegisteredEditorCommand",
        "args": { "commandName": "Profile.RenameVariable" },
        "when": "editorLangId == 'powershell'"
    },

Now you have a really basic version of this functionality. It can be expanded to hit the whole workspace and/or Set-Variable as well, though a good bit less trivial.

[JustinGrote]

My thought is that we should be able to trigger a modal dialog on first use linking to an article expressing the limitations, and make them click "OK" or "Don't show again" (dropping a setting into their user profile) to at least provide the basic functioanlity, which 99% of the time all I want is to rename a parameter and have all references to that parameter in my function updated (and I generally don't bleed context so I'm not worried about parent variables, etc.). This is a viable MVP case for the feature I think, and if we can get it to a point that it can handle more reasonable edge cases, it can evolve.

[FriedrichWeinmann]

Thanks for the shoutout @JustinGrote :) I'll have to admit that the function you pointed out above is still from one of my early days of messing with the AST, so pardon the implementation details ^^

That said, it is quite possible to detect variable assignments and scope boundaries using AST so it should be doable. Catching Set-Variable and Get-Variable references is also possible, though some of the explicit scope references might be difficult. Catching splatted values might also be possible, but not 100% reliable (but let's be honest, if you use weird hashtable definitions on a splat for Set-Variable, I shall accuse you of intentional refactor evasion ;) ).

Did some of that work in my Refactor module, which is my AST scanning platform where I will also migrate most of my PSMD stuff to.

Quite frankly, one of my Christmas break projects is to write a new PSScriptAnalyzer rule to detect variable scope boundary violations, which would solve most of my concerns for solving this - heck, we could gate the functionality behind that check and to refuse to do the refactor as long as SBVs are detected ...

@SeeminglyScience : Excellent edge cases writeup :) While we could find much of that ... but only in the file/workspace we work on. So yeah, some ambiguity around dotsourcing remain ... but anybody who knows that one should be able to understand and handle those cases. Furthermore, I think that internal variables should be dealt with like non-public properties/methods/classes in C# where yes you can access them with reflection, but we'll change things as we please and you'll have to live with it.

@JustinGrote : Regarding the parameter renaming feature - you may want to look into Refactor as it already is. While it can't (yet) rename the variables inside the function (unless you tell it how, it's extensible), it already can scan entire code-bases and find & update references. With some more modules (linked in the readme) it can also scan github and Azure DevOps for uses of the command.

[SeeminglyScience]

@SeeminglyScience : Excellent edge cases writeup :) While we could find much of that ... but only in the file/workspace we work on. So yeah, some ambiguity around dotsourcing remain ... but anybody who knows that one should be able to understand and handle those cases. Furthermore, I think that internal variables should be dealt with like non-public properties/methods/classes in C# where yes you can access them with reflection, but we'll change things as we please and you'll have to live with it.

I agree that it's definitely possible to make a "good enough" implementation for folks who are following what we would consider to be best practices. That said, the vast majority of users of the extension won't be, and the fact that the user still needs to be very careful needs to be communicated somehow.

[Razmo99]

Hello, not sure if this should be another post, I think it's relevant to the discussion.

I've been wanting this feature for a while and forked the repo and started working on a branch

I've just got it working for function renaming within a single file (with some test cases I could think of), as it appeared easier to me than variables Please keep in mind my C# skills are basic and this is my first post to a public repo, and the code is just a proof of concept with minimal safety's

Its using the System.Management.Automation.Language.ICustomAstVisitor2 which is used to visit all the nodes within the AST and to track the state it's in should rename and what's the target function etc.

I'm just looking to know if this is the right direction for implementation before I go too far down the rabbit hole because I could see that I could setup another Visitor for variables. Wondering what the next steps are

[starball5]

Related on Stack Overflow: PowerShell rename refactor (F2) does nothing in VS Code.

[mklement0]

Thanks for the great proof-of-concept, @SeeminglyScience; I took the liberty of enhancing it to deal with scope prefixes, $using: references, and splatted variables in this answer to the linked SO post. The limitations, tradeoffs and assumptions made are listed there.

One thing I noticed is that the use of multiple .CurrentFile.InsertText() invariably makes each substitution a separate action with respect to undo / redo functionality in VSCode, which is somewhat unfortunate (you cannot just revert to the previous state with a single Ctrl-Z).

Is there any way around that? Do the underlying VSCode APIs support that in principle? Is it worth opening an issue here?

[SeeminglyScience]

Is there any way around that? Do the underlying VSCode APIs support that in principle? Is it worth opening an issue here?

Yeah we just don't surface it atm. We could add an API sorta like

class FileEdit
{
    IFileRange Range { get; }

    string NewContent { get; }
}

partial class FileContext
{
    void ApplyEdits(params FileEdit[] edits);
}

[starball5]

@SeeminglyScience I'm not too familiar with the extension API, but what about WorkspaceEdit? It has insert and replace.

[SeeminglyScience]

@SeeminglyScience I'm not too familiar with the extension API, but what about WorkspaceEdit? It has insert and replace.

Yep! We'd basically be adding a publicly accessible version of that. Under the covers that's all InsertText uses today iirc

[starball5]

Yep! We'd basically be adding a publicly accessible version of that. Under the covers that's all InsertText uses today iirc

Wait- why is there a need to add a publicly accessible version of it? Isn't it already "publicly accessible" to extensions? workspace already has applyEdit, and it's the return type of RenameProvider

[JustinGrote]

@starball5 I think it's more exposing the rename action thru the LSP stack, e.g. VSCode -> Omnisharp -> PSES -> Extension and back.

[Razmo99]

If you have a look at what the C# extension does to setup the renameprovider it has all you'll need to add this functionality, additionally the prepearrename method can be used to catch out the use of dot sourcing etc and then allow a msg to be raised to the user

[Omzig]

can we bump this somehow and get vscode to F2 (rename) a powershell symbol?

[SeidChr]

*bump

[JustinGrote]

Progress is happening! Here is a new alpha preview build of the functionality if you want to try it out. powershell-2024.9.0-renamepreview.zip

1. Download the above file
2. Rename .zip to .vsix
3. Open VSCode, hit F1, and type "VSIX" and hit enter
4. Browse for the vsix file
5. Reload VSCode

Open a .ps1 file on disk (unsaved/untitled currently unsupported) and hit F2. You'll get a popup asking you to opt in, then you can try out renaming functions/providers/symbols

https://github.com/user-attachments/assets/741cbc08-6e3a-436e-8f63-da7b1f213488

[MartinGC94]

Awesome! It even takes splatting into account (though it is erroneously adding a $ before the parameter name).
Regarding the creation of an alias attribute when renaming a parameter, I don't think it should be enabled by default. I'd imagine most people will primarily use it in the initial development phase where breaking changes are not a concern so it just gets in the way.

[JustinGrote]

@MartinGC94 that's a fair point, it'll be a toggleable setting either way.

We are building a test suite of supported scenarios, I'm currently refactoring it to be LSP-only so that it will be portable to neovim and whatnot. Once that's done I may recruit you if you have some time to help us refine the AST visitors to help with more edge cases we pick up.