diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index 24b4d52e3..0676fccf1 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -31,14 +31,15 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index bb398e8f9..a0c649457 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -31,14 +31,15 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index c75ba18bd..21f4c0995 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -31,14 +31,15 @@ } PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index 1d13df6c5..34ea924a5 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -31,15 +31,16 @@ IndentationSize = 4 } - PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckSeparator = $true - CheckParameter = $false + PSUseConsistentWhitespace = @{ + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false } PSAlignAssignmentStatement = @{ diff --git a/RuleDocumentation/UseConsistentWhitespace.md b/RuleDocumentation/UseConsistentWhitespace.md index f24267a85..0e7509d1e 100644 --- a/RuleDocumentation/UseConsistentWhitespace.md +++ b/RuleDocumentation/UseConsistentWhitespace.md @@ -13,14 +13,15 @@ ```powershell Rules = @{ PSUseConsistentWhitespace = @{ - Enable = $true - CheckInnerBrace = $true - CheckOpenBrace = $true - CheckOpenParen = $true - CheckOperator = $true - CheckPipe = $true - CheckSeparator = $true - CheckParameter = $false + Enable = $true + CheckInnerBrace = $true + CheckOpenBrace = $true + CheckOpenParen = $true + CheckOperator = $true + CheckPipe = $true + CheckPipeForRedundantWhitespace = $false + CheckSeparator = $true + CheckParameter = $false } } ``` @@ -53,7 +54,11 @@ Checks if a comma or a semicolon is followed by a space. E.g. `@(1, 2, 3)` or `@ #### CheckPipe: bool (Default value is `$true`) -Checks if a pipe is surrounded on both sides by a space. E.g. `foo | bar` instead of `foo|bar`. +Checks if a pipe is surrounded on both sides by a space but ignores redundant whitespace. E.g. `foo | bar` instead of `foo|bar`. + +#### CheckPipeForRedundantWhitespace : bool (Default value is `$false`) + +Checks if a pipe is surrounded by redundant whitespace (i.e. more than 1 whitespace). E.g. `foo | bar` instead of `foo | bar`. #### CheckParameter: bool (Default value is `$false` at the moment due to the setting being new) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 75b14fca3..f589d9e45 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -48,6 +48,9 @@ private List>> violationFind [ConfigurableRuleProperty(defaultValue: true)] public bool CheckPipe { get; protected set; } + [ConfigurableRuleProperty(defaultValue: false)] + public bool CheckPipeForRedundantWhitespace { get; protected set; } + [ConfigurableRuleProperty(defaultValue: true)] public bool CheckOpenParen { get; protected set; } @@ -73,7 +76,7 @@ public override void ConfigureRule(IDictionary paramValueMap) violationFinders.Add(FindInnerBraceViolations); } - if (CheckPipe) + if (CheckPipe || CheckPipeForRedundantWhitespace) { violationFinders.Add(FindPipeViolations); } @@ -314,16 +317,19 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp continue; } - if (!IsNextTokenApartByWhitespace(pipe)) + if (!IsNextTokenApartByWhitespace(pipe, out bool hasRedundantWhitespace)) { - yield return new DiagnosticRecord( - GetError(ErrorKind.AfterPipe), - pipe.Value.Extent, - GetName(), - GetDiagnosticSeverity(), - tokenOperations.Ast.Extent.File, - null, - GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, true, false).ToList()); + if (CheckPipeForRedundantWhitespace && hasRedundantWhitespace || CheckPipe && !hasRedundantWhitespace) + { + yield return new DiagnosticRecord( + GetError(ErrorKind.AfterPipe), + pipe.Value.Extent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, true, false).ToList()); + } } } @@ -339,9 +345,11 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp continue; } - if (!IsPreviousTokenApartByWhitespace(pipe)) + if (!IsPreviousTokenApartByWhitespace(pipe, out bool hasRedundantWhitespace)) { - yield return new DiagnosticRecord( + if (CheckPipeForRedundantWhitespace && hasRedundantWhitespace || CheckPipe && !hasRedundantWhitespace) + { + yield return new DiagnosticRecord( GetError(ErrorKind.BeforePipe), pipe.Value.Extent, GetName(), @@ -349,6 +357,7 @@ private IEnumerable FindPipeViolations(TokenOperations tokenOp tokenOperations.Ast.Extent.File, null, GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, false, true).ToList()); + } } } } @@ -467,16 +476,28 @@ private bool IsKeyword(Token token) return openParenKeywordWhitelist.Contains(token.Kind); } - private bool IsPreviousTokenApartByWhitespace(LinkedListNode tokenNode) + private static bool IsPreviousTokenApartByWhitespace(LinkedListNode tokenNode) + { + return IsPreviousTokenApartByWhitespace(tokenNode, out _); + } + + private static bool IsPreviousTokenApartByWhitespace(LinkedListNode tokenNode, out bool hasRedundantWhitespace) + { + var actualWhitespaceSize = tokenNode.Value.Extent.StartColumnNumber - tokenNode.Previous.Value.Extent.EndColumnNumber; + hasRedundantWhitespace = actualWhitespaceSize - whiteSpaceSize > 0; + return whiteSpaceSize == actualWhitespaceSize; + } + + private static bool IsNextTokenApartByWhitespace(LinkedListNode tokenNode) { - return whiteSpaceSize == - (tokenNode.Value.Extent.StartColumnNumber - tokenNode.Previous.Value.Extent.EndColumnNumber); + return IsNextTokenApartByWhitespace(tokenNode, out _); } - private bool IsNextTokenApartByWhitespace(LinkedListNode tokenNode) + private static bool IsNextTokenApartByWhitespace(LinkedListNode tokenNode, out bool hasRedundantWhitespace) { - return whiteSpaceSize == - (tokenNode.Next.Value.Extent.StartColumnNumber - tokenNode.Value.Extent.EndColumnNumber); + var actualWhitespaceSize = tokenNode.Next.Value.Extent.StartColumnNumber - tokenNode.Value.Extent.EndColumnNumber; + hasRedundantWhitespace = actualWhitespaceSize - whiteSpaceSize > 0; + return whiteSpaceSize == actualWhitespaceSize; } private bool IsPreviousTokenOnSameLineAndApartByWhitespace(LinkedListNode tokenNode) diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index fb7864d7a..0d1441384 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -237,6 +237,7 @@ $x = "abc"; $ruleConfiguration.CheckOpenParen = $false $ruleConfiguration.CheckOperator = $false $ruleConfiguration.CheckPipe = $true + $ruleConfiguration.CheckPipeForRedundantWhitespace = $false $ruleConfiguration.CheckSeparator = $false } @@ -246,22 +247,20 @@ $x = "abc"; Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } - It "Should find a violation if there is no space before pipe" { + It "Should not find a violation if there is no space before pipe" { $def = 'Get-Item| foo' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings Test-CorrectionExtentFromContent $def $violations 1 '' ' ' } - It "Should find a violation if there is one space too much before pipe" { + It "Should not find a violation if there is one space too much before pipe" { $def = 'Get-Item | foo' - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty } It "Should find a violation if there is one space too much after pipe" { $def = 'Get-Item | foo' - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings - Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty } It "Should not find a violation if there is 1 space before and after a pipe" { @@ -294,6 +293,51 @@ foo } } + Context "CheckPipeForRedundantWhitespace" { + BeforeAll { + $ruleConfiguration.CheckInnerBrace = $false + $ruleConfiguration.CheckOpenBrace = $false + $ruleConfiguration.CheckOpenParen = $false + $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $false + $ruleConfiguration.CheckPipeForRedundantWhitespace = $true + $ruleConfiguration.CheckSeparator = $false + } + + It "Should not find a violation if there is no space around pipe" { + $def = 'foo|bar' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It "Should not find a violation if there is exactly one space around pipe" { + $def = 'foo | bar' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty + } + + It "Should find a violation if there is one space too much before pipe" { + $def = 'foo | bar' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } + + It "Should find a violation if there is two spaces too much before pipe" { + $def = 'foo | bar' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } + + It "Should find a violation if there is one space too much after pipe" { + $def = 'foo | bar' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } + + It "Should find a violation if there is two spaces too much after pipe" { + $def = 'foo | bar' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' + } + } Context "CheckInnerBrace" { BeforeAll {