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

Author: bergmeister

Engine/Settings/CodeFormatting.psd1

@@ -29,11 +29,13 @@
         }
 
         PSUseConsistentWhitespace  = @{
-            Enable         = $true
-            CheckOpenBrace = $true
-            CheckOpenParen = $true
-            CheckOperator  = $true
-            CheckSeparator = $true
+            Enable          = $true
+            CheckInnerBrace = $true
+            CheckOpenBrace  = $true
+            CheckOpenParen  = $true
+            CheckOperator   = $true
+            CheckPipe       = $true
+            CheckSeparator  = $true
         }
 
         PSAlignAssignmentStatement = @{

Engine/Settings/CodeFormattingAllman.psd1

@@ -29,11 +29,13 @@
         }
 
         PSUseConsistentWhitespace  = @{
-            Enable         = $true
-            CheckOpenBrace = $true
-            CheckOpenParen = $true
-            CheckOperator  = $true
-            CheckSeparator = $true
+            Enable          = $true
+            CheckInnerBrace = $true
+            CheckOpenBrace  = $true
+            CheckOpenParen  = $true
+            CheckOperator   = $true
+            CheckPipe       = $true
+            CheckSeparator  = $true
         }
 
         PSAlignAssignmentStatement = @{

Engine/Settings/CodeFormattingOTBS.psd1

@@ -30,9 +30,11 @@
 
         PSUseConsistentWhitespace  = @{
             Enable         = $true
+            CheckInnerBrace = $true
             CheckOpenBrace = $true
             CheckOpenParen = $true
             CheckOperator  = $true
+            CheckPipe       = $true
             CheckSeparator = $true
         }
 

Engine/Settings/CodeFormattingStroustrup.psd1

@@ -30,11 +30,13 @@
         }
 
         PSUseConsistentWhitespace = @{
-            Enable         = $true
-            CheckOpenBrace = $true
-            CheckOpenParen = $true
-            CheckOperator  = $true
-            CheckSeparator = $true
+            Enable          = $true
+            CheckInnerBrace = $true
+            CheckOpenBrace  = $true
+            CheckOpenParen  = $true
+            CheckOperator   = $true
+            CheckPipe       = $true
+            CheckSeparator  = $true
         }
 
         PSAlignAssignmentStatement = @{

RuleDocumentation/UseConsistentWhitespace.md

@@ -28,18 +28,26 @@
 
 Enable or disable the rule during ScriptAnalyzer invocation.
 
+#### CheckInnerBrace: bool (Default value is `$true`)
+
+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}`.
+
 #### CheckOpenBrace: bool (Default value is `$true`)
 
-Checks if there is a space between a keyword and its corresponding open brace. E.g. `foo { }`.
+Checks if there is a space between a keyword and its corresponding open brace. E.g. `foo { }` instead of `foo{ }`.
 
 #### CheckOpenParen: bool (Default value is `$true`)
 
-Checks if there is space between a keyword and its corresponding open parenthesis. E.g. `if (true)`.
+Checks if there is space between a keyword and its corresponding open parenthesis. E.g. `if (true)` instead of `if(true)`.
 
 #### CheckOperator: bool (Default value is `$true`)
 
-Checks if a binary or unary operator is surrounded on both sides by a space. E.g. `$x = 1`.
+Checks if a binary or unary operator is surrounded on both sides by a space. E.g. `$x = 1` instead of `$x=1`.
 
 #### CheckSeparator: bool (Default value is `$true`)
 
-Checks if a comma or a semicolon is followed by a space. E.g. `@(1, 2, 3)` or `@{a = 1; b = 2}`.
+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}`.
+
+#### 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`.

Rules/Strings.Designer.cs

@@ -957,7 +957,7 @@
   <data name="UseConsistentWhitespaceDescription" xml:space="preserve">
     <value>Check for whitespace between keyword and open paren/curly, around assigment operator ('='), around arithmetic operators and after separators (',' and ';')</value>
   </data>
-  <data name="UseConsistentWhitespaceErrorBeforeBrace" xml:space="preserve">
+  <data name="UseConsistentWhitespaceErrorBeforeOpeningBrace" xml:space="preserve">
     <value>Use space before open brace.</value>
   </data>
   <data name="UseConsistentWhitespaceErrorBeforeParen" xml:space="preserve">
@@ -1044,4 +1044,16 @@
   <data name="PossibleIncorrectComparisonWithNullSuggesteCorrectionDescription" xml:space="preserve">
     <value>Use $null on the left hand side for safe comparison with $null.</value>
   </data>
