Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove-Variable can cause cls and Clear-Host to throw You cannot call a method on a null-valued expression. #3718

Closed
5 tasks done
ruffin-- opened this issue Dec 10, 2021 · 8 comments · Fixed by PowerShell/PowerShellEditorServices#1647
Assignees
Labels
Area-Engine Issue-Bug A bug to squash.

Comments

@ruffin--
Copy link

ruffin-- commented Dec 10, 2021

Prerequisites

  • I have written a descriptive issue title.
  • I have searched all issues to ensure it has not already been reported.
  • I have read the troubleshooting guide.
  • I am sure this issue is with the extension itself and does not reproduce in a standalone PowerShell instance.
  • I have verified that I am using the latest version of Visual Studio Code and the PowerShell extension.

Summary

If I use this command, somewhat blindly copied from this answer:

Get-Variable -Exclude PWD,*Preference | Remove-Variable -EA 0

And then use cls or Clear-Host in a script or in the integrated terminal, I receive the following error:

You cannot call a method on a null-valued expression.
At C:\Users\MyUser\.vscode\extensions\ms-vscode.powershell-2021.10.2\modules\PowerShellEditorServices\Commands\Public\Clear-Host.ps1:12 char:9
+         $psEditor.Window.Terminal.Clear()
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : InvalidOperation: (:) [], RuntimeException
+ FullyQualifiedErrorId : InvokeMethodOnNull

The terminal is otherwise cleared.

Because it's referencing a script in the .vscode folder, I'm assuming this is an extension issue. Entering this command in a pwsh terminal does not cause the error to appear on cls.

Restarting the terminal (clicking the trash icon, for instance) "fixes" the issue until the command is re-executed.

restarting the terminal with the kill switch

Adding $psEditor to the -Exclude parameter in Get-Variable also seems to sidestep the issue.

Get-Variable -Exclude PWD,*Preference,psEditor | Remove-Variable -EA 0

The bug is likely (?) that $psEditor is exposed to the terminal window and is removable.

PowerShell Version

PS C:\projects\pslint> $PSVersionTable


Name                           Value
----                           -----
PSVersion                      7.2.0
PSEdition                      Core
GitCommitId                    7.2.0
OS                             Microsoft Windows 10.0.19043
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}       
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visual Studio Code Version

PS C:\projects\pslint> code --version

1.62.3
ccbaa2d27e38e5afa3e5c21c1c7bef4657064247
x64

Extension Version

PS C:\projects\pslint> code --list-extensions --show-versions | Select-String powershell


ms-vscode.powershell@2021.10.2

Steps to Reproduce

PS> Get-Variable -Exclude PWD,*Preference | Remove-Variable -EA 0
PS> cls

Visuals

image

Logs

logs20211210.zip

@ruffin-- ruffin-- added the Issue-Bug A bug to squash. label Dec 10, 2021
@ghost ghost added the Needs: Triage Maintainer attention needed! label Dec 10, 2021
@ruffin-- ruffin-- changed the title Remove-Variable can cause cls and Clear-Host to throw You cannot call a method on a null-valued expression. Remove-Variable can cause cls and Clear-Host to throw You cannot call a method on a null-valued expression. Dec 10, 2021
@andyleejordan
Copy link
Member

I'm not really sure what can be done here. $psEditor is intentionally exposed to the user in the integrated console because it's a user interface to the extension. If you remove it, things break. Don't do that!

@daxian-dbw do you know if it's possible to make a variable non-removable via Remove-Variable? If so, we could prevent users from accidentally doing this.

> $psEditor | Get-Member

   TypeName: Microsoft.PowerShell.EditorServices.Extensions.EditorObject

Name                  MemberType Definition
----                  ---------- ----------
Equals                Method     bool Equals(System.Object obj)
GetCommands           Method     Microsoft.PowerShell.EditorServices.Extensions.EditorCommand[], Microsoft.PowerShell.EditorServices, Version=3.0.2.0, Culture=neutral, PublicKeyToken…
GetEditorContext      Method     Microsoft.PowerShell.EditorServices.Extensions.EditorContext, Microsoft.PowerShell.EditorServices, Version=3.0.2.0, Culture=neutral, PublicKeyToken=n…
GetHashCode           Method     int GetHashCode()
GetType               Method     type GetType()
RegisterCommand       Method     bool RegisterCommand(Microsoft.PowerShell.EditorServices.Extensions.EditorCommand, Microsoft.PowerShell.EditorServices, Version=3.0.2.0, Culture=neut…
ToString              Method     string ToString()
UnregisterCommand     Method     void UnregisterCommand(string commandName)
EditorServicesVersion Property   version EditorServicesVersion {get;}
Window                Property   Microsoft.PowerShell.EditorServices.Extensions.EditorWindow, Microsoft.PowerShell.EditorServices, Version=3.0.2.0, Culture=neutral, PublicKeyToken=nu…
Workspace             Property   Microsoft.PowerShell.EditorServices.Extensions.EditorWorkspace, Microsoft.PowerShell.EditorServices, Version=3.0.2.0, Culture=neutral, PublicKeyToken…

@andyleejordan andyleejordan added Resolution-Answered Will close automatically. and removed Needs: Triage Maintainer attention needed! labels Dec 10, 2021
@andyleejordan
Copy link
Member

I'm wondering if creating the variable with -Option Constant would work...

@SeeminglyScience
Copy link
Collaborator

Constant | AllScope is probably what we want, that's what a few other similar variables (like $Host) are set with. We should definitely guard against this.


But for the record this is not a particularly advisable practice. A better approach is to consistently declare variables, e.g.

# Less advisable example
foreach ($thing in $stuff) {
    if ($thing -eq 5) {
        $found = $true
    }
}

if (-not $found) { throw }

# Better
$found = $false
foreach ($thing in $stuff) {
    if ($thing -eq 5) {
        $found = $true
    }
}

if (-not $found) { throw }

That said, there is a middle ground:

$existingVariables = Get-Variable
try {
    whole script here
} finally {
    Get-Variable |
        Where-Object Name -notin $existingVariables.Name |
        Remove-Variable
}

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Dec 10, 2021
@daxian-dbw
Copy link
Member

Constant should do the trick, but I recommend to not use AllScope, as that will copy this variable to very new scope created later on, for example, it will be created for every module scope when you load a module, or every local scope when running a script in a new local scope.

@SeeminglyScience
Copy link
Collaborator

Good point, could break someone if they happen to use $psEditor as a local. Thanks for catching that!

@andyleejordan andyleejordan removed Needs: Maintainer Attention Maintainer attention needed! Resolution-Answered Will close automatically. labels Dec 10, 2021
@andyleejordan
Copy link
Member

Thanks both! I'll resolve this with a code change that prevents users from accidentally removing the variable.

@ruffin--
Copy link
Author

🙏

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Dec 15, 2021
@andyleejordan andyleejordan removed the Needs: Maintainer Attention Maintainer attention needed! label Dec 16, 2021
@andyleejordan andyleejordan self-assigned this Dec 17, 2021
@andyleejordan
Copy link
Member

I was able to repro this, fix it, and verify the fix:

@andys-mac-mini ~ 
> Remove-Variable psEditor
Remove-Variable: Cannot remove variable psEditor because it is constant or read-only. If the variable is read-only, try the operation again specifying the Force option.
@andys-mac-mini ~ 
> $psEditor.EditorServicesVersion

Major  Minor  Build  Revision
-----  -----  -----  --------
3      0      2      0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Engine Issue-Bug A bug to squash.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants