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 488 forks source link

OutputType attribute referencing a PowerShell class breaks Tab Completion #1341

Closed mattpwhite closed 4 years ago

mattpwhite commented 6 years ago

System Details

Win10 1703 VS Code 1.23 Extension 1.7.0

$psversiontable

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

Issue Description

Here's a repro:

class Foo {
    [int] $Bar

    Foo([int]$bar) {
        $this.Bar = $bar
    }
}

function Get-Foo {
    [CmdletBinding()]
    #[OuputType([Foo])]
    param(
        [Parameter(Mandatory = $true)]
        [int]
        $Bar
    )

    return [Foo]::new($Bar)
}

Get-Foo -

Press Ctrl+Space at the bottom and you'll be offered -Bar and the common parameters as completion suggestions. Now go un-comment the OutputType attribute. Now ctrl+space will fall back to suggesting random tokens from the file as parameter names.

Logs attached. logs.zip

rjmholt commented 6 years ago

Thanks for opening an issue! @tylerl0706 and I have been looking into this. Does this work if you fix the typo in "OuputType" to make it "OutputType"?

rjmholt commented 6 years ago

In terms of using an attribute that doesn't exist, I feel like we should be more helpful in identifying that situation. Some notes on this:

SeeminglyScience commented 6 years ago

Copying the code into the PowerShell console gives the same behaviour -- so it seems to be a behaviour in the tab completion engine itself

Beat me to it :) was about to reply with something to that effect.

@mattpwhite As a work around you can use [OutputType('Foo')] instead.

mattpwhite commented 6 years ago

Sorry, the typo confused things.

Yes, an invalid attribute will break completion entirely. That's always been the case and I'm not asking for any change around that. It might be nice if someday this was caught at parse time rather than not until the function is called (PS 2.0 used to be stricter about this in both good and bad ways), but that's a totally different issue.

If you fix the typo, the completion failure behavior is still present in VS Code (works fine in a shell). Using a string type name rather than a bracketed type name fixes IntelliSense completion for the parameters in VS Code, but you still still lose out on completion of the members. I guess that's not the end of the world, but it's at least a little counter-intuitive/not as helpful as it could be.

I'm assuming that's because there is no type "Foo" in the PSES session, and there would have to be in order for this to work. I also assume that's why it chokes when you use the [Foo] syntax - that's not a type in the PSES session and if you put [OutputType([SomeTypeThatDoesNotExist])] on something you won't get completion in a shell or VS Code.

For example, here's what you get when accessing members of a return value from a function declares a DateTime OutputType:

image

And here's what you get when the OutputType is "Foo" (or [Foo]):

image

So, I guess this might be hard to fix unless there is some way to have types defined as classes in the PS file being edited come to exist in the PSES session?

rjmholt commented 6 years ago

Ah that's very helpful! Thanks for following up with all the extra data points.

If you've got time, definitely open a new issue on PowerShell about not getting parse-time checks on attributes and losing completions because of attribute typos (assuming there isn't one for it already).

For the VSCode-extension specific behaviour, we'll take a look and see if we can work it out.

TylerLeonhardt commented 6 years ago

@mattpwhite I can't seem to repro the

(Get-Foo 3). tab completion

In a shell. Can you send me a screenshot if that working for you?

We use the same completion command (TabExpansion2) that the shell uses so I would expect both of these to fail but if only VSCode is failing, this could be another issue.

mattpwhite commented 6 years ago

Here is a gif of it working with an overlay of the keystrokes to show tab completion working in a shell. Does that help?

outputtype

rjmholt commented 6 years ago

This might be the greatest repro ever

mattpwhite commented 6 years ago

ScreenToGif is a pretty great little program. https://github.com/NickeManarin/ScreenToGif

rjmholt commented 6 years ago

I suspect this might be related to the fact that PowerShellEditorServices doesn't handle classes properly yet, basically because we're forced to use the PowerShell v3 AST for compatibility.

See https://github.com/PowerShell/PowerShellEditorServices/issues/14 and https://github.com/PowerShell/vscode-powershell/issues/1310.

mattpwhite commented 6 years ago

Sounds pretty plausible, thanks for the pointer.

Seems like the discussion has largely concluded on those threads, but FWIW, I agree with the idea that PS3/4 support should be dropped/forked to simplify things and free up dev cycles for more useful things (especially general perf/stability improvements). Every version of PowerShell has come with breaking changes, documented/intentional and otherwise. Even smaller bumps like 4.0->5.0 (various COM things) and 5.0->5.1 (5.1's circular dependency checker is busted) broke real world code that I had deployed.

If you're trying to ship something (internally or externally) non-trivial that needs to run across multiple versions of PS with a high level of assurance that it will actually work, you already need CI boxes running tests on different versions of PS. Or you wing it, hope for the best and fix things when they break. Either might make sense for a given situation, but keeping v3 or 4 on the box someone uses to write code doesn't really buy you anything.

Halkcyon commented 6 years ago

It works in the console because you're loading the class into the session. If you do the same in VSCode, it'll work the same.

pcgeek86 commented 4 years ago

Has any progress been made on this? I just ran into this issue, and found it had already been created almost 2 years ago. It seems that the issue is only with custom PowerShell v5 classes, when using this syntax: [OutputType([MyClass])]. The work-around previously mentioned seems to work okay: [OutputType('MyClass')].

This Works

Screen Shot 2020-02-06 at 8 45 43 AM
class Car {
  [string] $Manufacturer
  [string] $Model
  [int] $Year
}

function Get-Car {
  [OutputType('Car')]
  [CmdletBinding()]
  param (

  )
}

### Remove the . and re-add it, to invoke Intellisense. CTRL + SPACE may be required as well.
(Get-Car).

This Also Works

Screen Shot 2020-02-06 at 8 47 53 AM
class Car {
  [string] $Manufacturer
  [string] $Model
  [int] $Year
}

function Get-Car {
  [OutputType([System.IO.FileStream])]
  [CmdletBinding()]
  param (

  )
}

(Get-Car).

This Doesn't Work

Screen Shot 2020-02-06 at 8 46 54 AM
class Car {
  [string] $Manufacturer
  [string] $Model
  [int] $Year
}

function Get-Car {
  [OutputType([Car])]
  [CmdletBinding()]
  param (

  )
}

(Get-Car).
SeeminglyScience commented 4 years ago

This should probably be moved to PowerShell/PowerShell. All we do is display the results that the engine gives us.

$script = 'class Car {
  [string] $Manufacturer
  [string] $Model
  [int] $Year
}

function Get-Car {
  [OutputType([Car])]
  [CmdletBinding()]
  param (

  )
}

(Get-Car).'

TabExpansion2 -inputScript $script -cursorColumn $script.Length

Returns:

CurrentMatchIndex ReplacementIndex ReplacementLength CompletionMatches
----------------- ---------------- ----------------- -----------------
               -1              161                 0 {}

So there's nothing that can be done in the extension itself.

SydneyhSmith commented 4 years ago

@SteveL-MSFT can you please transfer this issue to the PowerShell/PowerShell repo?

pcgeek86 commented 4 years ago

@SeeminglyScience good test, thanks for pointing that out. I didn't think about calling tabexpansion2 directly.