Skip to content

Formatting not working with 1.7.0 #1298

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

Closed
SQLDBAWithABeard opened this issue Apr 27, 2018 · 16 comments
Closed

Formatting not working with 1.7.0 #1298

SQLDBAWithABeard opened this issue Apr 27, 2018 · 16 comments
Assignees
Labels

Comments

@SQLDBAWithABeard
Copy link
Contributor

System Details

  • Operating system name and version:
  • VS Code version:
  • PowerShell extension version:
  • Output from $PSVersionTable:

1.22.2
3aeede733d9a3098f7b4bdc1f66b63b0f48c1ef9
x64

ajor Minor Build Revision


1 7 0 0

[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]

Name Value


PSVersion 5.1.16299.251
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.16299.251
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

Issue Description

Formatting is not working I have tried this with Insiders and normal versions with a couple of different scripts with the same results

formatting-error

Attached Logs

You can see the log entries in the gif, it just says formatting finished but can upload more if you wish

@rkeithhill
Copy link
Contributor

Those log entries are just from the extension - VSCode side. We need the logs from the language service - PSES side - to see what is going wrong in PSES.

@SQLDBAWithABeard
Copy link
Contributor Author

It's this again.
I have 1.16.1 fodler in $HOME.vscode-insiders\extensions\ms-vscode.powershell-1.7.0\modules\PSScriptAnalyzer this time.

2018-04-28 07:41:40 [ERROR] - Method "InvokePowerShell" at line 445 of C:\projects\powershelleditorservices\src\PowerShellEditorServices\Analysis\AnalysisService.cs

The 'Invoke-Formatter' command was found in the module 'PSScriptAnalyzer', but the module could not be loaded. For more information, run 'Import-Module PSScriptAnalyzer'.

1524897678-76060de6-59b9-4e70-9686-24a835ab003d1524897335539.zip

@rkeithhill
Copy link
Contributor

From a regular PowerShell prompt can you cd to ~\.vscode\extensions\ms-vscode.powershell-1.7.0\modules\PSScriptAnalyzer\1.16.1 and try to import that version ipmo .\PSScriptAnalyzer.psd1?

If that works, try the same thing from the PSIC in VSCode. And if that works, execute this in PSIC:

gmo PSScriptAnalyzer -ListAvailable | % Path

PSScriptAnalyzer is a module that VSCode does not load by path. It relies on module auto-loading when a PSSA command is invoked. And the extension appends its modules folder path to $env:PSModulePath so it will other versions of PSSA that you've installed manually before it finds the one it comes with.

I believe that strategy is flawed. We should be importing the module with a -minimumversion spec since I think Invoke-Formatter is a newer command.

If this is indeed the issue, I would recommend removing versions of PSSA that don't have Invoke-Formatter. In fact, maybe just remove all versions prior to 1.16.1.

@rkeithhill
Copy link
Contributor

Ah, it looks like PSES does attempt to import the module explicitly.

Import-Module C:\Users\mrrob\.vscode\extensions\ms-vscode.powershell-1.7.0\modules\PSScriptAnalyzer\1.16.1\PSScriptAnalyzer.psd1

However, this happens after several attempts to run Invoke-Formatter. So we have a timing issue I suppose.

@SeeminglyScience
Copy link
Collaborator

This line is the issue. Previously it would use Get-Module -ListAvailable to determine the highest version module installed and import that path explicitly into the initialsessionstate for PSSA.

That's my bad, I remember specifically talking about this in review but must have missed that portion being removed.

@TylerLeonhardt
Copy link
Member

I'm going to let @rjmholt respond since we removed that logic on purpose.

@SQLDBAWithABeard as a mitigation, if you install a newer version of PSScriptAnalyzer, your issue should go away.

@rjmholt
Copy link
Contributor

rjmholt commented Apr 30, 2018

Yes, this is my fault. I changed the PSScriptAnalyzer import so it would honour the module path. Previously it searched all available modules and found the most recent one on the path and imported it. I've been hearing a lot of PowerShell users saying how impossible it is when they can't specify a lower version of something (mainly in PowerShell), and it seemed bad that we were second-guessing the import module logic.

I think we should keep the behaviour the honours the module path, but we should do a minimum version check. We currently use a PowerShell module "API", but it's not a very good one and I think instead we should try actually invoking Import-Module PSScriptAnalyzer -MinimumVersion $Version.

But, if this is actually trying to import the module at the exact path like @rkeithhill says, then it might not be what we think and may be trickier. Again though, the "API" we use, despite claiming to import a module, only puts it in a list of modules to import at some later point. I think we might solve this by invoking Import-Module.

Does that seem reasonable? Or bad and broken?

@SeeminglyScience
Copy link
Collaborator

I've been hearing a lot of PowerShell users saying how impossible it is when they can't specify a lower version of something (mainly in PowerShell), and it seemed bad that we were second-guessing the import module logic.

Makes sense to me, I've run into that and it's not fun.

I think we might solve this by invoking Import-Module.

That would work with how we currently configure the PSSA RunspacePool, but if we ever decided to change the minimum or maximum Runspaces allocated then Import-Module can no longer be guaranteed to be persistent.

If it's possible to enforce a minimum version with InitialSessionState that would be ideal. If not, we would either need to decide to stick with one min/max runspace and use Import-Module or switch back to the old logic.

@rjmholt
Copy link
Contributor

rjmholt commented Apr 30, 2018

Yeah, I was wondering about that. I'll look into whether doing this with the InitialSessionState is possible. The old usage seemed to call Import-Module deep under the covers, so if we can make it do that somehow with the parameters we want, we should be good.

@SQLDBAWithABeard
Copy link
Contributor Author

gmo PSScriptAnalyzer -ListAvailable | % Path gives

C:\Users\mrrob\OneDrive\Documents\WindowsPowerShell\Modules\PSScriptAnalyzer\1.10.0\PSScriptAnalyzer.psd1
C:\Program Files\WindowsPowerShell\Modules\PSScriptAnalyzer\1.16.1\PSScriptAnalyzer.psd1
C:\Users\mrrob.vscode-insiders\extensions\ms-vscode.powershell-1.7.0\modules\PSScriptAnalyzer\1.16.1\PSScriptAnalyzer.psd1

Importing from VSIC doesn't resolve the issue :-(

But copying 1.16.1 into the PSMOdulePath and reloading does :-) Thankyou @tylerl0706

@TylerLeonhardt
Copy link
Member

Just for context, PowerShell resolves an ipmo in order of PSModulePaths:

if PSModulePath has:

C:\Users\mrrob\OneDrive\Documents\WindowsPowerShell\Modules;C:\Program Files\WindowsPowerShell\Modules

and you ipmo PSSA it will check the first directory for ANY PSSA and if it finds one, it will import that one. If it doesn't find one, it checks the next path and so on.

In the extension, we add a path to the END of PSModulePath:

C:\Users\mrrob\.vscode-insiders\extensions\ms-vscode.powershell-1.7.0\modules

which contains a version of PSSA and Plaster.

If you have a version of PSSA in one of the folders before that folder in your PSModulePath (and it looks like @SQLDBAWithABeard did! 😄), then we install that version of PSSA.

@rjmholt did a great job explaining why we are honoring what ipmo does.

@rjmholt
Copy link
Contributor

rjmholt commented May 1, 2018

@SQLDBAWithABeard Just to document, importing the module from within VSCode won't resolve the issue because PSScriptAnalyzer starts from a runspace pool when the extension starts up (which is precisely why using the -MinimumVersion flag is tricky).

PSScriptAnalyzer runs in its own sandbox essentially, so state changes in the editor won't affect it. It works by sending the text of the script into the sandbox over to it and spitting results back out. This is good because it means it doesn't depend on the state of the editor, so it can be parallelised.

Then when you start the extension up (formally, when you start the EditorServices process, which the extension does), it tacks its own modules directory onto the end of the module path. Which is nice because it means you can override its internal module versions by just sticking your own module anywhere on your module path. And the intent is that eventually it should be possible to integrate other modules with the extension just by having them on the path.

The problem was that despite going to the trouble of using the module path, we weren't honouring it. Except now we have the issue that an incompatible version of PSScriptAnalyzer on the module path can break us, so we need to address that by somehow specifying what our version constraints are at load time...

@andikrueger
Copy link

I saw the same issue and did delete the PSScriptAnalyzer stored in:
C:\Users\USERNAME\Documents\WindowsPowerShell\Modules\PSScriptAnalyzer\1.11.0\PSScriptAnalyzer.psd1

Can anyone explain, why it got stored in this location and not in Programs\WindowsPowerShell\Modules?

@TylerLeonhardt
Copy link
Member

@andikrueger it depends on whether you installed PSSciptAnalyzer with -Scope CurrentUser. For example, my PSModule path is:

/Users/tylerleonhardt/.local/share/powershell/Modules:/usr/local/share/powershell/Modules:/usr/local/microsoft/powershell/6.0.2/Modules

if I were to do

Install-Module PSScriptAnalyzer -Scope CurrentUser

The module would end up in the first location (aka the current user scope) in my PSModulePath:

/Users/tylerleonhardt/.local/share/powershell/Modules

if I were to do

Install-Module PSScriptAnalyzer

The module would end up in the second location in my PSModulePath:

/usr/local/share/powershell/Modules

@andrewducker
Copy link

Is there a workaround for this?

@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Jun 1, 2018

@andrewducker yes, Update-Module PSScriptAnalyzer should do the trick

You may need to do this in an elevated shell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants