Formatter should check for end-of-line comment when moving opening brace to same line

[daviwil]

From @MRWeather on October 25, 2017 16:30

PS v1.0> (Get-CimInstance Win32_OperatingSystem).version
10.0.15063

PS v1.0> code -v
1.17.2
b813d12980308015bcd2b3a2f6efa5c810c33ba5
PS v1.0> $pseditor.EditorServicesVersion
PS v1.0> code --list-extensions --show-versions
donjayamanne.python@0.7.0
ms-mssql.mssql@1.2.0
ms-vscode.PowerShell@1.4.3
ms-vsts.team@1.122.0
PS v1.0> $PSVersionTable

Name Value

---

PSVersion 5.1.15063.608
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.15063.608
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

Any variation of code formatting that moves the opening brace to the same line should check for an end-of-line comment:

before formatting

foreach ($s in $y ) # loop through $y
{
Write-Host $s
}

after formatting, the opening brace becomes part of the comment

foreach ($s in $y ) # loop through $y {
Write-Host $s
}
I don't code comments that way, but I have run into it several times when formatting other's code.

Copied from original issue: PowerShell/vscode-powershell#1069

[daviwil]

Ouch! That's definitely wrong. I'll move this issue over to the PSScriptAnalyzer repo.

[bergmeister]

@rkeithhill @SeeminglyScience @tylerl0706 @rjmholt
This bug still repro-es in 1.16.1 and the current state of the development branch. By debugging it, it confirms that the correction extent of the rule to be fixed is PSPlaceOpenBrace and I will look into it. My proposal is to append the comment after the new brace.
However, what concerned me when debugging this, is that subsequent rules that were being run as part of the style, were throwing parser errors due to the invalid syntax (which is expected). The PowerShellEditorServices should not use the correction at all if PSSA throws an error, or is there a reason why errors are being ignored and the correction getting applied? Invoke-Formatter should probably stop processing the script on the first error as well and not return a string at all.

[TylerLeonhardt]

Interesting, so the correction would be applied and then since the script was in a bad state, Invoke-Formatter would throw parse errors?

Yeah, we should stop processing the script as soon as we see the first parse error.

[rkeithhill]

@tylerl0706 Looks like we should make sure to pick up a newer version of PSSA for 1.6.1.

[bergmeister]

@rkeithhill We plan to release in the next upcoming weeks (Jim Truher has to finish some compliance work form the MS side first) so I am not sure if it is worth the effort. The next release will also include 2 new rules and a ton of other fixes for false positives, etc.

[TylerLeonhardt]

Good to know. I'll be watching on Twitter for the next release @bergmeister! Looking forward to it! (I'll reach out if we're releasing before you... we might wait)

[bergmeister]

I will definitely let you know when it gets released but the timeline depends mainly on Jim. There are 3 soon to be merged PRs (I just addressed their PR comments) that I tagged with 1.17 that should be merged before the release and those 3 I recommend you to take as well. After that you could potentially take the latest developmemt version of it and even if the final version of PSSA might have more additional PRs but they are not going to be that important to you. However, if PSSA needs to pass the new compliance rules then I would think that pseditorservices needs to do that as well but I will leave this fun up to you.
Are you planning to fix the error handling of pseditorservices?

[TylerLeonhardt]

Ah yes. Compliance :)

Are you planning to fix the error handling of pseditorservices?

Are you referring to this: PowerShell/vscode-powershell#1232

If so, that's the plan :)

[bergmeister]

Ok.
I was referring to this whereby from Invoke-Formatter was throwing parse errors but the editorservices seem to ignore them and accept the resulting string.

[TylerLeonhardt]

Oh right, yes. I've got to look into that part of the code so I'll add that to my list. Would you mind opening an issue on PSES for that so we can track it? If you're busy I can do it tonight :)