Skip to content

Commit 484f405

Browse files
bergmeisterbergmeister
authored
Add CheckInnerBrace and CheckPipe options to PSUseConsistentWhitespace (#1092)
* Check for 1 whitespace AFTER curly brace in new InnerCurly option * check for one space in closing of inner brace * add check for pipes. TODO: messages * update settings files and add first set of tests for curly braces. TODO: cater for newlines * fix new line handling for innerbrace * fix suggested corrections and add another test case for inner brace * order settings alphabetically and add tests for checkpipe * fix test that returned 2 warnings now due to checkinnerbrace * fix innerPipe and write documentation * tweak backtick scenarios * fix 1 failing test * customise warning messages * swap messages to be correct * add more test cases and fix small bug whereby the missing space before the closing brace would not be correctly replaced * enforce whitespace also if there is more than 1 curly brace. TODO: add test case for that * add test cases for nested parenthesis and add validation to test helper method to inform about limited functionality * add test case for more than 1 space inside curly braces and tidy up tests
1 parent d1c8e57 commit 484f405

11 files changed

+424
-95
lines changed

‎Engine/Settings/CodeFormatting.psd1‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@
2929
}
3030

3131
PSUseConsistentWhitespace = @{
32-
Enable = $true
33-
CheckOpenBrace = $true
34-
CheckOpenParen = $true
35-
CheckOperator = $true
36-
CheckSeparator = $true
32+
Enable = $true
33+
CheckInnerBrace = $true
34+
CheckOpenBrace = $true
35+
CheckOpenParen = $true
36+
CheckOperator = $true
37+
CheckPipe = $true
38+
CheckSeparator = $true
3739
}
3840

3941
PSAlignAssignmentStatement = @{

‎Engine/Settings/CodeFormattingAllman.psd1‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@
2929
}
3030

3131
PSUseConsistentWhitespace = @{
32-
Enable = $true
33-
CheckOpenBrace = $true
34-
CheckOpenParen = $true
35-
CheckOperator = $true
36-
CheckSeparator = $true
32+
Enable = $true
33+
CheckInnerBrace = $true
34+
CheckOpenBrace = $true
35+
CheckOpenParen = $true
36+
CheckOperator = $true
37+
CheckPipe = $true
38+
CheckSeparator = $true
3739
}
3840

3941
PSAlignAssignmentStatement = @{

‎Engine/Settings/CodeFormattingOTBS.psd1‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@
3030

3131
PSUseConsistentWhitespace = @{
3232
Enable = $true
33+
CheckInnerBrace = $true
3334
CheckOpenBrace = $true
3435
CheckOpenParen = $true
3536
CheckOperator = $true
37+
CheckPipe = $true
3638
CheckSeparator = $true
3739
}
3840

‎Engine/Settings/CodeFormattingStroustrup.psd1‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@
3030
}
3131

3232
PSUseConsistentWhitespace = @{
33-
Enable = $true
34-
CheckOpenBrace = $true
35-
CheckOpenParen = $true
36-
CheckOperator = $true
37-
CheckSeparator = $true
33+
Enable = $true
34+
CheckInnerBrace = $true
35+
CheckOpenBrace = $true
36+
CheckOpenParen = $true
37+
CheckOperator = $true
38+
CheckPipe = $true
39+
CheckSeparator = $true
3840
}
3941

