PowerShell / vscode-powershell

Provides PowerShell language and debugging support for Visual Studio Code
https://marketplace.visualstudio.com/items/ms-vscode.PowerShell
MIT License
1.69k stars 486 forks source link

Code folding not always correct #1631

Closed corbob closed 4 years ago

corbob commented 5 years ago

System Details

System Details Output

### VSCode version: 1.29.1 bc24f98b5f70467bc689abf41cc5550ca637088e x64

### VSCode extensions:
CoenraadS.bracket-pair-colorizer@1.0.61
DavidAnson.vscode-markdownlint@0.21.0
eamodio.gitlens@9.0.2
EditorConfig.EditorConfig@0.12.5
eg2.tslint@1.0.40
Equinusocio.vsc-material-theme@2.6.2
kisstkondoros.vscode-codemetrics@1.17.4
ms-vscode.csharp@1.17.1
ms-vscode.PowerShell@1.10.0
wesbos.theme-cobalt2@2.1.6

### PSES version: 1.10.0.0

### PowerShell version:

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

Issue Description

I am experiencing a problem with code folding. The SendMessage functions within the PoshBot Backends for slack and teams both fold rather weirdly.

Raw files: https://raw.githubusercontent.com/poshbotio/PoshBot/master/PoshBot/Implementations/Teams/TeamsBackend.ps1 https://raw.githubusercontent.com/poshbotio/PoshBot/master/PoshBot/Implementations/Slack/SlackBackend.ps1

Expected Behaviour

It should fold the entire function.

Actual Behaviour

It folds parts of the function.

glennsarti commented 5 years ago

In https://raw.githubusercontent.com/poshbotio/PoshBot/master/PoshBot/Implementations/Teams/TeamsBackend.ps1 Line 140 folds strangely. It goes to 169 not 450

glennsarti commented 5 years ago

Okay so found the problem.

The token matching is broken.

It looks for @{ ---> } and { ----> } Note that the end markers are both the same.

In the use case above the following code snippet is confusing the folder;

Bad folder

foreach ($1 in $2) {     <----- STARTS MATCH HERE (1)

    $x = @{  <-----   STARTS MATCH HERE (2)
        'abc' = 'def'
    }        <----- ENDS   MATCH HERE (1) (2)
}

Good folder

foreach ($1 in $2) {     <----- STARTS MATCH HERE (1)

    $x = @{  <-----   STARTS MATCH HERE (2)
        'abc' = 'def'
    }        <----- ENDS  MATCH HERE (2)
}        <----- ENDS  MATCH HERE (1)

The old grammar based folder knew the difference between a } terminating a hash vs a script block. The PowerShell Tokeniser does not.

Note I found this is also the case for @( ... ), ( ... ), and $( ... )

glennsarti commented 5 years ago

I have a workaround just need to add tests because obviously this was missed.

DarkLite1 commented 5 years ago

Folding this test.ps1 file is also horrible. It might be the same issue:

$ThresholdTest = @(
    @{
        Percentage = 85
        Color      = 'Red'
        Action     = @{
            Body             = @"
Dear [Source Io Owner],

Your personal home drive has reached [Quota Threshold]% of its maximum allowed volume.

In case you need assistance in handling this issue, we kindly ask you to contact us on

Kind regards
"@
            Type             = 2
            RunLimitInterval = 2880
        }
    }
    @{
        Percentage = 100
        Color      = 'Green'
        Action     = @{
            Subject          = 'Your personal home drive has reached or exceeded its maximum allowed volume'
            Body             = @"
Dear [Source Io Owner],

Your personal home drive has now reached [Quota Threshold]% of its maximum allowed volume.
Please take into account that you will no longer be able to edit or save files on your home drive. You can either take the necessary measures to free up space on your home drive, or submit a formal request for a higher volume limit.

In case you need assistance in handling this issue, we kindly ask you to contact us on bnl.servicedesk@heidelbergcement.com

Kind regards,
IT Service Desk
"@
            Type             = 2
            RunLimitInterval = 2880
        }
    }
)
<#
    Generate a valid JSON file to convert special chars:

    $ThresholdTest | ConvertTo-Json | Out-File $File -Force
#>

#region AD Groups
$ADGroupRemoveTest = New-Object Microsoft.ActiveDirectory.Management.ADGroup Identity -Property @{
    SamAccountName = $ADGroupRemoveNameTest
    Description    = 'Home drives remove quota limit'
    CanonicalName  = 'contoso.com/EU/BEL/Groups/{0}' -f $ADGroupRemoveNameTest
    GroupCategory  = 'Security'
    GroupScope     = 'Universal'
}

