Skip to content

PSUseConsistentIndentation indents inconsistently in function body after param block #1241

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

Closed
rjmholt opened this issue May 13, 2019 · 7 comments

Comments

@rjmholt
Copy link
Contributor

rjmholt commented May 13, 2019

From @ebmarquez in PowerShell/vscode-powershell#1958.

Repro:

$script = @'
function verb-noun {
    <#
        .SYNOPSIS
        example

        .INPUTS
        String
        System.Management.Automation.PSCredential

        .OUTPUTS
        System.Management.Automation.PSCustomObject

        .NOTES

    #>
    [CmdLetBinding()]
    [OutputType([PSCustomObject])]
    Param(

        # Computer
        [Parameter(Mandatory = $true)]
        [ValidateNotNullOrEmpty()]
        [String]
        $Computer,

        # Credential
        [Parameter(Mandatory = $true)]
        [System.Management.Automation.PSCredential]
        $Credential
    )

    $cmd = Get-ShowCommands | Where-Object { $_.Name -eq 'running-config' }
$result = Invoke-NetDeviceShowCommand -Computer $Computer -Credential $Credential -Command $cmd.CommandLine
Add-Member -InputObject $result -MemberType NoteProperty -Name "Name" -Value $cmd.Name
return $result
'@

$PSUseConsistentIndentation = @{ 
            Enable = 'true'
            IndentationSize     = 4;
             PipelineIndentation = 'IncreaseIndentationForFirstPipeline'; 
             Kind                = 'space'
        }
$rules = @{
    PSUseConsistentIndentation = $PSUseConsistentIndentation;
}

Invoke-Formatter -ScriptDefinition $script -Settings @{ 
    IncludeRules = @('PSUserConsistentIndentation')
    Rules        = $rules
}

Actual Output:

function verb-noun {
    <#
        .SYNOPSIS
        example

        .INPUTS
        String
        System.Management.Automation.PSCredential

        .OUTPUTS
        System.Management.Automation.PSCustomObject

        .NOTES

    #>
    [CmdLetBinding()]
    [OutputType([PSCustomObject])]
    Param(

        # Computer
        [Parameter(Mandatory = $true)]
        [ValidateNotNullOrEmpty()]
        [String]
        $Computer,

        # Credential
        [Parameter(Mandatory = $true)]
        [System.Management.Automation.PSCredential]
        $Credential
    )

    $cmd = Get-ShowCommands | Where-Object { $_.Name -eq 'running-config' }
$result = Invoke-NetDeviceShowCommand -Computer $Computer -Credential $Credential -Command $cmd.CommandLine
Add-Member -InputObject $result -MemberType NoteProperty -Name "Name" -Value $cmd.Name
return $result
}

Notice $cmd = ... is indented differently to $result = ... onward.

I would expect $result = ... etc to indent as $cmd = ... has.

I'll let @ebmarquez follow up.

@ebmarquez
Copy link

ebmarquez commented May 13, 2019

Thanks @rjmholt for opening this in the correct repo. Psscriptanalyzer is not indening correct after the param block. I've tried to set if to the correct location and PsScriptAnalyzer moving the lies forward.

>     $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.18362.1
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.18362.1
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

> Get-Module -Name PsScriptAnalyzer

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Script     1.18.0     PSScriptAnalyzer                    {Get-ScriptAnalyzerRule, Invoke-Formatter, Invoke-ScriptAnalyzer}

Example of syntax before: Test.psm1

function Test-Config {
    <#
        .SYNOPSIS
        Get test-Config

        .INPUTS
        String
        System.Management.Automation.PSCredential

        .OUTPUTS
        System.Management.Automation.PSCustomObject

        .NOTES

    #>
    [CmdLetBinding()]
    [OutputType([PSCustomObject])]
    Param(

        # Computer
        [Parameter(Mandatory = $true)]
        [ValidateNotNullOrEmpty()]
        [String]
        $Computer,

        # Credential
        [Parameter(Mandatory = $true)]
        [System.Management.Automation.PSCredential]
        $Credential
    )

    $cmd = Get-Commands | Where-Object { $_.Name -eq 'running-config' }
    $result = Invoke-NetDeviceCommand -Computer $Computer -Credential $Credential -Command $cmd.CommandLine
    Add-Member -InputObject $result -MemberType NoteProperty -Name "Name" -Value $cmd.Name
    return $result
}

After PsScriptAnalizer formats the syntax.

    $rules = @{
>>         PSUseConsistentIndentation = @{
>>             Enable = $true
>>             IndentationSize = 4
>>             PipelineIndentation = 'IncreaseIndentationAfterEveryPipeline'
>>             Kind = 'space'
>>         }
>>     }
C:\Users\emarq> $scriptDefinition = Get-Content -Raw C:\Temp\Test.psm1
C:\Users\emarq> Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings @{
>>     IncludeRules = @('PSUserConsistentIndentation')
>>     Rules        = $rules
>> }
function Test-Config {
    <#
        .SYNOPSIS
        Get test-Config

        .INPUTS
        String
        System.Management.Automation.PSCredential

        .OUTPUTS
        System.Management.Automation.PSCustomObject

        .NOTES

    #>
    [CmdLetBinding()]
    [OutputType([PSCustomObject])]
    Param(

        # Computer
        [Parameter(Mandatory = $true)]
        [ValidateNotNullOrEmpty()]
        [String]
        $Computer,

        # Credential
        [Parameter(Mandatory = $true)]
        [System.Management.Automation.PSCredential]
        $Credential
    )

    $cmd = Get-Commands | Where-Object { $_.Name -eq 'running-config' }
$result = Invoke-NetDeviceCommand -Computer $Computer -Credential $Credential -Command $cmd.CommandLine
Add-Member -InputObject $result -MemberType NoteProperty -Name "Name" -Value $cmd.Name
return $result
}

@msftrncs
Copy link

I think this is similar to #1236, if you turn off the indention for pipe's, it will format this correctly.

@johlju
Copy link
Contributor

johlju commented Jun 7, 2019

Saw this problem using IncreaseIndentationAfterEveryPipeline. It will also format correctly if writing the code correctly.
I think the problem is that it should first add a new line after each | before formatting. See next comment.

Assume this code.

if ($true)
{
    Get-ChildItem | Get-Content

    return
}

This fails when formatting.

if ($true)
{
    Get-ChildItem | Get-Content

return
}

Assume this code

if ($true)
{
    Get-ChildItem |
    Get-Content

    return
}

Formats correctly.

if ($true)
{
    Get-ChildItem |
        Get-Content

    return
}

@johlju
Copy link
Contributor

johlju commented Jun 7, 2019

I think the best solution is if the pipe is followed by something then do not format the pipe.

For Pester tests this would be better too.

Assume this code.

It 'My test' {
    $myValue1 | Should -Be 1
    $myValue2 | Should -Be 2

    Assert-MockCalled -CommandName Get-Content -Exactly -Times 1 -Scope It
}

That would end up (if the formatting would work) as

It 'My test' {
    $myValue1 | 
        Should -Be 1
    $myValue2 | 
        Should -Be 2

    Assert-MockCalled -CommandName Get-Content -Exactly -Times 1 -Scope It
}

Might not be the preference for tests. So if the user want to have that formatting then the user self must add row breaks after the first pipe.

@msftrncs
Copy link

The example from @rjmholt seems to format correctly now with 1.18.1.

@johlju
Copy link
Contributor

johlju commented Jun 17, 2019

Yes, I would say this issue is resolved. To get the indentation the user must self move the right part to the next line, as per my previous comment. I'm happy with this.

@bergmeister
Copy link
Collaborator

bergmeister commented Jun 18, 2019

Yes, I tested all examples and they are all resolved in 1.18.1. The fix for it was in PR #1191.
Note that even with the fixes in 1.18.1, people found another edge case, PR 1261 is open for that already.

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

No branches or pull requests

5 participants