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

Cmdlet aliases are expanded automatically when formatting document #1842

Closed
mkht opened this issue Apr 3, 2019 · 14 comments · Fixed by PowerShell/PSScriptAnalyzer#1216
Closed
Labels

Comments

@mkht
Copy link

mkht commented Apr 3, 2019

System Details

VSCode version

1.32.3 a3db5be9b5c6ba46bb7555ec5d60178ecc2eaae4 x64

VSCode extensions

ms-vscode.powershell@1.12.0

No other extensions is installed.
The vscode settings have not changed anything.

PSES version

1.12.0.0

PowerShell version:

Name                           Value
----                           -----
PSVersion                      5.1.17763.1
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.17763.1
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Issue Description

All aliases in my PS codes are expanded unexpectedly when executing "Format Document".

I tried formatting the code below.

1..10|%{$_*2}

Expected result

I expect this code will format like this.

1..10 | % { $_ * 2 }

Actual result

But vscode-powershell@1.12.0.0 formatted.

1..10 | ForEach-Object { $_ * 2 }

I know that formatting is provided by the PSScriptAnalyzer module.
However, I think this issue is specific to vscode-powershell.
I execute these commands in the Windows PowerShell terminal. (Not in the integrated terminal)

PS C:\> Install-Module PSScriptAnalyzer -Scope CurrentUser
PS C:\> Get-Module PSScriptAnalyzer -ListAvailable

    Directory: C:\Users\Administrator\Documents\WindowsPowerShell\Modules

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Script     1.18.0     PSScriptAnalyzer                    {Get-ScriptAnalyzerRule, Invoke-ScriptAnalyzer, Invoke-Formatter}

PS C:\> Invoke-Formatter '1..10|%{$_*2}'
1..10 | % { $_ * 2 }

In this result, The original formatter of the PSScriptAnalyzer@1.18.0 is not expand aliases automatically.

Is this bug? Does anyone have an idea of how to work around it?

@SydneyhSmith
Copy link
Collaborator

@mkht thanks for reporting this, I am able to reproduce it when I use Core but not with Windows (maybe just a coincidence?), which is interesting because you are running Windows...I am wondering if you have any cusotm settings in your VSCode profile?

@rjmholt
Copy link
Contributor

rjmholt commented Apr 3, 2019

@bergmeister do you know what settings you use to control this in PSScriptAnalyzer?

@bergmeister
Copy link
Contributor

bergmeister commented Apr 3, 2019

I have noticed this recently as well but I don't use Windows PowerShell much any more, I am on Core 6.2.0 for my shell and also in the vscode terminals (on Windows). The avoid alias rule is not included by default for Invoke-Formatter and Invoke-Formatter 'gci' does not fix the aliases (neither in Core or WPS) so it must be the way the extension calls PSSA,I think it is the hashtable that PSES supplies as the -settings param to PSSA here that somehow includes this rule. Maybe the logs give more hints. It could also be a side of effect of how the extension does formatting. Because PSSA returns something to the extension that the extension can fix, the extension it might be applying the auto-fix automatically for some reason. However, when using if ($a -eq $null) { gci }, only the aliases warning gets-auto-fixed. I have tried turning some formatting settings off and it still happens so not sure what causes it but I suspect it is the extension

@rjmholt
Copy link
Contributor

rjmholt commented Apr 3, 2019

Yeah it seems like this is an extension configuration thing.

Here's the list of rules we include:
https://github.com/PowerShell/PowerShellEditorServices/blob/8c66fa912ecfac4e93345ca96dc49d1787f81f1d/src/PowerShellEditorServices/Analysis/AnalysisService.cs#L33-L48

We do log the rules, so if there are logs we might be able to answer the question. Either way, if we have a repro for this, then it should be a relatively simple fix.

@mkht
Copy link
Author

mkht commented Apr 4, 2019

I tried only Windows PowerShell, not tried Core.
Confirmed that the issue reproduce on the Windows 10 Pro 1809 with VSCode 1.32.3 installed cleanly and has no any custom settings.

I attach the extension logs.
logs.zip

@SydneyhSmith SydneyhSmith added Triage and removed Triage labels Apr 4, 2019
@sandscap-sc
Copy link

Ref #496

@mkht
Copy link
Author

mkht commented Apr 5, 2019

I still could not find the cause of the issue.
However, I found two workarounds.

The first is to use custom PSSA rule settings.
I create the following configuration file and specify in powershell.scriptAnalysis.settingsPath.
Then aliases are no longer automatically expanded.

# This setting is exactly same as the "CodeFormatting" setting in the PSSA@1.18.0
@{
    IncludeRules = @(
        'PSPlaceOpenBrace',
        'PSPlaceCloseBrace',
        'PSUseConsistentWhitespace',
        'PSUseConsistentIndentation',
        'PSAlignAssignmentStatement',
        'PSUseCorrectCasing'
    )

    Rules        = @{
        PSPlaceOpenBrace           = @{
            Enable             = $true
            OnSameLine         = $true
            NewLineAfter       = $true
            IgnoreOneLineBlock = $true
        }

        PSPlaceCloseBrace          = @{
            Enable             = $true
            NewLineAfter       = $true
            IgnoreOneLineBlock = $true
            NoEmptyLineBefore  = $false
        }

        PSUseConsistentIndentation = @{
            Enable          = $true
            Kind            = 'space'
            PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline'
            IndentationSize = 4
        }

        PSUseConsistentWhitespace  = @{
            Enable          = $true
            CheckInnerBrace = $true
            CheckOpenBrace  = $true
            CheckOpenParen  = $true
            CheckOperator   = $true
            CheckPipe       = $true
            CheckSeparator  = $true
        }

        PSAlignAssignmentStatement = @{
            Enable         = $true
            CheckHashtable = $true
        }

        PSUseCorrectCasing     = @{
            Enable             = $true
        }
    }
}

