Formatting .where and .foreach methods is incorrect

[DarkLite1]

When using the methods .where and .foreach in VS Code there's an incorrect indentation of the closing brackets and a space is added after the method.

Expected behavior

@().foreach({

})

Actual behavior

@().foreach( {

    })

Environment data

Name                           Value
----                           -----
PSVersion                      5.1.14409.1018
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.14409.1018
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.17.1

Related to an issue in the PowerShell repo.

[DarkLite1]

Thank you @bergmeister , let's hope it's fixed already. Does the new version of PSScriptAnalyzer ship with the latest VS Code Insiders?

[bergmeister]

@DarkLite1
Sorry, things got busy the last days, I'll try to have a look at this this week.
VSCode or Insiders does not ship PSSA, it is the Powershell extension that comes with the latest version of PSSA (currently 1.17.1). We hope to release 1.18 this month, which will contain a lot of formatter fixes and features. Together with PSSA, you can expect the Powershell extension to be updated accordingly

[bergmeister]

@DarkLite1 This behaviour is still present in the current development branch of PSSA A quick look at the verbose log shows that the PSUseConsistentWhitespace and PSUseConsistentIndentation rule emit 1 DiagnosticRecord each. PSUseConsistentWhitespacecauses the 1st curly brace to move one character to the right andPSUseConsistentIndentation` seems to cause the indentation of the 2nd brace. I think the 1st indentation is by design but I'd have to look into the other one.

[DarkLite1]

Thank you for your time @bergmeister , I appreciate the effort. Would this also be included in the PoserShell Preview extension? As we're using that one for the the PSReadLine support.

It seems weird that the first indentation is by design. If that would be the case, then the closing bracket needs one indentation too I believe.

Some examples, I think it should be like this:

@().foreach({

})

But if that is not possible, this would be the next best thing:

@().foreach( {

} )

Although we strongly recommend option A if possible. Keeps it more concise.

[bergmeister]

Yes, the Powershell preview extension still contains PSSA but this is just a convenience so that it works out of the box. You can still manually install your preferred version, which will take precedence.
If you don't like the space of the first brace, then you could turn off the CheckOpenBrace option of the UseConsistentWhiteSpace rule.

[DarkLite1]

Thank you, can it be this much customized that you can achieve the following:

if (1 -ea 1) {

}

@().foreach({

})

So that after the if closing ) and before the starting { there is a space? And for a method it's all glued together?

We prefer to use the PowerShell Preview for the PSReadLine support. so we'll wait for PSSA to updated in the preview release.

[bergmeister]

No, the CheckOpenBrace customisation cannot be configured to achieve both your cases, you'll have to choose between always having a space after an opening brace or never. All those customisation features are already available in the current release. I don't see anything that would change for your cases in the next release as the release is imminent and the issue of PSUseConsistentIndentation still needs to be looked into. You can feel free to open a PR with a fix for it, we might be able to squeeze it in before the release this month

[DarkLite1]

If I had the skills I would definitely make a PR. But I'm not at home in other languages except for PowerShell. Thanks anyhow for the help.

[DarkLite1]

On a side note, I would like to argue that a method is not the same as an if case. So I think it deserves a different treatment. We should get some more opinions on this, because I think more people would like to have a different white space treatment for a method then for a logical decision case. @rkeithhill , what do you think?

[rkeithhill]

Yup, IMO this treatment should be like parameters. You wouldn't allow "foo".Substring( 1, 2 ). You'd fix that to "foo".Substring(1, 2)

[DarkLite1]

Still the same issue in PSScriptAnalyzer version 1.18.0.

Here is some test code:

$Processes = Get-Process

$Processes | Where-Object {
    ($_.ProcessName -eq 'PowerShell') -and ($_.Company -like 'Microsoft Corporation*')
}

$Processes.where( {
        ($_.ProcessName -eq 'PowerShell') -and ($_.Company -like 'Microsoft Corporation*')
    })

Notice

1. Indentation incorrect for the clause within the .where method
2. Closing bracket incorrect, not aligned to the left side
3. An extra space is added where it's not needed .where{ (
4. The color of the .where method is the same as for a variable, this is most likely a VS Code issue rather than a PSScriptAnalyzer issue I think.

[msftrncs]

Adding my 2 cents:

$Processes.where( {
        ($_.ProcessName -eq 'PowerShell') -and ($_.Company -like 'Microsoft Corporation*')
    })

The indention here is correct. At the end of the first line the indent level is 2, not 1, because both ( and { each add to the indention, and on the last line, the indention only removes 1 level for the first character (token) starting the line leaving 1 indent, because that's how logic works. This is because indention removal rules only looks at the first token for the entire line. This is definitely a VS Code thing as its how all other languages I use in VS Code behave as well.

What's flawed is the lack of space missing from between the } and the ). Either add no space between ( and { (treating the formatting with the 'special function' methods specially) or add a space between } and ) so they are treated consistently.

I would vote to remove the space:

The brace is not next to a keyword, so this rule should not apply.

@DarkLite1, you should check out PowerShell/EditorSyntax#156, for a sample of an improved grammar for PowerShell that will resolve issue 4. I have a version of the grammar file already in JSON format ready for VSCode in my own repository list.

[msftrncs]

I might also quickly point out that this behavior does not occur with the alternative syntax:

$Processes.where{
    ($_.ProcessName -eq 'PowerShell') -and ($_.Company -like 'Microsoft Corporation*')
}

So this behavior only happens between '(' and '{', and isn't related directly to the use of the special function method.

[DarkLite1]

Thank you for the feedback @msftrncs , much appreciated. With this part I totally agree with you:

What's flawed is the lack of space missing from between the } and the ). Either add no space between ( and { (treating the formatting with the 'special function' methods specially) or add a space between } and ) so they are treated consistently.

And indeed, it would be best to have no space between them. That would already solve one thing.. Then there's still the incorrect coloring of the .where method and the double indention that needs to become one.

[msftrncs]

I just noticed looking at ShowPSAst.psm1, that this is not just unique to the specialized foreach() and where() methods, but also any method which may be passed a scriptblock literal.

        $script:BufferIsDirty = $false
        $scriptView.Add_TextChanged( { $script:BufferIsDirty = $true })
        $scriptView.Add_KeyUp( { OnTextBoxKeyUp @args })

https://github.com/lzybkr/ShowPSAst/blob/master/ShowPSAst.psm1

The file itself is inconsistent with relationship to spacing, I had formatted the script at some point while evaluating it and got the result shown above.

[DarkLite1]

Indeed, the current logic for spacing is incorrect for methods that are being passed a scriptblock. Just like @rkeithhill wrote:

You wouldn't allow "foo".Substring( 1, 2 ). You'd fix that to "foo".Substring(1, 2)

[msftrncs]

You wouldn't allow "foo".Substring( 1, 2 ). You'd fix that to "foo".Substring(1, 2)

Actually, I wouldn't allow "foo".Substring( 1, 2). Either of the quoted-reply examples are acceptable.

Aside, in StructuredText (IEC 61131-3) I use function( parameter ) to separate it from groupings, base + (cos( angle ) + 1) * Length. Because of this, you might experience this in my PowerShell code as well.

[JVimes]

Also seeing this for ValidateScript. Before:

function Invoke-Build([ValidateScript({ /*...*/ })] $param1)
{ }

And after:

#                                     ⮦Space     ⮦No space
function Invoke-Build([ValidateScript( { /*...*/ })] $param1)
{ }

The spacing within the parenthesis is not symmetrical, and I'd expect it to be.