Unable to use both AlignAssignmentStatement and UseConsistentWhitespace rules

[gaudreaj]

Hi,

It seems like it is not possible to use both AlignAssignmentStatement and UseConsistentWhitespace rules because the first contradicts the second. UseConsistentWhitespace is enforcing that only one space is used before the operator while AlignAssignmentStatement assert that all "=" operators are aligned. The only way the analyzer would not fail is that all the keys from the hashtable are the same length ;)

To reproduce:

1. Create a script with the following code:

$hashtable = @{
    property1       = "value"
    anotherProperty = "another value"
}

2. Run the analyzer with code formatting settings:

Invoke-ScriptAnalyzer -Path MyScript.ps1 -Settings CodeFormatting

You should see a PSUseConsistentWhitespace rule warning

No warning should be the expected result

[kapilmb]

• UseConsistentWhitespace rule is more general in the sense that it takes into account any binary operator, including the assignment operator.
• AlignAssignmentStatement specifically targets property value pairs in a hashtable.

You're right that the results they produce can be contradictory when dealing with property value pairs in a hashtable and that can be confusing. However, their original goal was to be used in succession - first UseConsistentWhitespace then AlignAssignmentStatement - in the formatting tool used in the powershell extension in vscode. Looking back, it probably would've been more prudent to just have an alignment switch in UseConsistentWhitespace rule that would functionally be equivalent to the AlignAssignmentStatement rule.

However, given the current state I think it is best to ignore one of the rule or disable either AlignAssignmentStatement rule or UseConsistentWhitespace.CheckOperator switch.

[kapilmb]

Going forward, when we get a chance we can probably deprecate the AlignAssignmentRule and add an equivalent switch in the UseConsistentWhitespace rule.

@daviwil thoughts?

[daviwil]

Yep, it would be a cleaner approach to use a single rule for that. Sometimes we don't know until we get there :)

[jayvdb]

This is quite ridiculous. https://github.com/PowerShell/PSScriptAnalyzer/blob/master/Engine/Settings/CodeFormatting.psd1 defines both as enabled.

How can the supposed default style definition of PSSA have incompatible styles in it, 2 years after it was reported as not working. Is PSSA not dogfooding its own default style? The code in https://github.com/PowerShell/PSScriptAnalyzer/blob/master/Engine/Settings/CodeFormatting.psd1 fails when trying to format it with the "CodeFormatting" style, afaic.

https://github.com/PowerShell/PSScriptAnalyzer/blob/master/Engine/Settings/CodeFormattingOTBS.psd1 has this highly ironic snippet

        PSUseConsistentWhitespace  = @{
            Enable         = $true
            CheckInnerBrace = $true
            CheckOpenBrace = $true
            CheckOpenParen = $true
            CheckOperator  = $true
            CheckPipe       = $true
            CheckSeparator = $true
}

Is nobody else using this style definitions? They seem to be documentation of rules according to outside pre-existing definitions and are not rules which are actually possible with the software, and are actually tested to work as a style definition (as opposed to unit tests of each piece in isolation of the others). Are any of the coding styles actually usable?

CodeFormattingStroustrup and CodeFormattingAllman both also use CheckOperator = $true and CheckHashtable = $true.

It seems like the quick "fix" is to admit reality, and disable CheckOperator and/or CheckHashtable in all of those styles until such time as the rules are implemented in a way that supports both being used in the same style definition.