+  <data name="UseConsistentWhitespaceErrorAfterOpeningBrace" xml:space="preserve">
+    <value>Use space after open brace.</value>
+  </data>
+  <data name="UseConsistentWhitespaceErrorBeforeClosingInnerBrace" xml:space="preserve">
+    <value>Use space before closing brace.</value>
+  </data>
+  <data name="UseConsistentWhitespaceErrorSpaceAfterPipe" xml:space="preserve">
+    <value>Use space after pipe.</value>
+  </data>
+  <data name="UseConsistentWhitespaceErrorSpaceBeforePipe" xml:space="preserve">
+    <value>Use space before pipe.</value>
+  </data>
 </root>

Rules/Strings.resx

@@ -22,7 +22,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 #endif
     public class UseConsistentWhitespace : ConfigurableRule
     {
-        private enum ErrorKind { Brace, Paren, Operator, SeparatorComma, SeparatorSemi };
+        private enum ErrorKind { BeforeOpeningBrace, Paren, Operator, SeparatorComma, SeparatorSemi, AfterOpeningBrace, BeforeClosingBrace, BeforePipe, AfterPipe };
         private const int whiteSpaceSize = 1;
         private const string whiteSpace = " ";
         private readonly SortedSet<TokenKind> openParenKeywordWhitelist = new SortedSet<TokenKind>()
@@ -41,6 +41,12 @@ private List<Func<TokenOperations, IEnumerable<DiagnosticRecord>>> violationFind
         [ConfigurableRuleProperty(defaultValue: true)]
         public bool CheckOpenBrace { get; protected set; }
 
+        [ConfigurableRuleProperty(defaultValue: true)]
+        public bool CheckInnerBrace { get; protected set; }
+
+        [ConfigurableRuleProperty(defaultValue: true)]
+        public bool CheckPipe { get; protected set; }
+
         [ConfigurableRuleProperty(defaultValue: true)]
         public bool CheckOpenParen { get; protected set; }
 
@@ -58,6 +64,16 @@ public override void ConfigureRule(IDictionary<string, object> paramValueMap)
                 violationFinders.Add(FindOpenBraceViolations);
             }
 
+            if (CheckInnerBrace)
+            {
+                violationFinders.Add(FindInnerBraceViolations);
+            }
+
+            if (CheckPipe)
+            {
+                violationFinders.Add(FindPipeViolations);
+            }
+
             if (CheckOpenParen)
             {
                 violationFinders.Add(FindOpenParenViolations);
@@ -171,10 +187,18 @@ private string GetError(ErrorKind kind)
         {
             switch (kind)
             {
-                case ErrorKind.Brace:
-                    return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeBrace);
+                case ErrorKind.BeforeOpeningBrace:
+                    return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeOpeningBrace);
+                case ErrorKind.AfterOpeningBrace:
+                    return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorAfterOpeningBrace);
+                case ErrorKind.BeforeClosingBrace:
+                    return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeClosingInnerBrace);
                 case ErrorKind.Operator:
                     return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorOperator);
+                case ErrorKind.BeforePipe:
+                    return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSpaceBeforePipe);
+                case ErrorKind.AfterPipe:
+                    return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSpaceAfterPipe);
                 case ErrorKind.SeparatorComma:
                     return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSeparatorComma);
                 case ErrorKind.SeparatorSemi:
