PowerShell / vscode-powershell

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

ValidateSet/Register-ArgumentCompleter/Enum completions does not work correctly #2982

Open PrzemyslawKlys opened 4 years ago

PrzemyslawKlys commented 4 years ago

Issue Description

I am experiencing a problem with... completions failing for 2nd entry.

ThisWayWeird

Notice that it works on 14th line, and then very same command fails on line 10th.

function Get-GPOZaurrPermissionSummary {
    [cmdletBinding()]
    param(
        [validateSet('AuthenticatedUsers', 'DomainComputers', 'Unknown', 'WellKnownAdministrative', 'NotWellKnown', 'NotWellKnownAdministrative', 'NotAdministrative', 'Administrative', 'All')][string[]] $Type = 'All',
        [validateSet('Allow', 'Deny', 'All')][string] $PermitType = 'All',
        [ValidateSet('GpoApply', 'GpoEdit', 'GpoCustom', 'GpoEditDeleteModifySecurity', 'GpoRead', 'GpoOwner', 'GpoCustomCreate', 'GpoCustomOwner')][string[]] $IncludePermissionType,
        [ValidateSet('GpoApply', 'GpoEdit', 'GpoCustom', 'GpoEditDeleteModifySecurity', 'GpoRead', 'GpoOwner', 'GpoCustomCreate', 'GpoCustomOwner')][string[]] $ExcludePermissionType,
        [Microsoft.GroupPolicy.GPPermissionType[]] $Test,

        [alias('ForestName')][string] $Forest,
        [string[]] $ExcludeDomains,
        [alias('Domain', 'Domains')][string[]] $IncludeDomains,
        [System.Collections.IDictionary] $ExtendedForestInformation,

        [string] $Separator
    )

I tried to replicate it to give you reproducible code but I just can't. I'm experiencing it across all my modules and I can't pinpoint it to what I'm doing wrong. It fails the same way for both [Microsoft.GroupPolicy.GPPermissionType[]] which is an enum, but also have the same issue with ValidateSet and Register-ArgumentCompleter that makes me want to cry :-)

Attached Logs

1601583262-5511c227-6bca-4824-9bad-6f29a894ed501601499246405.zip

Follow the instructions in the README about capturing and sending logs.

Environment Information

Visual Studio Code

Name Version
Operating System Windows_NT x64 10.0.19042
VSCode 1.49.2
PowerShell Extension Version 2020.9.0

PowerShell Information

Name Value
PSVersion 5.1.19041.541
PSEdition Desktop
PSCompatibleVersions 1.0 2.0 3.0 4.0 5.0 5.1.19041.541
BuildVersion 10.0.19041.541
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

Visual Studio Code Extensions

Visual Studio Code Extensions(Click to Expand) |Extension|Author|Version| |---|---|---| |better-toml|bungcip|0.3.2| |csharp|ms-dotnettools|1.23.2| |errorlens|usernamehw|3.2.2| |github-linker|gimenete|0.2.3| |gitlens|eamodio|10.2.2| |line-endings|steditor|1.0.3| |LogFileHighlighter|emilast|2.9.0| |markdown-all-in-one|yzhang|3.3.0| |material-icon-theme|PKief|4.3.0| |open-in-browser|techer|2.0.0| |powershell-preview|ms-vscode|2020.9.0| |project-manager|alefragnani|11.3.0| |rainbow-brackets|2gua|0.0.6| |run-in-powershell|tobysmith568|1.1.0| |swdc-vscode|softwaredotcom|2.3.12| |vscode-markdownlint|DavidAnson|0.37.0| |vscode-toggle-quotes|BriteSnow|0.3.3| |vscode-wakatime|WakaTime|4.0.8| |vscode-yaml|redhat|0.11.1| |xml|DotJoshJohnson|2.5.1|
SydneyhSmith commented 4 years ago

Thanks @PrzemyslawKlys for the detailed information, apologies for my confusion but can you please clarify the GIF...what are you expecting to see on line 10 that you are not seeing? Thanks!

PrzemyslawKlys commented 4 years ago

Notice how 2nd element for Test parameter doesnt have proper autocompletion. Its just some cache from code not autocompletion. I have this behavior for validateset, enums, and so on. It works if its single entry. As soon as i expect array of entries only first one works. But again... not always. Works fine on line 14, and on line 10 same parameter for same command gives junk on 2nd and 3rd element.