The second is to install an older version of PSSA module on the system.

Install-Module PSScriptAnalyzer -MaximumVersion 1.17 -Force -Scope AllUsers

This method also avoided the issue.

@bergmeister
Copy link
Contributor

bergmeister commented Apr 7, 2019

Today, I could finally get some more information:
First of all, this issue can only be repro-ed within the integrated PowerShell session of the extension. There seem to be 2 issues;

  • The PSUseCorrectCasing rule of PSSA is the one that applies the correction but when I attaches the debugger to it, the rule itself has no issue BUT the CommandInfo that it retrieves from PSSA's CommandInfo cache is wrong, it asks for 'gci' but gets the 'get-childitem' CommandInfo of type Cmdlet and I see two duplicate items of it in the cache, maybe the implementation of the CommandInfoKey class used in the cache's dictionary needs to be reviewed. This is more complex and needs more time to be understood or fixed
  • I added a "powershell.codeformatting.useCorrectCasing" setting to the vscode extension to allow switching this rule on and off. For some reason the integration of this configuration has broken and the rule cannot be turned off any more (which would be the way to suppress this bug in the meantime). At the time when I created the PR, I tested this manually so something must've changed since then. We should focus our efforts to make this new setting work again and turn it off by default for the moment.

It's sad that things like this happen but even if we hadn't enabled this feature flag by defaul I think we probably wouldn't have received the feedback in time either. What do you users and maintainers think? Would you rather prefer new features to be not enabled by default in the first iteration?

@rjmholt
Copy link
Contributor

rjmholt commented Apr 10, 2019

Would you rather prefer new features to be not enabled by default in the first iteration?

Yeah, I think making the feature available in the first iteration and then enabling it in the second iteration is the wisest choice. It's not an uncommon pattern. A good example is the way Rust handles feature stabilisation.

@bergmeister
Copy link
Contributor

I found the issue that is causing some of the other alias to cmdlet expansion in the UseCorrectCasing rule. It is due to this legacy line that adds CommandInfo objects to the cache where the key is the alias and the value is the looked up value. The rule is then retrieving this incorrect cache value. Removing this legacy code has been a pain for a while now but at least this shows now cases where we should try to change the behaviour and understand better what the code should and should not do. A quick solution might be that the legacy method uses its own cache or we try to get rid of this legacy, I myself originally voted for keeping the legacy behaviour as I feared it might lead to a regression in existing code, but now that we understand the problem better, we can fix it in a way that makes the code better and reduce the area of commonly shared legacy code.

@SydneyhSmith SydneyhSmith added Triage Issue-Bug A bug to squash. and removed Needs-Repro-Info labels Apr 11, 2019
@SydneyhSmith
Copy link
Collaborator

SydneyhSmith commented Apr 11, 2019

Closing this as it is a bug in ScriptAnalyzer being tracked with issue PowerShell/PSScriptAnalyzer#1209 . The coming release of the VSCode will have this turned off by default as to not break users. This has been fixed with #1852 which will be in the next release of PSScriptAnalyzer.

@ili101
Copy link

ili101 commented Dec 19, 2019

So now that you disabled the automatic expending by default so 1..10|%{$_*2} no longer expends to 1..10 | ForEach-Object { $_ * 2 } how do I tern this feature back on?
I tried adding to the setting the following line:
"powershell.scriptAnalysis.settingsPath": "C:\\Users\\illym\\.vscode\\extensions\\ms-vscode.powershell-preview-2019.12.0\\modules\\PSScriptAnalyzer\\1.18.3\\Settings\\PSGallery.psd1"
but it didn't work, please help.

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Dec 19, 2019
@bergmeister
Copy link
Contributor

bergmeister commented Dec 19, 2019

@ili101 With the new powershell.codeFormatting.autoCorrectAliases setting that I added, see release notes. I suggest you revert your PSSA settings path change though.
If you feel this was helpful to you, please consider sponsoring me as I am a community contributor: https://github.com/sponsors/bergmeister

@ili101
Copy link

ili101 commented Dec 19, 2019

@bergmeister I looked over the setting list twice and missed it somehow... thank you.
Unfortunately after enabling it I found out it's good thing it's off because like the old bugs leading to it being disabled using it still brakes my scripts :/
& ..\..\Target.ps1 becomes & Get-..\..\Target.ps1 PowerShell/PSScriptAnalyzer#1369

@SydneyhSmith SydneyhSmith removed the Needs: Maintainer Attention Maintainer attention needed! label Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants
@sandscap-sc @ili101 @rjmholt @bergmeister @mkht @SydneyhSmith and others