From dba6ce77189d4dbd246e602d18f60b9307f5c5d1 Mon Sep 17 00:00:00 2001
From: Kapil Borle <kborle@microsoft.com>
Date: Thu, 30 Mar 2017 16:03:04 -0700
Subject: [PATCH 1/2] Apply NewLineAfter option only for if-else clause

This constrains the behavior of PlaceCloseBrace rule when NewLineAfter option is set to true. Previously, if this option is set to true, the rule will trigger on any close brace that is not followed by a new line except if the close brace is part of a command element AND script block expression. This caused the rule to be too aggressive. We illustrate two such instances.

In the following instance the rule would add a new line between the close brace and the comma following it, which makes the command expression invalid.
```powershell
Some-Command -Param1 @{
 key="value"
},@{
 key="value"
}
```
would get formatted to
```powershell
Some-Command -Param1 @{
 key="value"
}
,@{
 key="value"
}
```

In the following instance the rule would add a new line between the close brace and the parameter following it, which again makes the command expression invalid.
```powershell
Some-Command -Param1 @{
 key="value"
} -Param2
```
would get formatted to
```powershell
Some-Command -Param1 @{
 key="value"
}
-Param2
```
---
 Rules/PlaceCloseBrace.cs | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/Rules/PlaceCloseBrace.cs b/Rules/PlaceCloseBrace.cs
index bc58131ad..b36a3e39c 100644
--- a/Rules/PlaceCloseBrace.cs
+++ b/Rules/PlaceCloseBrace.cs
@@ -314,11 +314,9 @@ private DiagnosticRecord GetViolationForBraceShouldHaveNewLineAfter(
             if (tokens.Length > 1 && tokens.Length > expectedNewLinePos)
             {
                 var closeBraceToken = tokens[closeBracePos];
-                if (tokens[expectedNewLinePos].Kind != TokenKind.NewLine
-                    && tokens[expectedNewLinePos].Kind != TokenKind.Comment
-                    && !tokensToIgnore.Contains(tokens[closeBracePos]))
+                if ((tokens[expectedNewLinePos].Kind == TokenKind.Else
+                    || tokens[expectedNewLinePos].Kind == TokenKind.ElseIf))
                     {
-
                     return new DiagnosticRecord(
                         GetError(Strings.PlaceCloseBraceErrorShouldFollowNewLine),
                         closeBraceToken.Extent,

From acafe8527c3f38719bb11d46cf2723009358dd11 Mon Sep 17 00:00:00 2001
From: Kapil Borle <kborle@microsoft.com>
Date: Thu, 30 Mar 2017 16:24:05 -0700
Subject: [PATCH 2/2] Add tests for NewLineAfter behavior

---
 Tests/Rules/PlaceCloseBrace.tests.ps1 | 49 ++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/Tests/Rules/PlaceCloseBrace.tests.ps1 b/Tests/Rules/PlaceCloseBrace.tests.ps1
index 5e2f916db..c37f8fc05 100644
--- a/Tests/Rules/PlaceCloseBrace.tests.ps1
+++ b/Tests/Rules/PlaceCloseBrace.tests.ps1
@@ -122,6 +122,12 @@ $x = if ($true) { "blah" } else { "blah blah" }
 
     Context "When a close brace should be follow a new line" {
         BeforeAll {
+            $ruleConfiguration.'NoEmptyLineBefore' = $false
+            $ruleConfiguration.'IgnoreOneLineBlock' = $false
+            $ruleConfiguration.'NewLineAfter' = $true
+        }
+
+        It "Should find a violation for a close brace followed by else" {
             $def = @'
 if (Test-Path "blah") {
     "blah"
@@ -129,14 +135,8 @@ if (Test-Path "blah") {
     "blah blah"
 }
 '@
-            $ruleConfiguration.'NoEmptyLineBefore' = $false
-            $ruleConfiguration.'IgnoreOneLineBlock' = $false
-            $ruleConfiguration.'NewLineAfter' = $true
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
-        }
-
-        It "Should find two violations" {
-            $violations.Count | Should Be 2
+            $violations.Count | Should Be 1
             $params = @{
                 RawContent = $def
                 DiagnosticRecord = $violations[0]
@@ -145,15 +145,48 @@ if (Test-Path "blah") {
                 CorrectionText = '}' + [System.Environment]::NewLine
             }
             Test-CorrectionExtentFromContent @params
+        }
 
+        It "Should find a violation for a close brace followed by elseif" {
+            $def = @'
+if (Test-Path "blah") {
+    "blah"
+} elseif (Test-Path "blah blah") {
+    "blah blah"
+}
+'@
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+            $violations.Count | Should Be 1
             $params = @{
                 RawContent = $def
-                DiagnosticRecord = $violations[1]
+                DiagnosticRecord = $violations[0]
                 CorrectionsCount = 1
                 ViolationText = '}'
                 CorrectionText = '}' + [System.Environment]::NewLine
             }
             Test-CorrectionExtentFromContent @params
         }
+
+        It "Should not find a violation for a close brace followed by a comma in an array expression" {
+            $def = @'
+Some-Command -Param1 @{
+     key1="value1"
+    },@{
+     key2="value2"
+    }
+'@
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+            $violations.Count | Should Be 0
+        }
+
+        It "Should not find a violation for a close brace followed by parameter in a command expression" {
+            $def = @'
+Some-Command -Param1 @{
+    key="value"
+} -Param2
+'@
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+            $violations.Count | Should Be 0
+        }
     }
 }