SydneyhSmith commented 4 years ago

Thanks @PrzemyslawKlys we are trying to determine whether this issue is occurring in PowerShell or in Editor Services, but it is a bit difficult for us to check without the full program. If you take this program and find the offset of where your completion is in relation to the script, you should be able to use tabexpansion2 to determine if the error in completion reproduces.


TabExpansion2 -inputScript @'
import-module MyModule
get-process -Name
get-childitem
'@ -cursorColumn 41 | %{​​​​ $_.CompletionMatches }​​​​
PrzemyslawKlys commented 4 years ago

I am getting some errors when trying to replicate it using your code, but even for the simple stuff I can replicate it.

function New-Function {
    [cmdletBinding()]
    param(
        [validateSet('New', 'Old', 'Other')][string[]] $Test1,
        [validateSet('New', 'Old', 'Other')][string[]] $Test2
    )
}
New-Function -Test1 New, Old -Test2 New,

New-Function -Test1 New -Test2 New

New-Function -Test2 New, Old, Other -Test1 Other, New, Old

<#
New-HTMLDiagram {
    New-DiagramNode -IconColor
    New-DiagramNode -IconColor AirForceBlue, AlbescentWhite -IconBrands accessible-icon
}
#>

WeirdThings2

WeirdThings1

1602058787-8d37b9bb-1adf-4331-8a15-9bf79b06e5f41602056586158.zip

Usually it seems I can replicate it if I try to edit something that's not last but is in the beginning of the file. For example if I start typing New-Function on line 8 it works, on line 9 it works and if I come back to line 7 and do the same thing it doesn't work anymore on 2nd and more entries.

rjmholt commented 4 years ago

But if you take one of the scripts where completion isn't working from your examples, turn it into a string, and send it through to TabExpansion2 with the right offset (you'll need to calculate that -- although picking some values and refining them based on output helps), do you see the right completion or no?

If it doesn't replicate with TabExpansion2, it's a bug in our server engine, but if it does, it's a bug in the completion logic.

PrzemyslawKlys commented 4 years ago

I don't know how to calculate offset :) So if you can provide me details...

I just replicated it on the simple code (no need for my fancy modules):

function New-Function {
    [cmdletBinding()]
    param(
        [validateSet('New', 'Old', 'Other')][string[]] $Test1,
        [validateSet('New', 'Old', 'Other')][string[]] $Test2
    )
}
New-Function -Test1 New, Old -Test2 New,

New-Function -Test1 New -Test2 New

New-Function -Test2 New, Old, Other -Test1 Other, New, Old

But you need to start editing from bottom to top, like I am showing on the gifs. It seems to work if it's 1st entry, 2nd entry, but as soon as I go back to 1st entry and try to modify it, it no longer works correctly.

rjmholt commented 4 years ago

I don't know how to calculate offset :) So if you can provide me details...

It's the index into the string of your script, so you have to add up all the line lengths above it, plus the column offset.

Something like this:

function Get-Offset
{
    param(
        [Parameter(Mandatory)]
        [string]
        $Str,

        [Parameter(Mandatory)]
        [int]
        $Line,

        [Parameter(Mandatory)]
        [int]
        $Column
    )

    $i = -1
    $currLine = 0
    while ($i -lt $Str.Length -and $currLine -lt $Line)
    {
        $i = $Str.IndexOf("`n", $i + 1)

        if ($i -le -1)
        {
            throw "Line out of range"
        }

        if ($i + 1 -lt $Str.Length -and $Str[$i + 1] -eq "`r")
        {
            $i++
        }

        $currLine++
    }

    return $i + $Column - 1
}

VSCode will tell you the line and column of your cursor, but in 1-based form, so just subtract 1 from the line and column

PrzemyslawKlys commented 4 years ago

I tried, I really tried.

[string] $Script1 = Get-Content -Path $PSScriptRoot\Test7.ps1 -Raw

TabExpansion2 -inputScript $Script1 -cursorColumn 269 | ForEach-Object { ​​​​ $_.CompletionMatches }​​​​

Where Test7.ps1

