Unable to format document in special circumstances #1994

PrzemyslawKlys · 2024-04-10T11:12:24Z

Prerequisites

I have written a descriptive issue title.
I have searched all open and closed issues to ensure it has not already been reported.
I have read the troubleshooting guide.
I am sure this issue is with the extension itself and does not reproduce in a standalone PowerShell instance.
I have verified that I am using the latest version of Visual Studio Code and the PowerShell extension.
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

[email protected]

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 · 2024-04-10T13:29:31Z

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 · 2024-04-10T17:51:51Z

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

PrzemyslawKlys · 2024-04-10T19:52:28Z

No I didn't, sorry.

andyleejordan · 2024-04-10T20:36:54Z

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 · 2024-04-10T21:16:22Z

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

andyleejordan · 2024-04-10T21:31:24Z

Thanks so much @PrzemyslawKlys!

liamjpeters · 2024-04-12T10:43:51Z

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

Specifically from:

PSScriptAnalyzer/Rules/UseConsistentIndentation.cs

Lines 165 to 177 in a754b95

    
           case TokenKind.LParen: 
        
               // When a line starts with a parenthesis and it is not the last non-comment token of that line, 
        
               // then indentation does not need to be increased. 
        
               if ((tokenIndex == 0 || tokens[tokenIndex - 1].Kind == TokenKind.NewLine) && 
        
                   NextTokenIgnoringComments(tokens, tokenIndex)?.Kind != TokenKind.NewLine) 
        
               { 
        
                   onNewLine = false; 
        
                   lParenSkippedIndentation.Push(true); 
        
                   break; 
        
               } 
        
               lParenSkippedIndentation.Push(false); 
        
               AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine); 
        
               break;

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?

PrzemyslawKlys added the Issue-Bug label Apr 10, 2024

andyleejordan transferred this issue from PowerShell/vscode-powershell Apr 10, 2024

liamjpeters linked a pull request Apr 12, 2024 that will close this issue

PSUseConsistentIndentation: Check indentation of lines where first token is a LParen not followed by comment or new line #1995

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unable to format document in special circumstances #1994

Unable to format document in special circumstances #1994

PrzemyslawKlys commented Apr 10, 2024

ninmonkey commented Apr 10, 2024 •

edited

Loading

andyleejordan commented Apr 10, 2024

PrzemyslawKlys commented Apr 10, 2024

andyleejordan commented Apr 10, 2024

PrzemyslawKlys commented Apr 10, 2024

andyleejordan commented Apr 10, 2024

liamjpeters commented Apr 12, 2024 •

edited

Loading

Unable to format document in special circumstances #1994

Unable to format document in special circumstances #1994

Comments

PrzemyslawKlys commented Apr 10, 2024

Prerequisites

Summary

PowerShell Version

Visual Studio Code Version

Extension Version

Steps to Reproduce

Visuals

Logs

ninmonkey commented Apr 10, 2024 • edited Loading

You can reproduce it with this

contrary, working case

Another fail case

andyleejordan commented Apr 10, 2024

PrzemyslawKlys commented Apr 10, 2024

andyleejordan commented Apr 10, 2024

PrzemyslawKlys commented Apr 10, 2024

andyleejordan commented Apr 10, 2024

liamjpeters commented Apr 12, 2024 • edited Loading

ninmonkey commented Apr 10, 2024 •

edited

Loading

liamjpeters commented Apr 12, 2024 •

edited

Loading