@@ -200,7 +224,7 @@ private IEnumerable<DiagnosticRecord> FindOpenBraceViolations(TokenOperations to
                 if (!IsPreviousTokenApartByWhitespace(lcurly))
                 {
                     yield return new DiagnosticRecord(
-                        GetError(ErrorKind.Brace),
+                        GetError(ErrorKind.BeforeOpeningBrace),
                         lcurly.Value.Extent,
                         GetName(),
                         GetDiagnosticSeverity(),
@@ -211,6 +235,111 @@ private IEnumerable<DiagnosticRecord> FindOpenBraceViolations(TokenOperations to
             }
         }
 
+        private IEnumerable<DiagnosticRecord> FindInnerBraceViolations(TokenOperations tokenOperations)
+        {
+            foreach (var lCurly in tokenOperations.GetTokenNodes(TokenKind.LCurly))
+            {
+                if (lCurly.Next == null
+                    || !IsPreviousTokenOnSameLine(lCurly)
+                    || lCurly.Next.Value.Kind == TokenKind.NewLine
+                    || lCurly.Next.Value.Kind == TokenKind.LineContinuation
+                    )
+                {
+                    continue;
+                }
+
+                if (!IsNextTokenApartByWhitespace(lCurly))
+                {
+                    yield return new DiagnosticRecord(
+                        GetError(ErrorKind.AfterOpeningBrace),
+                        lCurly.Value.Extent,
+                        GetName(),
+                        GetDiagnosticSeverity(),
+                        tokenOperations.Ast.Extent.File,
+                        null,
+                        GetCorrections(lCurly.Previous.Value, lCurly.Value, lCurly.Next.Value, true, false).ToList());
+                }
+            }
+
+            foreach (var rCurly in tokenOperations.GetTokenNodes(TokenKind.RCurly))
+            {
+                if (rCurly.Previous == null
+                    || !IsPreviousTokenOnSameLine(rCurly)
+                    || rCurly.Previous.Value.Kind == TokenKind.LCurly
+                    || rCurly.Previous.Value.Kind == TokenKind.NewLine
+                    || rCurly.Previous.Value.Kind == TokenKind.LineContinuation
+                    )
+                {
+                    continue;
+                }
+
+                if (!IsPreviousTokenApartByWhitespace(rCurly))
+                {
+                    yield return new DiagnosticRecord(
+                        GetError(ErrorKind.BeforeClosingBrace),
+                        rCurly.Value.Extent,
+                        GetName(),
+                        GetDiagnosticSeverity(),
+                        tokenOperations.Ast.Extent.File,
+                        null,
+                        GetCorrections(rCurly.Previous.Value, rCurly.Value, rCurly.Next.Value, false, true).ToList());
+                }
+            }
+        }
+
+        private IEnumerable<DiagnosticRecord> FindPipeViolations(TokenOperations tokenOperations)
+        {
+            foreach (var pipe in tokenOperations.GetTokenNodes(TokenKind.Pipe))
+            {
+                if (pipe.Next == null
+                    || !IsPreviousTokenOnSameLine(pipe)
+                    || pipe.Next.Value.Kind == TokenKind.Pipe
+                    || pipe.Next.Value.Kind == TokenKind.NewLine
+                    || pipe.Next.Value.Kind == TokenKind.LineContinuation
+                    )
+                {
+                    continue;
+                }
+
+                if (!IsNextTokenApartByWhitespace(pipe))
+                {
+                    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());
+                }
+            }
+
+            foreach (var pipe in tokenOperations.GetTokenNodes(TokenKind.Pipe))
+            {
+                if (pipe.Previous == null
+                    || !IsPreviousTokenOnSameLine(pipe)
+                    || pipe.Previous.Value.Kind == TokenKind.Pipe
+                    || pipe.Previous.Value.Kind == TokenKind.NewLine
+                    || pipe.Previous.Value.Kind == TokenKind.LineContinuation
+                    )
+                {
+                    continue;
+                }
+
+                if (!IsPreviousTokenApartByWhitespace(pipe))
+                {
+                    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());
+                }
+            }
+        }
+
         private IEnumerable<DiagnosticRecord> FindOpenParenViolations(TokenOperations tokenOperations)
         {
             foreach (var lparen in tokenOperations.GetTokenNodes(TokenKind.LParen))
@@ -291,6 +420,12 @@ private bool IsPreviousTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
                 (tokenNode.Value.Extent.StartColumnNumber - tokenNode.Previous.Value.Extent.EndColumnNumber);
         }
 
+        private bool IsNextTokenApartByWhitespace(LinkedListNode<Token> tokenNode)
+        {
+            return whiteSpaceSize ==
+                (tokenNode.Next.Value.Extent.StartColumnNumber - tokenNode.Value.Extent.EndColumnNumber);
+        }
+
         private bool IsPreviousTokenOnSameLineAndApartByWhitespace(LinkedListNode<Token> tokenNode)
         {
             return IsPreviousTokenOnSameLine(tokenNode) && IsPreviousTokenApartByWhitespace(tokenNode);
@@ -342,8 +477,9 @@ private List<CorrectionExtent> GetCorrections(
             Token prevToken,
             Token token,
             Token nextToken,
-            bool hasWhitespaceBefore,
-            bool hasWhitespaceAfter)
+            bool hasWhitespaceBefore, // if this is false, then the returned correction extent will add a whitespace before the token
+            bool hasWhitespaceAfter   // if this is false, then the returned correction extent will add a whitespace after the token
+            )
         {
             var sb = new StringBuilder();
             IScriptExtent e1 = token.Extent;