Skip to content

Split CheckPipe feature of trimming redundant whitespace out into option CheckPipeForRedundantWhitespace #1413

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Feb 11, 2020
Merged

Split CheckPipe feature of trimming redundant whitespace out into option CheckPipeForRedundantWhitespace #1413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
993e119
Split CheckPipe feature of trimming redundant whitespace out into opt…
Feb 10, 2020
ffbec6c
docs and naming tweaks
Feb 10, 2020
dec5517
casing
Feb 10, 2020
cd31f2a
remove redundant whitespace
Feb 11, 2020
842475f
Update RuleDocumentation/UseConsistentWhitespace.md
bergmeister Feb 11, 2020
c2f3357
remove redundant newline
Feb 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions Engine/Settings/CodeFormatting.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = @{
Expand Down
17 changes: 9 additions & 8 deletions Engine/Settings/CodeFormattingAllman.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = @{
Expand Down
17 changes: 9 additions & 8 deletions Engine/Settings/CodeFormattingOTBS.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = @{
Expand Down
19 changes: 10 additions & 9 deletions Engine/Settings/CodeFormattingStroustrup.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = @{
Expand Down
23 changes: 14 additions & 9 deletions RuleDocumentation/UseConsistentWhitespace.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
```
Expand Down Expand Up @@ -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)

Expand Down
57 changes: 39 additions & 18 deletions Rules/UseConsistentWhitespace.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ private List<Func<TokenOperations, IEnumerable<DiagnosticRecord>>> 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; }

Expand All @@ -73,7 +76,7 @@ public override void ConfigureRule(IDictionary<string, object> paramValueMap)
violationFinders.Add(FindInnerBraceViolations);
}

if (CheckPipe)
if (CheckPipe || CheckPipeForRedundantWhitespace)
{
violationFinders.Add(FindPipeViolations);
}
Expand Down Expand Up @@ -314,16 +317,19 @@ private IEnumerable<DiagnosticRecord> 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());
}
}
}

Expand All @@ -339,16 +345,19 @@ private IEnumerable<DiagnosticRecord> 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(),
GetDiagnosticSeverity(),
tokenOperations.Ast.Extent.File,
null,
GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, false, true).ToList());
}
}
}
}
Expand Down Expand Up @@ -467,16 +476,28 @@ private bool IsKeyword(Token token)
return openParenKeywordWhitelist.Contains(token.Kind);
}

private bool IsPreviousTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
private static bool IsPreviousTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
{
return IsPreviousTokenApartByWhitespace(tokenNode, out _);
}

private static bool IsPreviousTokenApartByWhitespace(LinkedListNode<Token> 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<Token> tokenNode)
{
return whiteSpaceSize ==
(tokenNode.Value.Extent.StartColumnNumber - tokenNode.Previous.Value.Extent.EndColumnNumber);
return IsNextTokenApartByWhitespace(tokenNode, out _);
}

private bool IsNextTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
private static bool IsNextTokenApartByWhitespace(LinkedListNode<Token> 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<Token> tokenNode)
Expand Down
56 changes: 50 additions & 6 deletions Tests/Rules/UseConsistentWhitespace.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ $x = "abc";
$ruleConfiguration.CheckOpenParen = $false
$ruleConfiguration.CheckOperator = $false
$ruleConfiguration.CheckPipe = $true
$ruleConfiguration.CheckPipeForRedundantWhitespace = $false
$ruleConfiguration.CheckSeparator = $false
}

Expand All @@ -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" {
Expand Down Expand Up @@ -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 {
Expand Down