Description

Stephen (s-nix)

opened on Apr 30, 2024

Issue body actions

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

When formatting the following PowerShell while powershell.codeFormatting.whitespaceBetweenParameters = true, it removes two of the redirectors:

Pre-Format

Import-Module -Name Module 3>&1 2>&1 1>$null

Post-Format

Import-Module -Name Module 1>$null

I would expect the redirectors to be left alone.

PowerShell Version

Name                           Value
----                           -----
PSVersion                      7.4.2
PSEdition                      Core
GitCommitId                    7.4.2
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

Name             : Visual Studio Code Host
Version          : 2024.2.1
InstanceId       : 71fd0027-ddfe-4add-82a3-1e0b2dbe844e
UI               : System.Management.Automation.Internal.Host.InternalHostUserInterface
CurrentCulture   : en-US
CurrentUICulture : en-US
PrivateData      : Microsoft.PowerShell.ConsoleHost+ConsoleColorProxy
DebuggerEnabled  : True
IsRunspacePushed : False
Runspace         : System.Management.Automation.Runspaces.LocalRunspace

Visual Studio Code Version

1.88.1
e170252f762678dec6ca2cc69aba1570769a5d39
x64

Extension Version

[email protected]

Steps to Reproduce

1. Save the following to test.ps1:

Import-Module -Name Module 3>&1 2>&1 1>$null

2. Press Shift + Alt + F to format the document.
3. Review improper formatting

Visuals

No response

Logs

No response

Activity

1. Stephen (s-nix)

   added

   Issue-Bug

   on Apr 30, 2024
2. Sydney Smith (SydneyhSmith)

   transferred this issue fromPowerShell/vscode-powershellon May 7, 2024
3. SydneyhSmith commented on May 7, 2024

   

   Sydney Smith (SydneyhSmith)

   on May 7, 2024

   Collaborator

   More actions

   Thanks @nixuno we were able to reproduce this
4. Sydney Smith (SydneyhSmith)

   added

   Area - Rules

   Up for Grabs

   on May 7, 2024
5. liamjpeters commented on May 13, 2024

   

   Liam Peters (liamjpeters)

   on May 13, 2024

   Contributor

   More actions

   This is quite an interesting one 😀. A slightly simplified repro in PSScriptAnalyzer is:

   Invoke-Formatter -ScriptDefinition 'Invoke-Foo 3>&1 1>&1 2>&1' -Settings @{
       Rules = @{
           PSUseConsistentWhitespace = @{
               Enable         = $true
               CheckParameter = $true
           }
       }
   }

   Results in:

   Invoke-Foo 1>&1

   The PSUseConsistentWhitespace rule, when CheckParameter is true, is looking over all the CommandAst nodes.

   For each CommandAst, it's finding all the direct children of the current CommandAst node.

   PSScriptAnalyzer/Rules/UseConsistentWhitespace.cs

   Lines 399 to 400 in e1dc126

   	List<Ast> commandParameterAstElements = commandAst.FindAll(
   	testAst => testAst.Parent == commandAst, searchNestedScriptBlocks: false).ToList();

   It's checking then that each one sequentially is separated by a 1 character gap.

   PSScriptAnalyzer/Rules/UseConsistentWhitespace.cs

   Lines 410 to 411 in e1dc126

   	var expectedStartColumnNumberOfRightExtent = leftExtent.EndColumnNumber + 1;
   	if (rightExtent.StartColumnNumber > expectedStartColumnNumberOfRightExtent)

   It seems to be making the implicit assumption that the direct children of the CommandAst are read from the AST in token order. This is, apparently, not the case.

   Looking at the AST of our example Invoke-Foo 3>&1 1>&1 2>&1:

   ScriptBlockAst [0,25)
   └ NamedBlockAst [0,25)
     └ PipelineAst [0,25)
       └ CommandAst [0,25)
         ├ StringConstantExpression [0,10)
         ├ MergingRedirectionAst [16,20)
         ├ MergingRedirectionAst [21,25)
         └ MergingRedirectionAst [11,15)
   

   We see that evidentially, the MergingRedirectionAst nodes are read from the tree in stream order (very interesting!).

   Note the start of each MergingRedirectionAst nodes extent - the number after [ - is not in order down the list. The first node read from the tree is the second token, 1>&1, followed by the last token, 2>&1, and finally the first of the redirects in our input command, 3>&1.

   I'm not smart enough to confirm this within the main Powershell Repo's parser - but I've tried with various orders of Invoke-Foo 3>&1 2>&1 1>&1 5>&1 4>&1 6>&1 and it appears to be consistent.

   My unrefined naive solution would be to sort the command parameters (the direct children of the CommandAst), first by their Extents StartLineNumber, then by the StartColumnNumber. Then proceed as planned.

   This works as expected and our repro code correctly produces:

   Invoke-Foo 3>&1 1>&1 2>&1

   I would need to look at how badly this sorting impacts performance and for any edge cases.
6. Andy Jordan (andyleejordan)

   closed this as completedin #2001on Jul 16, 2024

Issue actions