Fix PlaceOpenBrace/PlaceCloseBrace corrections leaving stray whitespace and misindented braces

Rules/PlaceCloseBrace.cs

@@ -277,9 +277,18 @@ private List<CorrectionExtent> GetCorrectionsForBraceShouldHaveNewLineAfter(
             var corrections = new List<CorrectionExtent>();
             var nextToken = tokens[closeBracePos + 1];
             var closeBraceToken = tokens[closeBracePos];
+            var indentation = tokens[closeBracePos - 1].Kind == TokenKind.NewLine
+                ? closeBraceToken.Extent.StartScriptPosition.Line.Substring(0, closeBraceToken.Extent.StartColumnNumber - 1)
+                : GetIndentation(tokens, closeBracePos, openBracePos);
+
+            // Also remove the whitespace between the close brace and the next token
+            // and indent the next token to the same level as the close brace
             corrections.Add(new CorrectionExtent(
-                closeBraceToken.Extent,
-                closeBraceToken.Text + Environment.NewLine,
+                closeBraceToken.Extent.StartLineNumber,
+                nextToken.Extent.StartLineNumber,
+                closeBraceToken.Extent.StartColumnNumber,
+                nextToken.Extent.StartColumnNumber,
+                closeBraceToken.Text + Environment.NewLine + indentation,
                 fileName));
             return corrections;
         }
@@ -296,7 +305,7 @@ private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePo
             // if open brace follows any keywords use the identation of the first keyword
             // on the line containing the open brace
             Token firstTokenOnOpenBraceLine = openBraceToken;