function New-Function {
    [cmdletBinding()]
    param(
        [validateSet('New', 'Old', 'Other')][string[]] $Test1,
        [validateSet('New', 'Old', 'Other')][string[]] $Test2
    )
}
New-Function -Test1 New -Test2 New
New-Function -Test1 New, Old -Test2 New
New-Function -Test1 New, Old -Test2 New

function Get-Offset {
    param(
        [Parameter(Mandatory)][string]$Str,
        [Parameter(Mandatory)]     [int]        $Line,
        [Parameter(Mandatory)]        [int]        $Column
    )
    $i = -1
    $currLine = 0
    while ($i -lt $Str.Length -and $currLine -lt $Line) {
        $i = $Str.IndexOf("`n", $i + 1)
        if ($i -le -1) {
            throw "Line out of range"
        }
        if ($i + 1 -lt $Str.Length -and $Str[$i + 1] -eq "`r") {
            $i++
        }
        $currLine++
    }

    return $i + $Column - 1
}

ForEach-Object : Cannot bind parameter 'RemainingScripts'. Cannot convert the "​​​​" value of type "System.String" to type "System.Management.Automation.ScriptBlock".
At C:\Support\GitHub\PSWriteHTML\Ignore\TestingFill.ps1:3 char:101
+ ...  -cursorColumn 269 | ForEach-Object { ​​​​ $_.CompletionMatches }​​​​
+                                                                      ~~~~
    + CategoryInfo          : InvalidArgument: (:) [ForEach-Object], ParameterBindingException
    + FullyQualifiedErrorId : CannotConvertArgumentNoMessage,Microsoft.PowerShell.Commands.ForEachObjectCommand
rjmholt commented 4 years ago

Do you have a stack trace for the error? It might be that the completer has an error in it and it's bubbling up -- that might also explain why completions only work sometimes.

PrzemyslawKlys commented 4 years ago

I can confirm it fails in ISE as well.

image

If you try to autocomplete line 8 after new it won't do anything in ISE.

function New-Function {
    [cmdletBinding()]
    param(
        [validateSet('New', 'Old', 'Other')][string[]] $Test1,
        [validateSet('New', 'Old', 'Other')][string[]] $Test2
    )
}
New-Function -Test1 New -Test2 New,
New-Function -Test1 New, Old -Test2 New
New-Function -Test1 New, Old -Test2 New

In code it will insert junk so it would seem PowerShell issue.

rjmholt commented 4 years ago

But if you look at the error raised by TabExpansion2, there's a coercion issue coming from what I assume is your completer script, no?

PrzemyslawKlys commented 4 years ago

I think you're reading into it too much. Just check 10 lines of code I've attached. You can reproduce it on those 10 lines. Copy/Paste into Code/ISE and try to autocomplete parameter Test2 with 3 values.

vexx32 commented 4 years ago

Quick and dirty repro:

$script = @'
function New-Function {
        [cmdletBinding()]
        param(
            [validateSet('New', 'Old', 'Other')][string[]] $Test1,
            [validateSet('New', 'Old', 'Other')][string[]] $Test2
        )
    }
    New-Function -Test1 New -Test2 
    New-Function -Test1 New, Old -Test2 New
    New-Function -Test1 New, Old -Test2 New
'@

$cursorColumn = $script.IndexOf('-Test2 ')
TabExpansion2 -inputScript $script -cursorColumn $cursorColumn | % completionmatches

The symptom seems to show up essentially when you have the same function used later on in the script, and then you return to an earlier function call to the same function and start to tab complete it. It reproduces in a completely separate console as well, safe to say it's not the VS Code extension itself.

The completions I get from this are the generic "here's a list of files from the current dir" sort of completions, completely ignoring the validateset results or not finding them for some reason.

Looks like it's down to something in PS. 😞

PrzemyslawKlys commented 4 years ago

Should I open it up in PowerShell repo? as it seems PS5.1 / 7.1 as well.

rjmholt commented 4 years ago

Ah, interesting. Yeah without the complexity of added argument completers, it looks like this is a bug in PowerShell's completion engine (which there are still a number of).

Worth opening an issue in the PowerShell repo.

PrzemyslawKlys commented 4 years ago

Would you be able to workaround it in PS 5.1. Otherwise I'm gonna die daily. I've been frustrated so much by this lack of autocompletion thinking that I do something wrong. And if PS team fixes it in PowerShell it will be 7.1 only.