$ADGroup20GBTest = New-Object Microsoft.ActiveDirectory.Management.ADGroup Identity -Property @{
    SamAccountName = "$($ADGroupNameTest) 20GB"
    Description    = 'Home drives 20GB'
    CanonicalName  = 'contoso.com/EU/BEL/Groups/{0}' -f "$($ADGroupNameTest) 20GB"
    GroupCategory  = 'Security'
    GroupScope     = 'Universal'
}
#endregion

#region AD Users
$ADUserNorrisTest = New-Object Microsoft.ActiveDirectory.Management.ADUser Identity -Property @{
    SamAccountName = 'cnorris'
    GivenName      = 'Chcuk'
    Surname        = 'Norris'
    Enabled        = $true
    Description    = 'Normal users'
    HomeDirectory  = '\\contoso\home\cnorris'
    CanonicalName  = 'contoso.com/EU/BEL/Users/{0}' -f 'cnorris'
}

$ADUserSwaggerTest = New-Object Microsoft.ActiveDirectory.Management.ADUser Identity -Property @{
    SamAccountName = 'bswagger'
    GivenName      = 'Bob Lee'
    Surname        = 'NSwagger'
    Enabled        = $true
    Description    = 'Normal users'
    HomeDirectory  = '\\contoso\home\bswagger'
    CanonicalName  = 'contoso.com/EU/BEL/Users/{0}' -f 'bswagger'
}
#endregion

