Skip to content

Indentation don't follow the same indentation as the row with the open brace #1032

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
johlju opened this issue Jul 7, 2018 · 8 comments
Closed

Comments

@johlju
Copy link
Contributor

johlju commented Jul 7, 2018

Steps to reproduce

$string = '
$test | 
Get-Something | 
Where-Object -FilterScript {
$_.a -eq 1
}
'

Invoke-Formatter -ScriptDefinition $string

Expected behavior

$test |
    Get-Something |
    Where-Object -FilterScript {
        $_.a -eq 1
    }

Actual behavior

$test |
    Get-Something |
    Where-Object -FilterScript {
    $_.a -eq 1
}

Environment data

> $PSVersionTable

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

> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }

1.17.1
@bergmeister
Copy link
Collaborator

I am not sure why the current implementation behaves like that (if it was by design from the author or just a case that got forgotten) but I think your expected behaviour makes sense to, therefore I think we should try to correct it.

@fourpastmidnight
Copy link

I was about to submit a new issue and saw the title of this issue and thought just maybe it might be related. Good thing I checked, because I think this is exactly my issue. Here's what I was going to submit in a new issue:

Steps to reproduce

Just a quick note, other than enabling the rule, the default settings for indentation are used: 4 spaces.

function Get-SomeDataById {
    [CmdletBinding()]
    param (
        [Parameter(Mandatory = $True, Position = 1, ValueFromPipeline = $True, ValueFromPipelineByPropertyName = $True)]
        [string[]]$Id

    Process {
        foreach ($i in $Id) {
            try {
                $Results = [hashtable]@{}

                # Pretend I'm processing the results of some other action...
                $Results.Add($i, @(foreach ($Action in 1..10)) {
                    [PSCustomObject]@{
                        PSTypeName = 'MyCustomType'
                        Id = "$Action"
                    }
                }))
            } catch {
                throw [Exception]::new("Something went wrong.", $_.Exception)
            }
        }
    }
}                   

Expected behavior

I expected no warnings to be produced (when Severity is set to Warning), as the code is all nicely indented.

Actual behavior

RuleName   : PSUseConsistentIndentation
Severity   : Warning
ScriptName : Get-SomeDataById.ps1
Line       : 14
Message    : Indentation not consistent

RuleName   : PSUseConsistentIndentation
Severity   : Warning
ScriptName : Get-SomeDataById.ps1
Line       : 15
Message    : Indentation not consistent


RuleName   : PSUseConsistentIndentation
Severity   : Warning
ScriptName : Get-SomeDataById.ps1
Line       : 16
Message    : Indentation not consistent


RuleName   : PSUseConsistentIndentation
Severity   : Warning
ScriptName : Get-SomeDataById.ps1
Line       : 17
Message    : Indentation not consistent

RuleName   : PSUseConsistentIndentation
Severity   : Warning
ScriptName : Get-SomeDataById.ps1
Line       : 18
Message    : Indentation not consistent

