diff --git a/Rules/PlaceOpenBrace.cs b/Rules/PlaceOpenBrace.cs index ea6bab634..5a74626d9 100644 --- a/Rules/PlaceOpenBrace.cs +++ b/Rules/PlaceOpenBrace.cs @@ -14,7 +14,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// A class to walk an AST to check for violation. + /// A formatting rule about whether braces should start on the same line or not. /// #if !CORECLR [Export(typeof(IScriptRule))] @@ -201,6 +201,15 @@ private IEnumerable FindViolationsForBraceShouldBeOnSameLine( && tokens[k - 1].Kind == TokenKind.NewLine && !tokensToIgnore.Contains(tokens[k])) { + var precedingExpression = tokens[k - 2]; + Token optionalComment = null; + // If a comment is before the open brace, then take the token before the comment + if (precedingExpression.Kind == TokenKind.Comment && k > 2) + { + precedingExpression = tokens[k - 3]; + optionalComment = tokens[k - 2]; + } + yield return new DiagnosticRecord( GetError(Strings.PlaceOpenBraceErrorShouldBeOnSameLine), tokens[k].Extent, @@ -208,7 +217,7 @@ private IEnumerable FindViolationsForBraceShouldBeOnSameLine( GetDiagnosticSeverity(), fileName, null, - GetCorrectionsForBraceShouldBeOnSameLine(tokens[k - 2], tokens[k], fileName)); + GetCorrectionsForBraceShouldBeOnSameLine(precedingExpression, optionalComment, tokens[k], fileName)); } } } @@ -336,17 +345,19 @@ private int GetStartColumnNumberOfTokenLine(Token[] tokens, int refTokenPos) private List GetCorrectionsForBraceShouldBeOnSameLine( Token precedingExpression, + Token optionalCommentOfPrecedingExpression, Token lCurly, string fileName) { var corrections = new List(); + var optionalComment = optionalCommentOfPrecedingExpression != null ? $" {optionalCommentOfPrecedingExpression}" : string.Empty; corrections.Add( new CorrectionExtent( precedingExpression.Extent.StartLineNumber, lCurly.Extent.EndLineNumber, precedingExpression.Extent.StartColumnNumber, lCurly.Extent.EndColumnNumber, - precedingExpression.Text + " " + lCurly.Text, + $"{precedingExpression.Text} {lCurly.Text}{optionalComment}", fileName)); return corrections; } diff --git a/Tests/Rules/PlaceOpenBrace.tests.ps1 b/Tests/Rules/PlaceOpenBrace.tests.ps1 index e72278b81..b1ec42c8d 100644 --- a/Tests/Rules/PlaceOpenBrace.tests.ps1 +++ b/Tests/Rules/PlaceOpenBrace.tests.ps1 @@ -35,6 +35,59 @@ function foo ($param1) } } + Context "Handling of comments when using Invoke-Formatter" { + It "Should correct violation when brace should be on the same line" { + $scriptDefinition = @' +foreach ($x in $y) +{ + Get-Something +} +'@ + $expected = @' +foreach ($x in $y) { + Get-Something +} +'@ + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings 'CodeFormattingStroustrup' | Should -Be $expected + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings 'CodeFormattingOTBS' | Should -Be $expected + } + + It "Should correct violation when brace should be on the same line and take comment into account" { + $scriptDefinition = @' +foreach ($x in $y) # useful comment +{ + Get-Something +} +'@ + $expected = @' +foreach ($x in $y) { # useful comment + Get-Something +} +'@ + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings 'CodeFormattingStroustrup' | Should -Be $expected + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings 'CodeFormattingOTBS' | Should -Be $expected + } + + It "Should correct violation when the brace should be on the next line and take comment into account" { + $scriptDefinition = @' +foreach ($x in $y) # useful comment +{ + Get-Something +} +'@ + $expected = @' +foreach ($x in $y) { # useful comment + Get-Something +} +'@ + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings 'CodeFormattingStroustrup' | Should -Be $expected + Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings 'CodeFormattingOTBS' | Should -Be $expected + } + } + Context "When an open brace must be on the same line in a switch statement" { BeforeAll { $def = @'