SeeminglyScience commented 4 years ago

@PrzemyslawKlys in the ISE example you showed, the , character is being used as line continuation. If you look at the syntax highlighting in the ISE you'll see the second line is being treated as input to the first command.

@vexx32 IndexOf is putting the cursor index at <here>-Test2

SeeminglyScience commented 4 years ago

Also here's an editor command you can use to test if TabExpansion2 gives the same result:

Microsoft.VSCode_profile.ps1 ```powershell Register-EditorCommand -DisplayName 'Show Engine Completion Results' -Name EditorTabExpansion2 -ScriptBlock { param( [Microsoft.PowerShell.EditorServices.Extensions.EditorContext, Microsoft.PowerShell.EditorServices] $Context ) end { function GetLineMap { [OutputType([int[]])] param([string] $text) end { $lineStartMap = [System.Collections.Generic.List[int]]::new(100) $lineStartMap.Add(0) for ($i = 0; $i -lt $text.Length; ++$i) { $c = $text[$i]; if ($c -eq "`r"[0]) { if (($i + 1) -lt $text.Length -and $text[$i + 1] -eq "`n"[0]) { $i++; } $lineStartMap.Add($i + 1) } if ($c -eq "`n") { $lineStartMap.Add($i + 1); } } return ,$lineStartMap.ToArray() } } $fileContext = $Context.CurrentFile $scriptAst = $fileContext.Ast $scriptTokens = $fileContext.Tokens $scriptText = $scriptAst.Extent.Text $lineStartMap = GetLineMap $scriptText $offset = ($lineStartMap[$Context.CursorPosition.Line - 1] + $Context.CursorPosition.Column) - 1 $newPosition = $scriptAst.Extent.StartScriptPosition.GetType(). GetMethod( <# name: #> 'CloneWithNewOffset', <# bindingAttr: #> [System.Reflection.BindingFlags]'NonPublic, Public, Instance', <# binder: #> $null, <# types: #> [type[]][int], <# modifiers: #> $null). Invoke($scriptAst.Extent.StartScriptPosition, @($offset)) $completionMatches = [System.Management.Automation.CommandCompletion]:: CompleteInput($scriptAst, $scriptTokens, $newPosition, @{}). CompletionMatches if (-not $completionMatches) { Write-Host Write-Host 'CompleteInput returned no matches.' return } $completionMatches | Out-Host } } ```
keybindings.json ```json { "key": "ctrl+shift+space", "command": "PowerShell.InvokeRegisteredEditorCommand", "args": { "commandName": "EditorTabExpansion2" }, "when": "editorTextFocus && editorLangId == 'powershell'", }, ```
PrzemyslawKlys commented 4 years ago

@SeeminglyScience - well

image

Nothing shows up on line 8

image

rjmholt commented 4 years ago

Would you be able to workaround it in PS 5.1

I'm not sure. Our policy has generally been that the actual completions are the province of PowerShell and PSES just hooks them up. There are a number of cases now where it would be nice to improve on the behaviour though.

I suspect to do it properly, we would need to spin PowerShell's completer library into its own package, ship it with PSES and override TabExpansion2 in PSES on WinPS. I'm not sure how feasible that is, or whether I could convince the people I need to convince that it's worth it.

PrzemyslawKlys commented 4 years ago

Well autocomplete is useless for arrays it seems. You can't expect people using autocomplete only on last instance. I often go back to older code and it never works.

SeeminglyScience commented 4 years ago

As a work around, it seems like if you type an extra , and tab complete before it it works.

e.g. New-Function -Test1 New -Test2 Other,, > Left Arrow > Tab

PrzemyslawKlys commented 4 years ago

Doesn't work for me:

image

Edit... oh it works

image

I guess I could use it that way 💯

SeeminglyScience commented 4 years ago

FYI I updated the editor command above and added a key bind of Ctrl + Shift + Space. Hopefully that makes it a little easier to test this kind of thing.

Also, another work around is adding a pipe, e.g.

# This works, remove the | and it doesn't.
New-Function -Test1 New,<tab> |
somethingelsehere
PrzemyslawKlys commented 4 years ago

COol :-D I will be using the comma workaround in near future.