Check for new line after open brace in PlaceOpenBrace rule

Author: Kapil Borle

Rules/PlaceOpenBrace.cs

@@ -22,7 +22,6 @@
 
 namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 {
-    // TODO place public in front of all new rules to be discoverable in PS Core
     /// <summary>
     /// A class to walk an AST to check for [violation]
     /// </summary>
@@ -32,12 +31,16 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
     public class PlaceOpenBrace : ConfigurableScriptRule
     {
         private Func<Token[], string, IEnumerable<DiagnosticRecord>> findViolations;
+        private List<Func<Token[], string, IEnumerable<DiagnosticRecord>>> violationFinders = new List<Func<Token[], string, IEnumerable<DiagnosticRecord>>>();
 
         [ConfigurableRuleProperty()]
         public bool OnSameLine { get; protected set; } = true;
 
         [ConfigurableRuleProperty()]
-        public bool Enable {get; protected set;} = false;
+        public bool NewLineAfter { get; protected set; } = true;
+
+        [ConfigurableRuleProperty()]
+        public bool Enable { get; protected set; } = false;
 
         /// <summary>
         /// Analyzes the given ast to find the [violation]
@@ -57,24 +60,35 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
                 ConfigureRule();
                 if (OnSameLine)
                 {
-                    findViolations = this.FindViolationsForBraceShouldBeOnSameLine;
+                    // findViolations = this.FindViolationsForBraceShouldBeOnSameLine;
+                    violationFinders.Add(FindViolationsForBraceShouldBeOnSameLine);
                 }
                 else
                 {
-                    findViolations = this.FindViolationsForBraceShouldNotBeOnSameLine;
+                    // findViolations = this.FindViolationsForBraceShouldNotBeOnSameLine;
+                    violationFinders.Add(FindViolationsForBraceShouldNotBeOnSameLine);
+                }
+
+                if (NewLineAfter)
+                {
+                    violationFinders.Add(FindViolationsForNoNewLineAfterBrace);
                 }
             }
 
-            if (!Enable)
+            var diagnosticRecords = new List<DiagnosticRecord>();
+
+            if (Enable)
             {
-                return Enumerable.Empty<DiagnosticRecord>();
+                foreach (var violationFinder in violationFinders)
+                {
+                    diagnosticRecords.AddRange(violationFinder(Helper.Instance.Tokens, fileName));
+                }
             }
 
             // TODO Should have the following options
             // * new-line-after
             // * no-empty-line-after
-
-            return findViolations(Helper.Instance.Tokens, fileName);
+            return diagnosticRecords;
         }
 
         private IEnumerable<DiagnosticRecord> FindViolationsForBraceShouldBeOnSameLine(
@@ -98,6 +112,46 @@ private IEnumerable<DiagnosticRecord> FindViolationsForBraceShouldBeOnSameLine(
             }
         }
 
+        private IEnumerable<DiagnosticRecord> FindViolationsForNoNewLineAfterBrace(
+            Token[] tokens,
+            string fileName)
+        {
+            for (int k = 0; k < tokens.Length - 1; k++)
+            {
+                if (tokens[k].Kind == TokenKind.LCurly
+                    && tokens[k + 1].Kind != TokenKind.NewLine)
+                {
+                    yield return new DiagnosticRecord(
+                        GetError(),
+                        tokens[k].Extent,
+                        GetName(),
+                        GetDiagnosticSeverity(),
+                        fileName,
+                        null,
+                        GetCorrectionsForNoNewLineAfterBrace(tokens, k, fileName));
+                }
+            }
+        }
+
+        private List<CorrectionExtent> GetCorrectionsForNoNewLineAfterBrace(
+            Token[] tokens,
+            int openBracePos,
+            string fileName)
+        {
+            var corrections = new List<CorrectionExtent>();
+            var extent = tokens[openBracePos].Extent;
+
+            corrections.Add(
+                new CorrectionExtent(
+                    extent.StartLineNumber,
+                    extent.EndLineNumber,
+                    extent.StartColumnNumber,
+                    extent.EndColumnNumber,
+                    new StringBuilder().Append(extent.Text).Append(Environment.NewLine).ToString(),
+                    fileName));
+            return corrections;
+        }
+
         private IEnumerable<DiagnosticRecord> FindViolationsForBraceShouldNotBeOnSameLine(
             Token[] tokens,
             string fileName)

Tests/Rules/PlaceOpenBrace.tests.ps1

@@ -1,16 +1,18 @@
 Import-Module PSScriptAnalyzer
+$ruleConfiguration = @{
+    Enable = $true
+    OnSameLine = $true
+    NewLineAfter = $true
+}
+
 $settings = @{
     IncludeRules = @("PSPlaceOpenBrace")
     Rules = @{
-        PSPlaceOpenBrace = @{
-            Enable = $true
-            OnSameLine = $true
-        }
+        PSPlaceOpenBrace = $ruleConfiguration
     }
 }
 
-
-Describe "PlaceOpenBrace on same line" {
+Describe "PlaceOpenBrace" {
     Context "When an open brace must be on the same line" {
         BeforeAll{
             $def = @'
@@ -19,6 +21,7 @@ function foo ($param1)
 
 }
 '@
+            $ruleConfiguration.'OnSameLine' = $true
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
         }
 
@@ -38,10 +41,27 @@ function foo ($param1) {
 
 }
 '@
+            $ruleConfiguration.'OnSameLine' = $false
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+        }
+
+        It "Should find a violation" {
+            $violations.Count | Should Be 1
+        }
 
-            $settingsNewLine = $settings.Clone()
-            $settingsNewLine["Rules"]["PSPlaceOpenBrace"]["OnSameLine"] = $false
-            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settingsNewLine
+        It "Should mark only the open brace" {
+            $violations[0].Extent.Text | Should Be '{'
+        }
+    }
+
+    Context "When a new line should follow an open brace" {
+        BeforeAll{
+            $def = @'
+function foo { }
+'@
+            $ruleConfiguration.'OnSameLine' = $true
+            $ruleConfiguration.'NewLineAfter' = $true
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
         }
 
         It "Should find a violation" {