Skip to content

Commit 163d2c2

Browse files
bergmeisterbergmeister
authored
Split CheckPipe feature of trimming redundant whitespace out into option CheckPipeForRedundantWhiteSpace (#1413)
* Split CheckPipe feature of trimming redundant whitespace out into option CheckPipeForRedundantWhiteSpace * docs and naming tweaks * casing * remove redundant whitespace * Update RuleDocumentation/UseConsistentWhitespace.md * remove redundant newline
1 parent 3f5e772 commit 163d2c2

7 files changed

+140
-66
lines changed

‎Engine/Settings/CodeFormatting.psd1

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@
3131
}
3232

3333
PSUseConsistentWhitespace = @{
34-
Enable = $true
35-
CheckInnerBrace = $true
36-
CheckOpenBrace = $true
37-
CheckOpenParen = $true
38-
CheckOperator = $true
39-
CheckPipe = $true
40-
CheckSeparator = $true
41-
CheckParameter = $false
34+
Enable = $true
35+
CheckInnerBrace = $true
36+
CheckOpenBrace = $true
37+
CheckOpenParen = $true
38+
CheckOperator = $true
39+
CheckPipe = $true
40+
CheckPipeForRedundantWhitespace = $false
41+
CheckSeparator = $true
42+
CheckParameter = $false
4243
}
4344

4445
PSAlignAssignmentStatement = @{

‎Engine/Settings/CodeFormattingAllman.psd1

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@
3131
}
3232

3333
PSUseConsistentWhitespace = @{
34-
Enable = $true
35-
CheckInnerBrace = $true
36-
CheckOpenBrace = $true
37-
CheckOpenParen = $true
38-
CheckOperator = $true
39-
CheckPipe = $true
40-
CheckSeparator = $true
41-
CheckParameter = $false
34+
Enable = $true
35+
CheckInnerBrace = $true
36+
CheckOpenBrace = $true
37+
CheckOpenParen = $true
38+
CheckOperator = $true
39+
CheckPipe = $true
40+
CheckPipeForRedundantWhitespace = $false
41+
CheckSeparator = $true
42+
CheckParameter = $false
4243
}
4344

4445
PSAlignAssignmentStatement = @{

‎Engine/Settings/CodeFormattingOTBS.psd1

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@
3131
}
3232

3333
PSUseConsistentWhitespace = @{
34-
Enable = $true
35-
CheckInnerBrace = $true
36-
CheckOpenBrace = $true
37-
CheckOpenParen = $true
38-
CheckOperator = $true
39-
CheckPipe = $true
40-
CheckSeparator = $true
41-
CheckParameter = $false
34+
Enable = $true
35+
CheckInnerBrace = $true
36+
CheckOpenBrace = $true
37+
CheckOpenParen = $true
38+
CheckOperator = $true
39+
CheckPipe = $true
40+
CheckPipeForRedundantWhitespace = $false
41+
CheckSeparator = $true
42+
CheckParameter = $false
4243
}
4344

4445
PSAlignAssignmentStatement = @{

‎Engine/Settings/CodeFormattingStroustrup.psd1

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,16 @@
3131
IndentationSize = 4
3232
}
3333

