Handle open brace in command element

Author: Kapil Borle

Rules/PlaceOpenBrace.cs

@@ -30,7 +30,8 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 #endif
     public class PlaceOpenBrace : ConfigurableScriptRule
     {
-        private List<Func<Token[], string, IEnumerable<DiagnosticRecord>>> violationFinders = new List<Func<Token[], string, IEnumerable<DiagnosticRecord>>>();
+        private List<Func<Token[], Ast, string, IEnumerable<DiagnosticRecord>>> violationFinders
+            = new List<Func<Token[], Ast, string, IEnumerable<DiagnosticRecord>>>();
 
         [ConfigurableRuleProperty()]
         public bool OnSameLine { get; protected set; } = true;
@@ -53,21 +54,48 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
 
             // TODO Should have the following option
             // * no-empty-lines-after
-            // TODO handle open brace for a command parameter.
+            var diagnosticRecords = new List<DiagnosticRecord>();
+            if (!Enable)
+            {
+                return diagnosticRecords;
+            }
+
+            var tokens = Helper.Instance.Tokens;
+            foreach (var violationFinder in violationFinders)
+            {
+                diagnosticRecords.AddRange(violationFinder(tokens, ast, fileName));
+            }
+
+            return diagnosticRecords;
+        }
+
+        private static HashSet<Token> FindTokensToIgnore(Token[] tokens, Ast ast)
+        {
+            // Ignore open braces that are part of arguments to a command
             // * E.g. get-process | % { "blah }
             // In the above case even if OnSameLine == false, we should not
             // flag the open brace as it would move the brace to the next line
-            // and it will invalidate the command
-            var diagnosticRecords = new List<DiagnosticRecord>();
-            if (Enable)
+            // and will invalidate the command
+
+            var cmdElemAsts = ast.FindAll(x => x is CommandElementAst && x is ScriptBlockExpressionAst, true);
+            var tokensToIgnore = new HashSet<Token> ();
+            if (cmdElemAsts == null)
             {
-                foreach (var violationFinder in violationFinders)
+                return tokensToIgnore;
+            }
+
+            foreach (var cmdElemAst in cmdElemAsts)
+            {
+                var tokenToIgnore = tokens.FirstOrDefault(
+                    token => token.Kind == TokenKind.LCurly
+                    && cmdElemAst.Extent.StartOffset == token.Extent.StartOffset);
+                if (tokenToIgnore != null)
                 {
-                    diagnosticRecords.AddRange(violationFinder(Helper.Instance.Tokens, fileName));
+                    tokensToIgnore.Add(tokenToIgnore);
                 }
             }
 
-            return diagnosticRecords;
+            return tokensToIgnore;
         }
 
         public override void ConfigureRule(IDictionary<string, object> paramValueMap)
@@ -90,6 +118,7 @@ public override void ConfigureRule(IDictionary<string, object> paramValueMap)
 
         private IEnumerable<DiagnosticRecord> FindViolationsForBraceShouldBeOnSameLine(
             Token[] tokens,
+            Ast ast,
             string fileName)
         {
             for (int k = 2; k < tokens.Length; k++)
@@ -111,6 +140,7 @@ private IEnumerable<DiagnosticRecord> FindViolationsForBraceShouldBeOnSameLine(
 
         private IEnumerable<DiagnosticRecord> FindViolationsForNoNewLineAfterBrace(
             Token[] tokens,
+            Ast ast,
             string fileName)
         {
             for (int k = 0; k < tokens.Length - 1; k++)
@@ -158,8 +188,11 @@ private List<CorrectionExtent> GetCorrectionsForNoNewLineAfterBrace(
 
         private IEnumerable<DiagnosticRecord> FindViolationsForBraceShouldNotBeOnSameLine(
             Token[] tokens,
+            Ast ast,
             string fileName)
         {
+
+            var tokensToIgnore = FindTokensToIgnore(tokens, ast);
             for (int k = 1; k < tokens.Length; k++)
             {
                 if (tokens[k].Kind == TokenKind.EndOfInput)
@@ -168,7 +201,8 @@ private IEnumerable<DiagnosticRecord> FindViolationsForBraceShouldNotBeOnSameLin
                 }
 
                 if (tokens[k].Kind == TokenKind.LCurly
-                    && tokens[k - 1].Kind != TokenKind.NewLine)
+                    && tokens[k - 1].Kind != TokenKind.NewLine
+                    && !tokensToIgnore.Contains(tokens[k]))
                 {
                     yield return new DiagnosticRecord(
                         GetError(),

Tests/Rules/PlaceOpenBrace.tests.ps1

@@ -42,7 +42,12 @@ function foo ($param1) {
 }
 '@
             $ruleConfiguration.'OnSameLine' = $false
+	    $ruleConfiguration.'NewLineAfter' = $false
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+            $defShouldIgnore = @'
+Get-Process | % { "blah" }
+'@
+            $violationsShouldIgnore = Invoke-ScriptAnalyzer -ScriptDefinition $defShouldIgnore -Settings $settings
         }
 
         It "Should find a violation" {
@@ -52,6 +57,10 @@ function foo ($param1) {
         It "Should mark only the open brace" {
             $violations[0].Extent.Text | Should Be '{'
         }
+
+        It "Should ignore violations for a command element" {
+            $violationsShouldIgnore.Count | Should Be 0
+        }
     }
 
     Context "When a new line should follow an open brace" {