1.18.0 - Incorrect indentation of brace after pipe

[hbuckle]

Steps to reproduce

$def = @'
function hello {
  if ($true) {
    Write-Host "hello"
  }
  if ($true) {
    "hello" | Out-Host
  }
}
'@
Invoke-Formatter -ScriptDefinition $def -Settings CodeFormattingOTBS

Expected behavior

This is the output when using 1.17.1

function hello {
    if ($true) {
        Write-Host "hello"
    }
    if ($true) {
        "hello" | Out-Host
    }
}

Actual behavior

This is the output when using 1.18.0

function hello {
    if ($true) {
        Write-Host "hello"
    }
    if ($true) {
        "hello" | Out-Host
}
}

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      6.1.3
PSEdition                      Core
GitCommitId                    6.1.3
OS                             Microsoft Windows 10.0.17763
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0
> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.18.0
1.17.1

[bergmeister]

Thanks, very good and concise repro :-)
This seems to be a regression in the PSUseConsistentIndentation rule that got introduced in 1.18.0, which is not specific to CodeFormattingOTBS. It seems the number of emitted DiagnosticRecords are the same but the last DiagnosticRecord has a wrong correction.
The following works fine so it seems to be indirectly caused by the pipeline

$def = @'
function hello {
  if ($true) {
    Write-Host "hello"
  }
  if ($true) {
    Write-Host "hello2"
  }
}
'@
Invoke-Formatter -ScriptDefinition $def -Settings CodeFormattingOTBS

Therefore the problem shows up in this simpler scenario as well:

$def = @'
function hello {
  if ($true) {
    "hello" | Out-Host
  }
}
'@
Invoke-Formatter -ScriptDefinition $def -Settings CodeFormattingOTBS

I made improvements to the multi-line pipeline indentation behaviour in PSUseConsistentWhiteSpace and added the PipelineIndentation configuration to it in the recent release but it seems I need to review the logic and look at this problem in the debugger to see why it gets it wrong, sorry for that. Using the IncreaseIndentationForFirstPipeline or IncreaseIndentationAfterEveryPipeline, the issue happens but not with NoIndentation, which would be a viable work-around at the moment. You can change the PipelineIndentation configuration setting in the CodeFormattingOTB.psd1 file itself, which you find in the Settings folder in PSSA installation folder.

[PrzemyslawKlys]

I actually noticed that in VSCode as well. after updating to the newest version when using formatting when it finds pipeline 1 line above, everything under it gets moved closer to the left.

[bergmeister]

I guess the best option would be either to set the default setting in the next vscode extension update to NoIndentation to fallback to 1.17.1 behaviour or try fix it in PSSA and ship a patch. WDYT @TylerLeonhardt @rjmholt

[TylerLeonhardt]

@bergmeister it seems that option 1 is the fastest so lets go with that. I had snapped some builds already but seeing this I can snap some more.

[DarkLite1]

Same issue after upgrading to 1.18.0:

Describe 'test' {
    Context 'test' {
        It 'test' {
            $true | Should -BeTrue
    }
    It 'test' {
        $true | Should -BeTrue
}
It 'test' {
    $true | Should -BeTrue
}
}
}

[fatherjack]

Seeing this too. It's badly reformatting all my code!

Example here is just start of issues in a function. Once things have moved left they stay left all the way to the end of the file

> gmo -list PSScriptAnalyzer

ModuleType Version    Name  
---------- -------    ----  
Script     1.18.0     PSScriptAnalyzer 

FWIW - the suggested work around of editing PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline' to be PipelineIndentation = 'NoIndentation' has no discernable affect for me

[bergmeister]

@DarkLite1 @fatherjack Your cases will be fixed with the next update of the vscode extension, which will change the default setting to NoIndentation, where this bug does not surface but behaviour is still better than with 1.17.1. At the moment, the extension is not aware of this setting yet and the PSSA internal default applies to it. For the moment, you could uninstall PSSA 1.18.0 if it annoys you.

Just to give a bit of background in case some people are concerned:

1. PSScriptAnalyzer 1.17.1 did not handle multi-line pipelines well. With more than 2 lines the indentation was mostly inconsistent and I'd even say that the behaviour was ill-defined as the code did not cater for it at all.
2. In 1.18.0 I improved it to handle multi-line pipelines. Because at this point I had to introduce logic where people might have different style preferences, I made the new multi-line behaviour configurable (i.e. the indentation after the first line), those are the 3 new settings: IncreaseIndentationAfterEveryPipeline, IncreaseIndentationForFirstPipeline and NoIndendation
3. Unfortunately there are lots of cases to consider and although different test cases were written, there was one flaw in the logic in the case when the setting was IncreaseIndentationAfterEveryPipeline or IncreaseIndentationForFirstPipeline. This is the reason why changing the default to NoIndendation makes the bug not appear any more.
4. The good news is that although users will not be able to use the first 2 settings (that might be more popular for most users), the behaviour will be as it was in PSSA 1.17.1, except that the multi-line behaviour will not be ill-defined as it used to be and therefore the bug fix that I did has still lead to some improvement, therefore this PR just means that part of a new feature will not be available right now until there is a fix in PSSA.

Concluding, it was unfortunate that this happened but at least with this we can protect the user's from it without any noticeable, negative impact, which shows that configurable settings or flags can really be helpful. Unfortunately there are many different cases and unfortunately some special ones are only found once it is in the wild. I hope that in the future, PSSA publishes preview releases (this is not entirely in my hand) and that user's are understandable, the feedback from users is very valuable though and we try to be as response as possible (in 1.17, a patch to the signing problem and a reported NullReferenceException was released only a few days later)

[fatherjack]

@bergmeister Hi, thanks for the detailed response. In reference to your points:
1 - I never noticed any bad indenting around lines with more than 3 or 4 pipelines.
2 - no comment
3 - As mentioned in my report of the issue, using "NoIndentation" doesnt resolve the issue in my experience. Can you please confirm the file I should edit and precise syntax I need to replace?
4 - no comment

Do you have any estimate on when the next update of the extension is likely to be please?

[bergmeister]

@fatherjack
Re 1: Details about the isssue that got fixes are in #1066
Re 3: How are you formatting? The vscode extension is not aware of this new setting yet, hence why changing the PSSA settings (either in the settings files that PSSA ships with or your custom settings in PSSA settings file) do not apply to behaviour in the editor. Only the scriptanalysis can be customized inside vscode, the formatting is hard-coded or configurable via the exposed settings (that are not available yet), hence why at the moment the PSSA internal defaults apply in the editor.
You can use the Invoke-Formatter cmdlet of PSSA to test the NoIndentation setting by either editing the settings file that you use/prefer or specify the Settings as a hashtable (in either case to the -Settings parameter of it. I have tested your use case and it is OK using this setting.

The vscode extension will probably release this week, for editor related issues you can also just uninstall 1.18.0 for the moment

[DarkLite1]

@bergmeister thanks for the detailed feedback and the work you put into this. It's easy for users to complain if they didn't write the code themselves, I know how that feels. In some languages they have a saying like "you can't make an omelet without breaking eggs". So no worries, I'm sure it gets fixed in the remaining 2 feature flags too, when you or someone else has some time.

In the meantime, reverted back to 1.17.1.