4042
PSAlignAssignmentStatement = @{

‎RuleDocumentation/UseConsistentWhitespace.md‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,26 @@
2828

2929
Enable or disable the rule during ScriptAnalyzer invocation.
3030

31+
#### CheckInnerBrace: bool (Default value is `$true`)
32+
33+
Checks if there is a space after the opening brace and a space before the closing brace. E.g. `if ($true) { foo }` instead of `if ($true) {bar}`.
34+
3135
#### CheckOpenBrace: bool (Default value is `$true`)
3236

33-
Checks if there is a space between a keyword and its corresponding open brace. E.g. `foo { }`.
37+
Checks if there is a space between a keyword and its corresponding open brace. E.g. `foo { }` instead of `foo{ }`.
3438

3539
#### CheckOpenParen: bool (Default value is `$true`)
3640

37-
Checks if there is space between a keyword and its corresponding open parenthesis. E.g. `if (true)`.
41+
Checks if there is space between a keyword and its corresponding open parenthesis. E.g. `if (true)` instead of `if(true)`.
3842

3943
#### CheckOperator: bool (Default value is `$true`)
4044

41-
Checks if a binary or unary operator is surrounded on both sides by a space. E.g. `$x = 1`.
45+
Checks if a binary or unary operator is surrounded on both sides by a space. E.g. `$x = 1` instead of `$x=1`.
4246

4347
#### CheckSeparator: bool (Default value is `$true`)
4448

45-
Checks if a comma or a semicolon is followed by a space. E.g. `@(1, 2, 3)` or `@{a = 1; b = 2}`.
49+
Checks if a comma or a semicolon is followed by a space. E.g. `@(1, 2, 3)` or `@{a = 1; b = 2}` instead of `@(1,2,3)` or `@{a = 1;b = 2}`.
50+
51+
#### CheckPipe: bool (Default value is `$true`)
52+
53+
Checks if a pipe is surrounded on both sides by a space. E.g. `foo | bar` instead of `foo|bar`.

‎Rules/Strings.Designer.cs‎

Lines changed: 38 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rules/Strings.resx‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,7 @@
957957
<data name="UseConsistentWhitespaceDescription" xml:space="preserve">
958958
<value>Check for whitespace between keyword and open paren/curly, around assigment operator ('='), around arithmetic operators and after separators (',' and ';')</value>
959959
</data>
960-
<data name="UseConsistentWhitespaceErrorBeforeBrace" xml:space="preserve">
960+
<data name="UseConsistentWhitespaceErrorBeforeOpeningBrace" xml:space="preserve">
961961
<value>Use space before open brace.</value>
962962
</data>
963963
<data name="UseConsistentWhitespaceErrorBeforeParen" xml:space="preserve">
@@ -1044,4 +1044,16 @@
10441044
<data name="PossibleIncorrectComparisonWithNullSuggesteCorrectionDescription" xml:space="preserve">
10451045
<value>Use $null on the left hand side for safe comparison with $null.</value>
10461046
</data>
1047+
<data name="UseConsistentWhitespaceErrorAfterOpeningBrace" xml:space="preserve">
1048+
<value>Use space after open brace.</value>
1049+
</data>
1050+
<data name="UseConsistentWhitespaceErrorBeforeClosingInnerBrace" xml:space="preserve">
1051+
<value>Use space before closing brace.</value>
1052+
</data>
1053+
<data name="UseConsistentWhitespaceErrorSpaceAfterPipe" xml:space="preserve">
1054+
<value>Use space after pipe.</value>
1055+
</data>
1056+
<data name="UseConsistentWhitespaceErrorSpaceBeforePipe" xml:space="preserve">
1057+
<value>Use space before pipe.</value>
1058+
</data>
10471059
</root>

‎Rules/UseConsistentWhitespace.cs‎

Lines changed: 142 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
2222
#endif
2323
public class UseConsistentWhitespace : ConfigurableRule
2424
{
25-
private enum ErrorKind { Brace, Paren, Operator, SeparatorComma, SeparatorSemi };
25+
private enum ErrorKind { BeforeOpeningBrace, Paren, Operator, SeparatorComma, SeparatorSemi, AfterOpeningBrace, BeforeClosingBrace, BeforePipe, AfterPipe };
2626
private const int whiteSpaceSize = 1;
2727
private const string whiteSpace = " ";
2828
private readonly SortedSet<TokenKind> openParenKeywordWhitelist = new SortedSet<TokenKind>()
@@ -41,6 +41,12 @@ private List<Func<TokenOperations, IEnumerable<DiagnosticRecord>>> violationFind
4141
[ConfigurableRuleProperty(defaultValue: true)]
4242
public bool CheckOpenBrace { get; protected set; }
4343

44+
[ConfigurableRuleProperty(defaultValue: true)]
45+
public bool CheckInnerBrace { get; protected set; }
46+
47+
[ConfigurableRuleProperty(defaultValue: true)]
48+
public bool CheckPipe { get; protected set; }
49+
4450
[ConfigurableRuleProperty(defaultValue: true)]
4551
public bool CheckOpenParen { get; protected set; }
4652

@@ -58,6 +64,16 @@ public override void ConfigureRule(IDictionary<string, object> paramValueMap)
5864
violationFinders.Add(FindOpenBraceViolations);
5965
}
6066

67+
if (CheckInnerBrace)
68+
{
69+
violationFinders.Add(FindInnerBraceViolations);
70+
}
71+
72+
if (CheckPipe)
73+
{
74+
violationFinders.Add(FindPipeViolations);
75+
}
76+
6177
if (CheckOpenParen)
6278
{
6379
violationFinders.Add(FindOpenParenViolations);
@@ -171,10 +187,18 @@ private string GetError(ErrorKind kind)
171187
{
172188
switch (kind)
173189
{
174-
case ErrorKind.Brace:
175-
return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeBrace);
190+
case ErrorKind.BeforeOpeningBrace:
191+
return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeOpeningBrace);
192+
case ErrorKind.AfterOpeningBrace:
193+
return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorAfterOpeningBrace);
194+
case ErrorKind.BeforeClosingBrace:
195+
return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeClosingInnerBrace);
176196
case ErrorKind.Operator:
177197
return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorOperator);
198+
case ErrorKind.BeforePipe:
199+
return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSpaceBeforePipe);
200+
case ErrorKind.AfterPipe:
201+
return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSpaceAfterPipe);
178202
case ErrorKind.SeparatorComma:
179203
return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSeparatorComma);
180204
case ErrorKind.SeparatorSemi:
@@ -200,7 +224,7 @@ private IEnumerable<DiagnosticRecord> FindOpenBraceViolations(TokenOperations to
200224
if (!IsPreviousTokenApartByWhitespace(lcurly))
201225
{
202226
yield return new DiagnosticRecord(
203-
GetError(ErrorKind.Brace),
227+
GetError(ErrorKind.BeforeOpeningBrace),
204228
lcurly.Value.Extent,
205229
GetName(),
206230
GetDiagnosticSeverity(),
@@ -211,6 +235,111 @@ private IEnumerable<DiagnosticRecord> FindOpenBraceViolations(TokenOperations to
211235
}
212236
}
213237

238+
private IEnumerable<DiagnosticRecord> FindInnerBraceViolations(TokenOperations tokenOperations)
239+
{
240+
foreach (var lCurly in tokenOperations.GetTokenNodes(TokenKind.LCurly))
241+
{
242+
if (lCurly.Next == null
243+
|| !IsPreviousTokenOnSameLine(lCurly)
244+
|| lCurly.Next.Value.Kind == TokenKind.NewLine
245+
|| lCurly.Next.Value.Kind == TokenKind.LineContinuation
246+
)
247+
{
248+
continue;
249+
}
250+
251+
if (!IsNextTokenApartByWhitespace(lCurly))
252+
{
253+
yield return new DiagnosticRecord(
254+
GetError(ErrorKind.AfterOpeningBrace),
255+
lCurly.Value.Extent,
256+
GetName(),
257+
GetDiagnosticSeverity(),
258+
tokenOperations.Ast.Extent.File,
259+
null,
260+
GetCorrections(lCurly.Previous.Value, lCurly.Value, lCurly.Next.Value, true, false).ToList());
261+
}
262+
}
263+
264+
foreach (var rCurly in tokenOperations.GetTokenNodes(TokenKind.RCurly))
265+
{
266+
if (rCurly.Previous == null
267+
|| !IsPreviousTokenOnSameLine(rCurly)
268+
|| rCurly.Previous.Value.Kind == TokenKind.LCurly
269+
|| rCurly.Previous.Value.Kind == TokenKind.NewLine
270+
|| rCurly.Previous.Value.Kind == TokenKind.LineContinuation
271+
)
272+
{
273+
continue;
274+
}
275+
276+
if (!IsPreviousTokenApartByWhitespace(rCurly))
277+
{
278+
yield return new DiagnosticRecord(
279+
GetError(ErrorKind.BeforeClosingBrace),
280+
rCurly.Value.Extent,
281+
GetName(),
282+
GetDiagnosticSeverity(),
283+
tokenOperations.Ast.Extent.File,
284+
null,
285+
GetCorrections(rCurly.Previous.Value, rCurly.Value, rCurly.Next.Value, false, true).ToList());
286+
}
287+
}
288+
}
289+
290+
private IEnumerable<DiagnosticRecord> FindPipeViolations(TokenOperations tokenOperations)
291+
{
292+
foreach (var pipe in tokenOperations.GetTokenNodes(TokenKind.Pipe))
293+
{
294+
if (pipe.Next == null
295+
|| !IsPreviousTokenOnSameLine(pipe)
296+
|| pipe.Next.Value.Kind == TokenKind.Pipe
297+
|| pipe.Next.Value.Kind == TokenKind.NewLine
298+
|| pipe.Next.Value.Kind == TokenKind.LineContinuation
299+
)
300+
{
301+
continue;
302+
}
303+
304+
if (!IsNextTokenApartByWhitespace(pipe))
305+
{
306+
yield return new DiagnosticRecord(
307+
GetError(ErrorKind.AfterPipe),
308+
pipe.Value.Extent,
309+
GetName(),
310+
GetDiagnosticSeverity(),
311+
tokenOperations.Ast.Extent.File,
312+
null,
313+
GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, true, false).ToList());
314+
}
315+
}
316+
317+
foreach (var pipe in tokenOperations.GetTokenNodes(TokenKind.Pipe))
318+
{
319+
if (pipe.Previous == null
320+
|| !IsPreviousTokenOnSameLine(pipe)
321+
|| pipe.Previous.Value.Kind == TokenKind.Pipe
322+
|| pipe.Previous.Value.Kind == TokenKind.NewLine
323+
|| pipe.Previous.Value.Kind == TokenKind.LineContinuation
324+
)
325+
{
326+
continue;
327+
}
328+
329+
if (!IsPreviousTokenApartByWhitespace(pipe))
330+
{
331+
yield return new DiagnosticRecord(
332+
GetError(ErrorKind.BeforePipe),
333+
pipe.Value.Extent,
334+
GetName(),
335+
GetDiagnosticSeverity(),
336+
tokenOperations.Ast.Extent.File,
337+
null,
338+
GetCorrections(pipe.Previous.Value, pipe.Value, pipe.Next.Value, false, true).ToList());
339+
}
340+
}
341+
}
342+
214343
private IEnumerable<DiagnosticRecord> FindOpenParenViolations(TokenOperations tokenOperations)
215344
{
216345
foreach (var lparen in tokenOperations.GetTokenNodes(TokenKind.LParen))
@@ -291,6 +420,12 @@ private bool IsPreviousTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
291420
(tokenNode.Value.Extent.StartColumnNumber - tokenNode.Previous.Value.Extent.EndColumnNumber);
292421
}
293422

423+
private bool IsNextTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
424+
{
425+
return whiteSpaceSize ==
426+
(tokenNode.Next.Value.Extent.StartColumnNumber - tokenNode.Value.Extent.EndColumnNumber);
427+
}
428+
294429
private bool IsPreviousTokenOnSameLineAndApartByWhitespace(LinkedListNode<Token> tokenNode)
295430
{
296431
return IsPreviousTokenOnSameLine(tokenNode) && IsPreviousTokenApartByWhitespace(tokenNode);
@@ -342,8 +477,9 @@ private List<CorrectionExtent> GetCorrections(
342477
Token prevToken,
343478
Token token,
344479
Token nextToken,
345-
bool hasWhitespaceBefore,
346-
bool hasWhitespaceAfter)
480+
bool hasWhitespaceBefore, // if this is false, then the returned correction extent will add a whitespace before the token
481+
bool hasWhitespaceAfter // if this is false, then the returned correction extent will add a whitespace after the token
482+
)
347483
{
348484
var sb = new StringBuilder();
349485
IScriptExtent e1 = token.Extent;

0 commit comments

Comments
 (0)