Unable to format document in special circumstances

PrzemyslawKlys commented 2 months ago

Prerequisites

[X] I have written a descriptive issue title.
[X] I have searched all open and closed issues to ensure it has not already been reported.
[X] I have read the troubleshooting guide.
[X] I am sure this issue is with the extension itself and does not reproduce in a standalone PowerShell instance.
[X] I have verified that I am using the latest version of Visual Studio Code and the PowerShell extension.
[X] If this is a security issue, I have read the security issue reporting guidance.

Summary

Extension works perfectly most of the time in recent months. Today I've noticed weird issue

Where a simple code like this format mostly everything with the exception of lines with Get-Item

    $Files = Get-ChildItem -Path $Directory -File -Recurse -Include "*$ExtensionFrom"
    foreach ($File in $Files) {
        if ($File.Extension -eq $ExtensionFrom) {
            if ($OnlyNewerThan -and $File.LastWriteTime -lt (Get-Date).AddDays(-$OnlyNewerThan)) {
                #Write-Color -Text '[i]', "[TheDashboard] ", "Skipping $($File.FullName) as it's older than $OnlyNewerThan days" -Color Yellow
                continue
            }
            Write-Color -Text '[i]', "[TheDashboard] ", "Processing fixes $($File.FullName) / $($File.LastWriteTime)" -Color Yellow
            # Store original dates
            $originalCreationTime = $File.CreationTime
            $originalLastWriteTime = $File.LastWriteTime

            $Encoding = Get-FileEncoding -Path $File.FullName
            $FileContent = Get-Content -Raw -Path $File.FullName -Encoding $Encoding
            if ($FileContent -match $SearchString) {
                Write-Color -Text '[i]', "[TheDashboard] ", "Processing fixes $($File.FullName) for ($SearchString)" -Color Green
                $FileContent -replace $SearchString, $ReplaceString | Set-Content -Path $File.FullName -Encoding $Encoding

                # Restore original dates
                (Get-Item $File.FullName).CreationTime = $originalCreationTime
              (Get-Item $File.FullName).LastWriteTime = $originalLastWriteTime
            }
        }
    }

Trying to format it, leaves it as is, at the same time it formats everything else around it. So it seems there's something special about it.

PowerShell Version

Name                           Value
----                           -----
PSVersion                      7.4.1
PSEdition                      Core
GitCommitId                    7.4.1
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0.}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visual Studio Code Version

1.89.0-insider
x64

Extension Version

ms-vscode.powershell@2024.3.2

Steps to Reproduce

Add provided code to code
Make sure lines with Get-Item are not formatted
Try to format them
No formatting applies

Visuals

No response

Logs

No response

ninmonkey commented 2 months ago

I've tried a bunch of variations. You can reproduce it with almost any expression that starts with a (

You can reproduce it with this

    ($x = 10)
    $y = 30

formats as

    ($x = 10)
$y = 30

contrary, working case

     1, ($x = 10)

formats correctly

1, ($x = 10)

Another fail case

    ($x = 10), 1

# formats as the same
    ($x = 10), 1

andyleejordan commented 2 months ago

Oh weird, thanks for the report. Did you try it under PSSA directly too?

PrzemyslawKlys commented 2 months ago

No I didn't, sorry.

andyleejordan commented 2 months ago

The latest version of the extension is using PSSA 1.22, which just came out. It would be helpful to know if this repros directly with Invoke-Formatter and if so we can move the bug over there and get it fixed.

PrzemyslawKlys commented 2 months ago

I would say it's PSScriptAnalyzer issue. Will you move it?

andyleejordan commented 2 months ago

Thanks so much @PrzemyslawKlys!

liamjpeters commented 2 months ago

Stepping through Formatter.Format() this behaviour arises from the PSUseConsistentIndentation rule.

Specifically from:

https://github.com/PowerShell/PSScriptAnalyzer/blob/a754b950467aa9e78a1eba1a3423bbd055ed8772/Rules/UseConsistentIndentation.cs#L165-L177

The issue seems to be that, if the left paren ( is:

the first token (first non-whitespace character in the file) OR
the first non-whitespace character on a line

AND

isn't followed by a new line or comment

THEN that line's indentation is not checked against what we expect it to be. (Other necessary things still happen however)

Coming at it for the first time, I find the code-flow of the rule to be difficult to follow. The actual evaluation of whether the indentation is correct happens within the AddViolation() function, and only if onNewLine is true. It reads as though at each token parse, we're adding a rule violation - when in reality, checks are being performed and conditionally a violation is being added.

I believe, in the short-term, a change should be made so that in the situation outlined above, we still check the lines indentation is at the level we expect (but we don't increment the indentation level). Longer-term perhaps this rule should be revisited to make it a bit easier to follow and update it's styling slightly.

@bergmeister - this looks to be a rule you've put a lot of time and effort into, and you put together the PR that introduced this LParen check (#1469) - What do you think?

PowerShell / PSScriptAnalyzer