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

Accept multiple suggested corrections from PSScriptAnalyzer #2076

Open mcgurdan opened 5 years ago

mcgurdan commented 5 years ago

VSCode seems to be bomb out if a diagnostic records have multiple elements within it's SuggestedCorrections property; despite the property being specified as plural and of type ObjectModel.Collection containing [PowerShell.ScriptAnalyzer.Generic.CorrectionExtent].

This almost seems like a bug, but maybe it was just overlooked as generally there is only a single suggested correction (if any). I am currently implementing many custom rules which should always contain a suggested correction that suppresses the diagnostic record and if applicable, the fix that can be applied.

SydneyhSmith commented 5 years ago

Thanks for reporting this @mcgurdan it would be great if we could identify what the exception was....It would be really helpful if you could also attach the logs--instructions for how to do that are here

mcgurdan commented 5 years ago

Okay, so below I have created a diagnostic record using invoke-scriptanalyzer:

Line                 : 34
Column               : 5
Message              : Test
Extent               : return "hello"
RuleName             : AvoidUsingReturn
Severity             : Warning
ScriptName           : TestScript.ps1
ScriptPath           : C:\temp\TestScript.ps1
RuleSuppressionID    :
SuggestedCorrections : {Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.CorrectionExtent, Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.CorrectionExtent}

The suggested corrections property contains two CorrectionExtents (as below):

File              : AvoidJSUsingReturn
Description       : Apply Fix: Remove the use of "return"
StartLineNumber   : 34
StartColumnNumber : 5
EndLineNumber     : 34
EndColumnNumber   : 19
Text              : "test"
Lines             : {"hello"}
Start             : Microsoft.Windows.PowerShell.ScriptAnalyzer.Position
End               : Microsoft.Windows.PowerShell.ScriptAnalyzer.Position

File              : AvoidJSUsingReturn
Description       : Test
StartLineNumber   : 34
StartColumnNumber : 5
EndLineNumber     : 34
EndColumnNumber   : 19
Text              : "hello"
Lines             : {"hello"}
Start             : Microsoft.Windows.PowerShell.ScriptAnalyzer.Position
End               : Microsoft.Windows.PowerShell.ScriptAnalyzer.Position

Rather than showing all of the suggested corrections, VSCode seems to show the last: image

rjmholt commented 5 years ago

It looks like we collect all the corrections, but then it was implicitly assumed that they are unique for a given span (the dict.Add requires uniqueness).

But semantically that should give you an error.

Anyway, looking at the code this was done deliberately I suspect. @rkeithhill may know more.

If you're looking to get this working with your own custom tooling, we'd be happy to accept a PR on PSES.