Formatter inserts redundant whitespace in ValidateSet attribute

[bergmeister]

Steps to reproduce

Format the following snippet with the default settings:

function Get-Noun
{
    param(
        [ValidateScript({ Test-Path $_ })]
        $Path
    )
}

Expected behavior

No changes to the line of ValidateScript

Actual behavior

It inserts a space after the ( of ValidateScript:

function Get-Noun
{
    param(
        [ValidateScript( { Test-Path $_ })]
        $Path
    )
}

If an unexpected error was thrown then please report the full error details using e.g. $error[0] | Select-Object *

Environment data

This happens in both PSSA 1.18.3 and 1.19.0

[bergmeister]

Also happening in this case:

BeforeAll {
    function Get-TypeNameAstFromScript
    {
        param([string]$Script)

        $ast = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$null, [ref]$null)
        $typeExpAst = $ast.Find({
            $args[0] -is [System.Management.Automation.Language.TypeExpressionAst]
        }, $true)

        return $typeExpAst.TypeName
    }
}

It got formatted to

BeforeAll {
    function Get-TypeNameAstFromScript
    {
        param([string]$Script)

        $ast = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$null, [ref]$null)
        $typeExpAst = $ast.Find( {
                $args[0] -is [System.Management.Automation.Language.TypeExpressionAst]
            }, $true)

        return $typeExpAst.TypeName
    }
}

[Jaykul]

YES. This drives me crazy.

[bergmeister]

Hmm, thinking about it again, right now, I think the right expected behaviour should maybe rather be to also insert a space after the closing brace:

function Get-Noun
{
    param(
        [ValidateScript( { Test-Path $_ } )]
        $Path
    )
}

The reason why I think that is because PSSA has two configurable rules (that can be turned on or off individually):

• CheckOpenBrace: Insert a space before open brace (has been present in PSSA for years for scenarios like if ($true) { }
• CheckInnerBrace: Insert a space inside braces, i.e. after the open brace and before the close brace. I added this in PR Add CheckInnerBrace and CheckPipe options to PSUseConsistentWhitespace #1092

The suggestion would be to add a third configurable option CheckCloseBrace to insert a space after the closing brace. Of course that would includes some logic like e.g. excluding cases where the next token is a newline, etc.

What do you think @Jaykul ? This way, the default settings would be symmetric but due to the configuration option, you could turn CheckOpenBrace and CheckCloseBrace off if you do not like it.

[Jaykul]

I want a space between a closing parenthesis or brace and an opening one, like: if ($true) { ...

But I don't want them between "open" and "open" or between "close" and "close"
I don't like this: $ast.Find( { $Other -is [TypeExpressionAst] } )

That's especially true when there are more levels of them 🤔

[bergmeister]

Ok. Do I correctly understand that you want $ast.Find({ $Other -is [TypeExpressionAst] })? This is what I am going to propose in PR #1633
I think it would be sensible to refine the logic that already checks the previous/next token so that e.g. a space gets inserted only when an opening brace is preceded by a closing brace or bracket. I haven't thought about all cases yet, so implementation details of the logic might vary (e.g. one could invert the logic to prevent a space from getting inserted when an opening brace is being preceded by an opening parenthesis)

It's interesting because the logic to insert a space after an opening brace has been there since day 1 but I guess now that I've added the space inside braces, this asymmetry becomes more apparent...