PSPlaceCloseBrace can't handle empty lines

[Jaykul]

Basically, it gets fixed if I have:

}
else {

But it doesn't get fixed if I have:

}

else {

And I would really like it to be able to handle:

}
# An explanation of the next code block
else {

And format it like:

} else {
    # An explanation of the next code block

Steps to reproduce

Invoke-Formatter {
if (Test-Path ReadMe.md) {
    Show-Markdown ReadMe.md
}

elseif (Test-Path ReadMe*) {
    Get-Item ReadMe* | Select-Object -First 1 | Get-Content
} 
} -Settings @{
  Rules = @{
    PSPlaceCloseBrace = @{
      Enable = $true
      NoEmptylineBefore = $true
      NewLineAfter = $false
    }
  }
}

Expected behavior

It should output

if (Test-Path ReadMe.md) {
    Show-Markdown ReadMe.md
} elseif (Test-Path ReadMe*) {
    Get-Item ReadMe* | Select-Object -First 1 | Get-Content
} 

Actual behavior

if (Test-Path ReadMe.md) {
    Show-Markdown ReadMe.md
}

elseif (Test-Path ReadMe*) {
    Get-Item ReadMe* | Select-Object -First 1 | Get-Content
} 

Environment data

> $PSVersionTable
$PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.1.0
PSEdition                      Core
GitCommitId                    7.1.0
OS                             Microsoft Windows 10.0.19041
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.19.1
1.18.1
1.18.0
1.17.1

[rjmholt]

Yeah I think the rule should handle the empty line by removing it. There are a bunch of issues around these formatting rules, mostly caused by the fact that they process things at the token level rather than at the AST level.

Moving the comment though, I think is out of scope for us. I note you're asking not only for the comment to be moved, but also moved into a block and indented. Comments only exist as tokens, and moving the comment means reordering tokens, which gets dicey. If the rule operates at the token level, we have to implicitly understand the structure of the tokens and then move back and forth around them (rather than just laying them down in order with different whitespace as currently happens). If we move to being an AST rule, the best way to keep comments is to track the comment tokens by extent as we progress along. But then moving tokens around starts to get tricky again because of the lookahead and backtracking needed. Just putting comments in the right place as we format tokens/an AST is already hard, but reordering presents a new level of difficulty.

But also I think that messing with the comments and how they order wrt other program tokens is something that many users would object to. Comments are primarily human-facing, and unlike with PowerShell semantics, PSSA can't know whether moving a comment is going to be "valid" in the mind of the consumer of the comment. So comments are something I think PSSA should take a conservative approach with.

[Jaykul]

The problem isn't just about automatically refactoring it though! Neither of these cases are flagged by the rule in Invoke-ScriptAnalyzer in the first place.

Invoke-ScriptAnalyzer -ScriptDefinition {
# GOOD: This produces a violation
if (Test-Path ReadMe.md) {
    Show-Markdown ReadMe.md
}
elseif (Test-Path ReadMe*) {
    Get-Item ReadMe* | Select-Object -First 1 | Get-Content
}

# BAD: Adding a comment "#" fixes the violation!?
if (Test-Path ReadMe.md) {
    Show-Markdown ReadMe.md
} #
elseif (Test-Path ReadMe*) {
    Get-Item ReadMe* | Select-Object -First 1 | Get-Content
}

# BAD: Adding whitespace fixes the violation
if (Test-Path ReadMe.md) {
    Show-Markdown ReadMe.md
}

elseif (Test-Path ReadMe*) {
    Get-Item ReadMe* | Select-Object -First 1 | Get-Content
}
}.ToString() -Settings @{
  Rules = @{
    PSPlaceCloseBrace = @{
      Enable = $true
      NoEmptylineBefore = $true
      NewLineAfter = $false
    }
  }
}

So yeah ... I can live with the comments blocking FIXING it because it's "hard" and there's probably not a one-size fits all place to put those comments, but the rule needs to FLAG it as being wrong.