I didn't expect these warnings since the code is consistently indented! There's no ragged left edge anywhere. I have a feeling that the logic used to detect when indentation should occur is not as smart as it should be, taking a brief look at the code. It appears (at a terse glance) that when certain characters are seen (i.e. {, (, [, etc.) that the rule expects a new line to occur, and of course then, the proper 4-space indentation. However, this kind of naive logic leads to very long listings of code and too many levels of indentation. This results in people turning off the rule (which I'm about to do) even though, in theory, this is a sound rule to enforce to keep the code looking nice.

Environment data

> $PSVersionTable
PS C:\WINDOWS\system32> $PSVersionTable

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

> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.17.1

@fourpastmidnight
Copy link

Hmm, my "analysis" is off, as it appears given the OP's description, the analyzer appears to "mis-count" the amount of indentation required--which of course, is what I meant (and have experienced in other places of my code), but not at all what I said. Anyway, just wanted to post my example, too.

@bergmeister
Copy link
Collaborator

bergmeister commented Jan 24, 2019

@fourpastmidnight
The original issue will be fixed by PR #1102 that makes the rule handle multi-line pipeline statements. I can have a brief look at your example but I suspect it might get fixed as part of that as well.

@ThubLives
Copy link

I'm seeing a case where the indentation of closing braces is still falling down when the pipelineIndentationStyle option is set to anything but NoIndentation.

If the pipeline is on a single line, but includes a multi-line script block (e.g. ForEach-Object) the indentation goes sideways. I'm using PSScriptAnalyzer 1.18.1. For example:

# Some arbitrary script blocks to test indenting.
if ($true) {
  if (-not $false) {
    # Do some important work.
    $ListOfThings = 'Thing A', 'Thing B', 'Doohickey C', 'Thing D'

    # The script block all crammed onto one line is fine.
    $ListOfThings | ForEach-Object { if ($_ -imatch 'thing') { Write-Host """$_"" is a thing." } else { Write-Host """$_"" is not a thing." } }

    # The same script block in multi-line form is fine as long as the opening
    # brace doesn't share the line with another pipeline object.
    $ListOfThings |
      ForEach-Object {
        if ($_ -imatch 'thing') {
          Write-Host """$_"" is a thing."
        } else {
          Write-Host """$_"" is not a thing."
        }
      }

    # The same script block sharing its opening brace on the same line as
    # another pipeline object.
    $ListOfThings | ForEach-Object {
      if ($_ -imatch 'thing') {
        Write-Host """$_"" is a thing."
      } else {
        Write-Host """$_"" is not a thing."
      }
    }
    # This line and the following brace get mismatched!
  }
}

@mrboring
Copy link

I think I may have the same issue. This occurs in VS Code.

The following code results in a PSUseConsistentIndentation problem. It refers to the second from last closing brace:

Describe -Name 'Test' -Fixture {
    It -Name 'Test' -Test {
        {1 / 0 -or `
                2 / 0} | Should throw
    }
}

If I format the document using Shift+Alt+F I get:

Describe -Name 'Test' -Fixture {
    It -Name 'Test' -Test {
        {1 / 0 -or `
                2 / 0} | Should throw
}
}

Notice that the second from last closing brace has been unindented.

Strangely, if I move the Should throw to a new line, and format the document, the PSUseConsistentIndentation problem goes away:

Describe -Name 'Test' -Fixture {
    It -Name 'Test' -Test {
        {1 / 0 -or `
                2 / 0} |
            Should throw
    }
}

VS Code

Version: 1.36.1 (user setup)
Commit: 2213894ea0415ee8c85c5eea0d0ff81ecc191529
Date: 2019-07-08T22:59:35.033Z
Electron: 4.2.5
Chrome: 69.0.3497.128
Node.js: 10.11.0
V8: 6.9.427.31-electron.0
OS: Windows_NT x64 10.0.18362

Installed Extensions for VS Code

alefragnani.Bookmarks
Damien.autoit
eamodio.gitlens
ms-vscode.csharp
ms-vscode.powershell
ms-vscode.powershell-preview
streetsidesoftware.code-spell-checker
vscode-icons-team.vscode-icons

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      6.2.2
PSEdition                      Core
GitCommitId                    6.2.2
OS                             Microsoft Windows 10.0.18362 
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }

1.18.1
1.18.0
1.18.0

Various Settings Files

settings.json
PSScriptAnalyzerSettings.psd1
CustomRule.psm1

Various configuration files.zip

@bergmeister
Copy link
Collaborator

bergmeister commented Jul 28, 2019

@ThubLives I can repro your issue for PSSA 1.18.1 but PR #1261 already fixed this case, therefore your case will be fixed in the next version. In the meantime, the NoIndentation setting is there as fallback (as this PipelineIndentation feature is quite new). Thanks for taking the time to submit feedback though. :-)

@mrboring Thanks for your examples and info, unfortunately, I can repro your issue even with the latest version of master. But I'll have a look into it so that we can get it fixed in the next version. I might extract your details into a separate issue as it seems that all other comments in this issue were fixed by PR #1261

@bergmeister
Copy link
Collaborator

bergmeister commented Aug 17, 2019

Closing this now as the issue of @ThubLives is already fixed in master and will be in the next release.
I extracted the issue of @mrboring that still applies to master into issue #1311 and opened PR #1312 for a fix for it.

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

No branches or pull requests

5 participants