Describe $sut {
    Mock Get-ADGroup
    Mock Get-ADGroupMember
    Mock Get-ADUser
    Mock Get-DFSDetailsHC {
        @{
            Path         = $ADUserNorrisTest.HomeDirectory
            ComputerName = $env:COMPUTERNAME
            ComputerPath = $null
        }
        @{
            Path         = $ADUserSwaggerTest.HomeDirectory
            ComputerName = $env:COMPUTERNAME
            ComputerPath = $null
        }
    }
    Mock Invoke-Command
    Mock Send-MailHC
    Mock Write-EventLog

    in $TestDrive {
        #region Script and SetQuotaScript need to be in the same folder
        Copy-Item -LiteralPath "$here\$sut" -Destination $TestDrive
        $here = $TestDrive.FullName
        #endregion

        #region Set stage
        $SetQuotaTestFile = 'SetQuotaTestFile.ps1'
        $SetQuotaTestScript | Out-File $SetQuotaTestFile -Encoding utf8

        $ThresholdTestFile = 'Thresholds.json'
        $ThresholdTest | ConvertTo-Json | Out-File $ThresholdTestFile

        $Log = (New-Item Log -ItemType Directory -EA Ignore).FullName
        #endregion

        Describe 'error handling' {
            BeforeEach {
                $ThresholdTest | ConvertTo-Json | Out-File $ThresholdTestFile -Force

                $Params = @{
                    ADGroupName        = $ADGroupNameTest
                    ADGroupRemoveName  = $ADGroupRemoveNameTest
                    ScriptName         = 'Test'
                    LogFolder          = $Log
                    ThresholdFile      = $ThresholdTestFile
                    SetQuotaScriptFile = $SetQuotaTestFile
                    MailTo             = $ScriptAdmin
                }
            }
            Context 'mandatory parameters' {
                it 'ScriptName' {
                    (Get-Command "$here\$sut").Parameters['ScriptName'].Attributes.Mandatory | Should -BeTrue
                } -Skip:$Skip
                it 'ADGroupName' {
                    (Get-Command "$here\$sut").Parameters['ADGroupName'].Attributes.Mandatory | Should -BeTrue
                } -Skip:$Skip
                it 'ADGroupRemoveName' {
                    (Get-Command "$here\$sut").Parameters['ADGroupRemoveName'].Attributes.Mandatory | Should -BeTrue
                } -Skip:$Skip
                it 'MailTo' {
                    (Get-Command "$here\$sut").Parameters['MailTo'].Attributes.Mandatory | Should -BeTrue
                } -Skip:$Skip
            }

            Context 'not found' {
                It 'LogFolder' {
                    $Params.LogFolder = 'NotExisting'
                    ."$here\$sut" @Params

                    Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                        (&$MailAdminParams) -and ($Message -like "*Path*not found*")
                    }
                    Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                        $EntryType -eq 'Error'
                    }
                    Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                } -Skip:$Skip
                Context 'ThresholdFile' {
                    it 'invalid JSON data in ThresholdFile' {
                        'Incorrect data' | Out-File $ThresholdTestFile -Force

                        ."$here\$sut" @Params

                        Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                            (&$MailAdminParams) -and ($Message -like "*Threshold file*does not contain valid (JSON) data*")
                        }
                        Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                        Assert-MockCalled Get-ADUser -Exactly 0 -Scope It
                        $Users | Should -BeNullOrEmpty
                    } -Skip:$Skip
                    It "ThresholdFile given but it's not available" {
                        $Params.ThresholdFile = 'NotExisting'
                        ."$here\$sut" @Params

                        Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                            (&$MailAdminParams) -and ($Message -like "*Threshold file*not found*")
                        }
                        Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                            $EntryType -eq 'Error'
                        }
                        Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                    } -Skip:$Skip
                    It 'Percentage' {
                        @{
                            #Percentage = 85
                            Color  = 'Red'
                            Action = @{
                                Subject          = "Subject"
                                Body             = "Body"
                                Type             = 2
                                RunLimitInterval = 2880
                            }
                        }| ConvertTo-Json | Out-File $ThresholdTestFile -Force

                        ."$here\$sut" @Params

                        Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                            (&$MailAdminParams) -and ($Message -like "*Threshold*Percentage*not found*")
                        }
                        Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                            $EntryType -eq 'Error'
                        }
                        Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                    } -Skip:$Skip
                    It 'Color' {
                        @{
                            Percentage = 85
                            #Color = 'Red'
                            Action     = @{
                                Subject          = "Subject"
                                Body             = "Body"
                                Type             = 2
                                RunLimitInterval = 2880
                            }
                        }| ConvertTo-Json | Out-File $ThresholdTestFile -Force

                        ."$here\$sut" @Params

                        Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                            (&$MailAdminParams) -and ($Message -like "*Color*not found*")
                        }
                        Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                            $EntryType -eq 'Error'
                        }
                        Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                    } -Skip:$Skip
                    It 'Action' {
                        @{
                            Percentage = 85
                            Color      = 'Red'
                            #Action = @{
                            #    Subject = "Subject"
                            #    Body    = "Body"
                            #    Type = 2
                            #    RunLimitInterval = 2880
                            #}
                        }| ConvertTo-Json | Out-File $ThresholdTestFile -Force

                        ."$here\$sut" @Params

                        Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                            (&$MailAdminParams) -and ($Message -like "*Action*not found*")
                        }
                        Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                            $EntryType -eq 'Error'
                        }
                        Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                    } -Skip:$Skip
                    Context 'Action' {
                        It 'MailTo' {
                            @{
                                Percentage = 85
                                Color      = 'Red'
                                Action     = @{
                                    Subject          = "Subject"
                                    Body             = "Body"
                                    Type             = 2
                                    RunLimitInterval = 2880
                                }
                            }| ConvertTo-Json | Out-File $ThresholdTestFile -Force

                            ."$here\$sut" @Params

                            Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                                (&$MailAdminParams) -and ($Message -like "*MailTo*not found*")
                            }
                            Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                                $EntryType -eq 'Error'
                            }
                            Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                        } -Skip:$Skip
                        It 'Subject' {
                            @{
                                Percentage = 85
                                Color      = 'Red'
                                Action     = @{
                                    MailTo           = 'Brecht.Goijbels@heidelbergcement.com'
                                    #Subject = "Subject"
                                    Body             = "Body"
                                    Type             = 2
                                    RunLimitInterval = 2880
                                }
                            }| ConvertTo-Json | Out-File $ThresholdTestFile -Force

                            ."$here\$sut" @Params

                            Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                                (&$MailAdminParams) -and ($Message -like "*Subject*not found*")
                            }
                            Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                                $EntryType -eq 'Error'
                            }
                            Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                        } -Skip:$Skip
                    }
                }
                It 'SetQuotaScriptFile' {
                    $Params.SetQuotaScriptFile = 'NotExisting.ps1'
                    ."$here\$sut" @Params

                    Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                        (&$MailAdminParams) -and ($Message -like "*Quota script file*not found*")
                    }
                    Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                        $EntryType -eq 'Error'
                    }
                    Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                } -Skip:$Skip
                It 'ADGroupName' {
                    Mock Get-ADGroup

                    ."$here\$sut" @Params

                    Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                        (&$MailAdminParams) -and ($Message -like "*Couldn't find active directory groups starting with*")
                    }
                    Assert-MockCalled Write-EventLog -Exactly 1 -Scope It -ParameterFilter {
                        $EntryType -eq 'Error'
                    }
                    Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                } -Skip:$Skip
            }

            Context 'general' {
                It 'ADGroupName ending with an incorrect quota limit string' {
                    Mock Get-ADGroup {
                        $ADGroupRemoveTest

                        New-Object Microsoft.ActiveDirectory.Management.ADGroup Identity -Property @{
                            SamAccountName = "$($ADGroupNameTest) Wrong"
                            Description    = 'Home drives 20GB'
                            CanonicalName  = 'contoso.com/EU/BEL/Groups/{0}' -f "$($ADGroupNameTest) 20GB"
                            GroupCategory  = 'Security'
                            GroupScope     = 'Universal'
                        }
                    }

                    ."$here\$sut" @Params

                    Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                        (&$MailAdminParams) -and ($Message -like "*Failed converting the quota limit string*KB size*")
                    }
                    Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                    Assert-MockCalled Get-ADUser -Exactly 0 -Scope It
                    $Users | Should -BeNullOrEmpty
                } -Skip:$Skip
                It 'User in multiple groups, report error and do nothing' {
                    Mock Get-ADGroup {
                        $ADGroup20GBTest
                        $ADGroupRemoveTest
                    }
                    Mock Get-ADGroupMember {
                        $ADUserSwaggerTest
                    } -ParameterFilter {
                        $ADGroup20GBTest.SamAccountName -eq $Identity
                    }
                    Mock Get-ADGroupMember {
                        $ADUserSwaggerTest
                    } -ParameterFilter {
                        $ADGroupRemoveTest.SamAccountName -eq $Identity
                    }
                    Mock Get-ADUser {
                        $ADUserSwaggerTest
                    } -ParameterFilter {
                        $ADUserSwaggerTest.SamAccountName -eq $Identity
                    }
                    Mock Write-Error -ParameterFilter {
                        $Message -like '*member of multiple groups*'
                    }

                    ."$here\$sut" @Params

                    Assert-MockCalled Write-Error -Exactly 1 -Scope it
                    $Users | Should -BeNullOrEmpty
                } -Skip:$Skip
            }
        }
        Describe 'script execution' {
            $Params = @{
                ADGroupName        = $ADGroupNameTest
                ADGroupRemoveName  = $ADGroupRemoveNameTest
                ScriptName         = 'Test'
                LogFolder          = $Log
                ThresholdFile      = $ThresholdTestFile
                SetQuotaScriptFile = $SetQuotaTestFile
                MailTo             = $ScriptAdmin
            }

            It 'AD group has no members, do nothing' {
                Mock Get-ADGroup {
                    $ADGroup20GBTest
                    $ADGroupRemoveTest
                }
                ."$here\$sut" @Params

                Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                    $MailParams.Subject -notlike 'Failure*'
                }
                Assert-MockCalled Get-ADGroupMember -Exactly 2 -Scope It
                Assert-MockCalled Invoke-Command -Exactly 0 -Scope It
                Assert-MockCalled Get-ADUser -Exactly 0 -Scope It
                $Users | Should -BeNullOrEmpty
            } -Skip:$Skip

            It 'One user in group with threshold, execute script' {
                Mock Get-ADGroup {
                    $ADGroup20GBTest
                    $ADGroupRemoveTest
                }
                Mock Get-ADGroupMember {
                    $ADUserSwaggerTest
                } -ParameterFilter {
                    $ADGroup20GBTest.SamAccountName -eq $Identity
                }
                Mock Get-ADUser {
                    $ADUserSwaggerTest
                } -ParameterFilter {
                    $ADUserSwaggerTest.SamAccountName -eq $Identity
                }

                ."$here\$sut" @Params

                Assert-MockCalled Send-MailHC -Exactly 1 -Scope It -ParameterFilter {
                    $MailParams.Subject -notlike 'Failure*'
                }
                Assert-MockCalled Invoke-Command -Exactly 2 -Scope It
                ($Users | Measure-Object).Count | Should -BeExactly 1
            } -Skip:$Skip
        }
    }
}

Fully folded it looks like this: image

Currently on code-insiders version: Version: 1.30.0-insider (user setup) Commit: 241923763dbeaf3b64443187e935d55b29f6125d Date: 2018-12-07T22:31:41.479Z Electron: 2.0.12 Chrome: 61.0.3163.100 Node.js: 8.9.3 V8: 6.1.534.41 OS: Windows_NT x64 6.2.9200

With extension PowerShell 1.10.1

BjornNordblom commented 4 years ago

Seems to work fine now, both the examples in original issue descriptions folds as expected.

Tested on VSCode 1.44.1 with Powershell extension 2020.3.0.