34-
PSUseConsistentWhitespace = @{
35-
Enable = $true
36-
CheckInnerBrace = $true
37-
CheckOpenBrace = $true
38-
CheckOpenParen = $true
39-
CheckOperator = $true
40-
CheckPipe = $true
41-
CheckSeparator = $true
42-
CheckParameter = $false
34+
PSUseConsistentWhitespace = @{
35+
Enable = $true
36+
CheckInnerBrace = $true
37+
CheckOpenBrace = $true
38+
CheckOpenParen = $true
39+
CheckOperator = $true
40+
CheckPipe = $true
41+
CheckPipeForRedundantWhitespace = $false
42+
CheckSeparator = $true
43+
CheckParameter = $false
4344
}
4445

4546
PSAlignAssignmentStatement = @{

‎RuleDocumentation/UseConsistentWhitespace.md

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,15 @@
1313
```powershell
1414
Rules = @{
1515
PSUseConsistentWhitespace = @{
16-
Enable = $true
17-
CheckInnerBrace = $true
18-
CheckOpenBrace = $true
19-
CheckOpenParen = $true
20-
CheckOperator = $true
21-
CheckPipe = $true
22-
CheckSeparator = $true
23-
CheckParameter = $false
16+
Enable = $true
17+
CheckInnerBrace = $true
18+
CheckOpenBrace = $true
19+
CheckOpenParen = $true
20+
CheckOperator = $true
21+
CheckPipe = $true
22+
CheckPipeForRedundantWhitespace = $false
23+
CheckSeparator = $true
24+
CheckParameter = $false
2425
}
2526
}
2627
```
@@ -53,7 +54,11 @@ Checks if a comma or a semicolon is followed by a space. E.g. `@(1, 2, 3)` or `@
5354

5455
#### CheckPipe: bool (Default value is `$true`)
5556

56-
Checks if a pipe is surrounded on both sides by a space. E.g. `foo | bar` instead of `foo|bar`.
57+
Checks if a pipe is surrounded on both sides by a space but ignores redundant whitespace. E.g. `foo | bar` instead of `foo|bar`.
58+
59+
#### CheckPipeForRedundantWhitespace : bool (Default value is `$false`)
60+
61+
Checks if a pipe is surrounded by redundant whitespace (i.e. more than 1 whitespace). E.g. `foo | bar` instead of `foo | bar`.
5762

5863
#### CheckParameter: bool (Default value is `$false` at the moment due to the setting being new)
5964

‎Rules/UseConsistentWhitespace.cs

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ private List<Func<TokenOperations, IEnumerable<DiagnosticRecord>>> violationFind
4848
[ConfigurableRuleProperty(defaultValue: true)]
4949
public bool CheckPipe { get; protected set; }
5050

51+
[ConfigurableRuleProperty(defaultValue: false)]
52+
public bool CheckPipeForRedundantWhitespace { get; protected set; }
53+
5154
[ConfigurableRuleProperty(defaultValue: true)]
5255
public bool CheckOpenParen { get; protected set; }
5356

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

76-
if (CheckPipe)
79+
if (CheckPipe || CheckPipeForRedundantWhitespace)
7780
{
7881
violationFinders.Add(FindPipeViolations);
7982
}
@@ -314,16 +317,19 @@ private IEnumerable<DiagnosticRecord> FindPipeViolations(TokenOperations tokenOp
314317
continue;
315318
}
316319

317-
if (!IsNextTokenApartByWhitespace(pipe))
320+
if (!IsNextTokenApartByWhitespace(pipe, out bool hasRedundantWhitespace))
318321
{
319-
yield return new DiagnosticRecord(
320-
GetError(ErrorKind.AfterPipe),
321-
pipe.Value.Extent,
322-
GetName(),
323-
GetDiagnosticSeverity(),
324-
tokenOperations.Ast.Extent.File,
325-
null,
326-
GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, true, false).ToList());
322+
if (CheckPipeForRedundantWhitespace && hasRedundantWhitespace || CheckPipe && !hasRedundantWhitespace)
323+
{
324+
yield return new DiagnosticRecord(
325+
GetError(ErrorKind.AfterPipe),
326+
pipe.Value.Extent,
327+
GetName(),
328+
GetDiagnosticSeverity(),
329+
tokenOperations.Ast.Extent.File,
330+
null,
331+
GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, true, false).ToList());
332+
}
327333
}
328334
}
329335

@@ -339,16 +345,19 @@ private IEnumerable<DiagnosticRecord> FindPipeViolations(TokenOperations tokenOp
339345
continue;
340346
}
341347

342-
if (!IsPreviousTokenApartByWhitespace(pipe))
348+
if (!IsPreviousTokenApartByWhitespace(pipe, out bool hasRedundantWhitespace))
343349
{
344-
yield return new DiagnosticRecord(
350+
if (CheckPipeForRedundantWhitespace && hasRedundantWhitespace || CheckPipe && !hasRedundantWhitespace)
351+
{
352+
yield return new DiagnosticRecord(
345353
GetError(ErrorKind.BeforePipe),
346354
pipe.Value.Extent,
347355
GetName(),
348356
GetDiagnosticSeverity(),
349357
tokenOperations.Ast.Extent.File,
350358
null,
351359
GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, false, true).ToList());
360+
}
352361
}
353362
}
354363
}
@@ -467,16 +476,28 @@ private bool IsKeyword(Token token)
467476
return openParenKeywordWhitelist.Contains(token.Kind);
468477
}
469478

470-
private bool IsPreviousTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
479+
private static bool IsPreviousTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
480+
{
481+
return IsPreviousTokenApartByWhitespace(tokenNode, out _);
482+
}
483+
484+
private static bool IsPreviousTokenApartByWhitespace(LinkedListNode<Token> tokenNode, out bool hasRedundantWhitespace)
485+
{
486+
var actualWhitespaceSize = tokenNode.Value.Extent.StartColumnNumber - tokenNode.Previous.Value.Extent.EndColumnNumber;
487+
hasRedundantWhitespace = actualWhitespaceSize - whiteSpaceSize > 0;
488+
return whiteSpaceSize == actualWhitespaceSize;
489+
}
490+
491+
private static bool IsNextTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
471492
{
472-
return whiteSpaceSize ==
473-
(tokenNode.Value.Extent.StartColumnNumber - tokenNode.Previous.Value.Extent.EndColumnNumber);
493+
return IsNextTokenApartByWhitespace(tokenNode, out _);
474494
}
475495

476-
private bool IsNextTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
496+
private static bool IsNextTokenApartByWhitespace(LinkedListNode<Token> tokenNode, out bool hasRedundantWhitespace)
477497
{
478-
return whiteSpaceSize ==
479-
(tokenNode.Next.Value.Extent.StartColumnNumber - tokenNode.Value.Extent.EndColumnNumber);
498+
var actualWhitespaceSize = tokenNode.Next.Value.Extent.StartColumnNumber - tokenNode.Value.Extent.EndColumnNumber;
499+
hasRedundantWhitespace = actualWhitespaceSize - whiteSpaceSize > 0;
500+
return whiteSpaceSize == actualWhitespaceSize;
480501
}
481502

482503
private bool IsPreviousTokenOnSameLineAndApartByWhitespace(LinkedListNode<Token> tokenNode)

‎Tests/Rules/UseConsistentWhitespace.tests.ps1

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ $x = "abc";
237237
$ruleConfiguration.CheckOpenParen = $false
238238
$ruleConfiguration.CheckOperator = $false
239239
$ruleConfiguration.CheckPipe = $true
240+
$ruleConfiguration.CheckPipeForRedundantWhitespace = $false
240241
$ruleConfiguration.CheckSeparator = $false
241242
}
242243

@@ -246,22 +247,20 @@ $x = "abc";
246247
Test-CorrectionExtentFromContent $def $violations 1 '' ' '
247248
}
248249

249-
It "Should find a violation if there is no space before pipe" {
250+
It "Should not find a violation if there is no space before pipe" {
250251
$def = 'Get-Item| foo'
251252
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
252253
Test-CorrectionExtentFromContent $def $violations 1 '' ' '
253254
}
254255

255-
It "Should find a violation if there is one space too much before pipe" {
256+
It "Should not find a violation if there is one space too much before pipe" {
256257
$def = 'Get-Item | foo'
257-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
258-
Test-CorrectionExtentFromContent $def $violations 1 ' ' ' '
258+
Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty
259259
}
260260

261261
It "Should find a violation if there is one space too much after pipe" {
262262
$def = 'Get-Item | foo'
263-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
264-
Test-CorrectionExtentFromContent $def $violations 1 ' ' ' '
263+
Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty
265264
}
266265

267266
It "Should not find a violation if there is 1 space before and after a pipe" {
@@ -294,6 +293,51 @@ foo
294293
}
295294
}
296295

296+
Context "CheckPipeForRedundantWhitespace" {
297+
BeforeAll {
298+
$ruleConfiguration.CheckInnerBrace = $false
299+
$ruleConfiguration.CheckOpenBrace = $false
300+
$ruleConfiguration.CheckOpenParen = $false
301+
$ruleConfiguration.CheckOperator = $false
302+
$ruleConfiguration.CheckPipe = $false
303+
$ruleConfiguration.CheckPipeForRedundantWhitespace = $true
304+
$ruleConfiguration.CheckSeparator = $false
305+
}
306+
307+
It "Should not find a violation if there is no space around pipe" {
308+
$def = 'foo|bar'
309+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty
310+
}
311+
312+
It "Should not find a violation if there is exactly one space around pipe" {
313+
$def = 'foo | bar'
314+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -BeNullOrEmpty
315+
}
316+
317+
It "Should find a violation if there is one space too much before pipe" {
318+
$def = 'foo | bar'
319+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
320+
Test-CorrectionExtentFromContent $def $violations 1 ' ' ' '
321+
}
322+
323+
It "Should find a violation if there is two spaces too much before pipe" {
324+
$def = 'foo | bar'
325+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
326+
Test-CorrectionExtentFromContent $def $violations 1 ' ' ' '
327+
}
328+
329+
It "Should find a violation if there is one space too much after pipe" {
330+
$def = 'foo | bar'
331+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
332+
Test-CorrectionExtentFromContent $def $violations 1 ' ' ' '
333+
}
334+
335+
It "Should find a violation if there is two spaces too much after pipe" {
336+
$def = 'foo | bar'
337+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
338+
Test-CorrectionExtentFromContent $def $violations 1 ' ' ' '
339+
}
340+
}
297341

298342
Context "CheckInnerBrace" {
299343
BeforeAll {

0 commit comments

Comments
 (0)