PSScriptAnalyzer icon indicating copy to clipboard operation
PSScriptAnalyzer copied to clipboard

Unable to format document in special circumstances

Open PrzemyslawKlys opened this issue 1 year ago • 7 comments

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
            }
        }
    }

image

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

[email protected]

Steps to Reproduce

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

Visuals

No response

Logs

No response

PrzemyslawKlys avatar Apr 10 '24 11:04 PrzemyslawKlys

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

ninmonkey avatar Apr 10 '24 13:04 ninmonkey

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

andyleejordan avatar Apr 10 '24 17:04 andyleejordan

No I didn't, sorry.

PrzemyslawKlys avatar Apr 10 '24 19:04 PrzemyslawKlys

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.

andyleejordan avatar Apr 10 '24 20:04 andyleejordan

image

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

PrzemyslawKlys avatar Apr 10 '24 21:04 PrzemyslawKlys

Thanks so much @PrzemyslawKlys!

andyleejordan avatar Apr 10 '24 21:04 andyleejordan

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?

liamjpeters avatar Apr 12 '24 10:04 liamjpeters