Repository navigation
Formatter inserts redundant whitespace in ValidateSet attribute #1468
Description
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
Activity
bergmeister commentedon Jun 5, 2020
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 } }[-]Formatter insert redundant whitespace in ValidateSet attribute[/-][+]Formatter inserts redundant whitespace in ValidateSet attribute[/+]Jaykul commentedon Nov 17, 2020
YES. This drives me crazy.
bergmeister commentedon Jan 23, 2021
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:
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 likeif ($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 #1092The suggestion would be to add a third configurable option
CheckCloseBraceto 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 Joel Bennett (@Jaykul) ? This way, the default settings would be symmetric but due to the configuration option, you could turn
CheckOpenBraceandCheckCloseBraceoff if you do not like it.Jaykul commentedon Jan 24, 2021
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 commentedon Jan 24, 2021
Ok. Do I correctly understand that you want
$ast.Find({ $Other -is [TypeExpressionAst] })? This is what I am going to propose in PR #1633I 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...