PowerShell / PSScriptAnalyzer

Download ScriptAnalyzer from PowerShellGallery
https://www.powershellgallery.com/packages/PSScriptAnalyzer/
MIT License
1.8k stars 366 forks source link

The PSAvoidTrailingWhitespace rule is not applied when using Invoke-Formatter #1992

Open Ju-l1a opened 2 months ago

Ju-l1a commented 2 months ago

Steps to reproduce

Using Invoke-Formatter does not apply the PSAvoidTrailingWhitespace rule as shown by the following example script:

$script = @"
Function Get-Example {   
    'Example'   
}   
"@

$settings = @{
    IncludeRules = @("PSAvoidTrailingWhitespace")
}

$formatted = Invoke-Formatter -ScriptDefinition $script -Settings $settings

Out-File -FilePath ./result.ps1 -InputObject $formatted -Encoding utf8 -NoNewline

Invoke-ScriptAnalyzer -Path ./result.ps1

Expected behavior

# There should be no linting results after formatting

Actual behavior

# There are linting results even after formatting
RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSAvoidTrailingWhitespace           Information  result.ps1 1     Line has trailing whitespace
PSAvoidTrailingWhitespace           Information  result.ps1 2     Line has trailing whitespace
PSAvoidTrailingWhitespace           Information  result.ps1 3     Line has trailing whitespace

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      5.1.22621.2506
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.22621.2506
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.22.0
Ju-l1a commented 2 months ago

This is possibly a duplicate of https://github.com/PowerShell/PSScriptAnalyzer/issues/1757

bergmeister commented 2 months ago

Invoke-Formatter has a hard-coded list of 'allowed' rules that are defined here that PSAvoidTrailingWhitespace is not part of it but I think we could add it to enable your use case :-) https://github.com/PowerShell/PSScriptAnalyzer/blob/a754b950467aa9e78a1eba1a3423bbd055ed8772/Engine/Formatter.cs#L38-L50

Ju-l1a commented 2 months ago

Adding the rule to this list (and running build.ps1& importing the module) alone does not seem to change the behavior for me. Am I missing something?

liamjpeters commented 2 months ago

It looks as though the formatter considers each of the rules in the ruleOrder array, so adding it is correct.

It does, however, skip over any rule that doesn't have (even empty) arguments passed in through settings. I'm not clear on the context of why this is done.

https://github.com/PowerShell/PSScriptAnalyzer/blob/a754b950467aa9e78a1eba1a3423bbd055ed8772/Engine/Formatter.cs#L56-L62

So for instance, once the rule is added as @bergmeister suggests:

$script = @"
Function Get-Example {   
    'Example'   
}   
"

$settings = @{
    Rules = @{
        PSAvoidTrailingWhitespace = @{}
    }
}

$formatted = Invoke-Formatter -ScriptDefinition $script -Settings $settings

Runs the rule and "fixes" it, but I get a "fixed" script that no longer includes the closing curly-brace.

Function Get-Example {
    'Example'
RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
MissingEndCurlyBrace                ParseError              1     Missing closing '}' in statement block or type definition.

This looks to be the same issue described in #1757

liamjpeters commented 2 months ago

Interestingly:

$script = @"
Function Get-Example {   
    'Example'   
 }   
"

$settings = @{
    Rules = @{
        PSAvoidTrailingWhitespace = @{}
    }
}

$formatted = Invoke-Formatter -ScriptDefinition $script -Settings $settings

Note the space before the closing curly-brace in $script

results in:

Function Get-Example {
    'Example'
 }

and

$script = @"
Function Get-Example {   
    'Example'   
 }   
Function Get-Example {   
    'Example'   
}   
Function Get-Example {   
    'Example'   
 }   
"@

$settings = @{
    Rules = @{
        PSAvoidTrailingWhitespace = @{}
    }
}

$formatted = Invoke-Formatter -ScriptDefinition $script -Settings $settings

results in:

Function Get-Example {
    'Example'
 }
Function Get-Example {
    'Example'

Function Get-Example {
    'Example'
 }

So it looks like PSAvoidTrailingWhitespace has an issue with lines which contain trailing whitespace, but are only a single non-whitespace character in length. e.g. }.

The start column of the violationExtent calculated for the last line (}) looks to be 1:

image

It's worked out here.

https://github.com/PowerShell/PSScriptAnalyzer/blob/a754b950467aa9e78a1eba1a3423bbd055ed8772/Rules/AvoidTrailingWhitespace.cs#L56-L64

It's looking backward through the line for a non-space, non-tab character, and if it's not finding it, it's assuming the whitespace starts at column 1 (assume columns are 1-indexed and not 0-indexed?).

It's only checking up to character 1 in the string (loop condition is i > 0) and so it's stopping short of finding the curly-brace character at line[0]. If it find that, it would correctly say that the whitespace starts at column 2 (again, assuming columns are 1-indexed).

I think this bug should be simply changing i > 0 to be i >= 0.

Indeed doing so, I get correct looking results and all tests still pass. Happy to PR this, unless you want to have a go @Ju-l1a

Ju-l1a commented 2 months ago

@liamjpeters I'd be grateful if you can do the PR so I can see how it's done properly :)

Thank you for your help!