diff --git a/Rules/PlaceCloseBrace.cs b/Rules/PlaceCloseBrace.cs index 652197afd..6d2bd51b3 100644 --- a/Rules/PlaceCloseBrace.cs +++ b/Rules/PlaceCloseBrace.cs @@ -277,9 +277,18 @@ private List GetCorrectionsForBraceShouldHaveNewLineAfter( var corrections = new List(); 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 GetCorrectionsForBraceShouldBeOnNewLine( { var corrections = new List(); 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)); diff --git a/Rules/PlaceOpenBrace.cs b/Rules/PlaceOpenBrace.cs index 5a74626d9..e2ba4b03d 100644 --- a/Rules/PlaceOpenBrace.cs +++ b/Rules/PlaceOpenBrace.cs @@ -288,12 +288,24 @@ private List GetCorrectionsForNoNewLineAfterBrace( var corrections = new List(); 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; diff --git a/Tests/Rules/PlaceCloseBrace.tests.ps1 b/Tests/Rules/PlaceCloseBrace.tests.ps1 index fb741ee2a..f44d50bb1 100644 --- a/Tests/Rules/PlaceCloseBrace.tests.ps1 +++ b/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 + } + } } diff --git a/Tests/Rules/PlaceOpenBrace.tests.ps1 b/Tests/Rules/PlaceOpenBrace.tests.ps1 index 4c8206102..352c58ffa 100644 --- a/Tests/Rules/PlaceOpenBrace.tests.ps1 +++ b/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 + } } }