-            for (int k = openBracePos; k > 0; --k)
+            for (int k = openBracePos; k >= 0; --k)
             {
                 if (tokens[k].Extent.StartLineNumber == firstTokenOnOpenBraceLine.Extent.StartLineNumber)
                 {
@@ -433,10 +442,21 @@ private List<CorrectionExtent> GetCorrectionsForBraceShouldBeOnNewLine(
         {
             var corrections = new List<CorrectionExtent>();
             var closeBraceToken = tokens[closeBracePos];
+
+            // Also remove any whitespace between the preceding token and the close brace
+            // so that the preceding line does not end with a trailing whitespace
+            var startLineNumber = closeBraceToken.Extent.StartLineNumber;
+            var startColumnNumber = closeBraceToken.Extent.StartColumnNumber;
+            var precedingTokenExtent = tokens[closeBracePos - 1].Extent;
+            if (precedingTokenExtent.EndLineNumber == startLineNumber)
+            {
+                startColumnNumber = precedingTokenExtent.EndColumnNumber;
+            }
+
             corrections.Add(new CorrectionExtent(
-                closeBraceToken.Extent.StartLineNumber,
+                startLineNumber,
                 closeBraceToken.Extent.EndLineNumber,
-                closeBraceToken.Extent.StartColumnNumber,
+                startColumnNumber,
                 closeBraceToken.Extent.EndColumnNumber,
                 Environment.NewLine + GetIndentation(tokens, closeBracePos, openBracePos) + closeBraceToken.Extent.Text,
                 fileName));

Rules/PlaceOpenBrace.cs

@@ -288,12 +288,24 @@ private List<CorrectionExtent> GetCorrectionsForNoNewLineAfterBrace(
             var corrections = new List<CorrectionExtent>();
             var extent = tokens[openBracePos].Extent;
 
+            // Also remove any whitespace between the open brace and the next token on the same line
+            // so that the next token does not start with a leading whitespace on the new line
+            var endColumnNumber = extent.EndColumnNumber;
+            if (openBracePos + 1 < tokens.Length)
+            {
+                var nextTokenExtent = tokens[openBracePos + 1].Extent;
+                if (nextTokenExtent.StartLineNumber == extent.EndLineNumber)
+                {
+                    endColumnNumber = nextTokenExtent.StartColumnNumber;
+                }
+            }
+
             corrections.Add(
                 new CorrectionExtent(
                     extent.StartLineNumber,
                     extent.EndLineNumber,
                     extent.StartColumnNumber,
-                    extent.EndColumnNumber,
+                    endColumnNumber,
                     new StringBuilder().Append(extent.Text).Append(Environment.NewLine).ToString(),
                     fileName));
             return corrections;

Tests/Rules/PlaceCloseBrace.tests.ps1

@@ -174,12 +174,77 @@ if (Test-Path "blah") {
                 RawContent       = $def
                 DiagnosticRecord = $violations[0]
                 CorrectionsCount = 1
-                ViolationText    = '}'
+                ViolationText    = '} '
                 CorrectionText   = '}' + [System.Environment]::NewLine
             }
             Test-CorrectionExtentFromContent @params
         }
 
+        It "Should preserve close brace indentation after a multiline if condition" {
+            $def = @'
+if (
+    $true) {
+    $true
+} else {
+    $false
+}
+'@
+            $expected = @'
+if (
+    $true) {
+    $true
+}
+else {
+    $false
+}
+'@
+            Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
+        }
+
+        It "Should preserve nested close brace indentation after a multiline if condition" {
+            $def = @'
+function Test {
+    if (
+        $true) {
+        $true
+    } else {
+        $false
+    }
+}
+'@
+            $expected = @'
+function Test {
+    if (
+        $true) {
+        $true
+    }
+    else {
+        $false
+    }
+}
+'@
+            Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
+        }
+
+        It "Should preserve tab indentation before a close brace" {
+            $def = @'
+if ($true) {
+    $true
+	}	else {
+	    $false
+	}
+'@
+            $expected = @'
+if ($true) {
+    $true
+	}
+	else {
+	    $false
+	}
+'@
+            Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
+        }
+
         It "Should find a violation for a close brace followed by elseif" {
             $def = @'
 if (Test-Path "blah") {
@@ -194,7 +259,7 @@ if (Test-Path "blah") {
                 RawContent       = $def
                 DiagnosticRecord = $violations[0]
                 CorrectionsCount = 1
-                ViolationText    = '}'
+                ViolationText    = '} '
                 CorrectionText   = '}' + [System.Environment]::NewLine
             }
             Test-CorrectionExtentFromContent @params
@@ -215,7 +280,7 @@ try {
                 RawContent       = $def
                 DiagnosticRecord = $violations[0]
                 CorrectionsCount = 1
-                ViolationText    = '}'
+                ViolationText    = '} '
                 CorrectionText   = '}' + [System.Environment]::NewLine
             }
             Test-CorrectionExtentFromContent @params
@@ -428,4 +493,73 @@ else {
             Invoke-Formatter -ScriptDefinition $OTBSDefinition -Settings 'CodeFormattingStroustrup' | Should -Be $StroustrupDefinition
         }
     }
+
+    Context "When formatting a one line if/else statement with IgnoreOneLineBlock set to false" {
+        BeforeAll {
+            $oneLineDefinition = "if (`$RegEx) { '-match' } else { '-like' }"
+        }
+
+        It "Should not leave stray whitespace or misplace the close braces" {
+            $formatterSettings = @{
+                Rules = @{
+                    PSPlaceOpenBrace  = @{ Enable = $true; IgnoreOneLineBlock = $false }
+                    PSPlaceCloseBrace = @{ Enable = $true; IgnoreOneLineBlock = $false }
+                }
+            }
+            $expected = @"
+if (`$RegEx) {
+'-match'
+}
+else {
+'-like'
+}
+"@
+            $expected = $expected.Replace("`n", [System.Environment]::NewLine)
+            Invoke-Formatter -ScriptDefinition $oneLineDefinition -Settings $formatterSettings | Should -Be $expected
+        }
+
+        It "Should indent the close brace to the level of its statement" {
+            $formatterSettings = @{
+                Rules = @{
+                    PSPlaceOpenBrace  = @{ Enable = $true; IgnoreOneLineBlock = $false }
+                    PSPlaceCloseBrace = @{ Enable = $true; IgnoreOneLineBlock = $false }
+                }
+            }
+            $def = @"
+function foo {
+    if (`$RegEx) {
+        '-match'
+    } else {
+        '-like'
+    }
+}
+"@
+            $expected = @"
+function foo {
+    if (`$RegEx) {
+        '-match'
+    }
+    else {
+        '-like'
+    }
+}
+"@
+            Invoke-Formatter -ScriptDefinition $def -Settings $formatterSettings | Should -Be $expected
+        }
+
+        It "Should format to OTBS style" {
+            $formatterSettings = Import-PowerShellDataFile (Join-Path (Get-Module PSScriptAnalyzer).ModuleBase 'Settings/CodeFormattingOTBS.psd1')
+            $formatterSettings.Rules.PSPlaceOpenBrace.IgnoreOneLineBlock = $false
+            $formatterSettings.Rules.PSPlaceCloseBrace.IgnoreOneLineBlock = $false
+            $expected = @"
+if (`$RegEx) {
+    '-match'
+} else {
+    '-like'
+}
+"@
+            $expected = $expected.Replace("`n", [System.Environment]::NewLine)
+            Invoke-Formatter -ScriptDefinition $oneLineDefinition -Settings $formatterSettings | Should -Be $expected
+        }
+    }
 }

Tests/Rules/PlaceOpenBrace.tests.ps1

@@ -168,5 +168,14 @@ function foo { }
         It "Should mark only the open brace" {
             $violations[0].Extent.Text | Should -Be '{'
         }
+
+        It "Should remove the whitespace following the open brace when correcting" {
+            $def = "if (`$true) { 'blah' }"
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+            $violations.Count | Should -Be 1
+            $violations[0].SuggestedCorrections[0].Text | Should -Be ('{' + [System.Environment]::NewLine)
+            $violations[0].SuggestedCorrections[0].StartColumnNumber | Should -Be 12
+            $violations[0].SuggestedCorrections[0].EndColumnNumber | Should -Be 14
+        }
     }
 }