Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

whitespaceBetweenParameters Removing Redirectors #2000

Open
6 tasks done
nixuno opened this issue Apr 30, 2024 · 2 comments · May be fixed by #2001
Open
6 tasks done

whitespaceBetweenParameters Removing Redirectors #2000

nixuno opened this issue Apr 30, 2024 · 2 comments · May be fixed by #2001

Comments

@nixuno
Copy link

nixuno commented Apr 30, 2024

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

ms-vscode.powershell@2024.2.1

Steps to Reproduce

  1. Save the following to test.ps1:
Import-Module -Name Module 3>&1 2>&1 1>$null
  1. Press Shift + Alt + F to format the document.
  2. Review improper formatting

Visuals

No response

Logs

No response

@SydneyhSmith SydneyhSmith transferred this issue from PowerShell/vscode-powershell May 7, 2024
@SydneyhSmith
Copy link
Collaborator

Thanks @nixuno we were able to reproduce this

@liamjpeters
Copy link
Contributor

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.

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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants