diff --git a/Engine/Generic/CorrectionExtent.cs b/Engine/Generic/CorrectionExtent.cs index f4857c597..e2d57c5ae 100644 --- a/Engine/Generic/CorrectionExtent.cs +++ b/Engine/Generic/CorrectionExtent.cs @@ -128,6 +128,21 @@ public CorrectionExtent( } + public CorrectionExtent( + IScriptExtent violationExtent, + string replacementText, + string filePath) + : this( + violationExtent.StartLineNumber, + violationExtent.EndLineNumber, + violationExtent.StartColumnNumber, + violationExtent.EndColumnNumber, + replacementText, + filePath) + { + + } + private void ThrowIfInvalidArguments() { ThrowIfNull(text, "text"); diff --git a/RuleDocumentation/PlaceCloseBrace.md b/RuleDocumentation/PlaceCloseBrace.md index 12b891dc0..8da7dc92e 100644 --- a/RuleDocumentation/PlaceCloseBrace.md +++ b/RuleDocumentation/PlaceCloseBrace.md @@ -13,6 +13,7 @@ Close brace placement should follow a consistent style. It should be on a new li Enable = $true NoEmptyLineBefore = $false IgnoreOneLineBlock = $true + NewLineAfter = $true } } ``` @@ -28,4 +29,7 @@ Create violation if there is an empty line before a close brace. #### IgnoreOneLineBlock: bool (Default value is `$true`) Indicates if close braces in a one line block should be ignored or not. E.g. $x = if ($true) { "blah" } else { "blah blah" } -In the above example, if the property is set to true then the rule will not fire a violation. \ No newline at end of file +In the above example, if the property is set to true then the rule will not fire a violation. + +#### NewLineAfter: bool (Default value is `$true`) +Indicates if a new line should follow a close brace. If set to true a close brace should be followed by a new line. diff --git a/Rules/PlaceCloseBrace.cs b/Rules/PlaceCloseBrace.cs index 671f496f3..bc58131ad 100644 --- a/Rules/PlaceCloseBrace.cs +++ b/Rules/PlaceCloseBrace.cs @@ -31,7 +31,7 @@ public class PlaceCloseBrace : ConfigurableRule /// /// Indicates if there should or should not be an empty line before a close brace. /// - /// Default value if false. + /// Default value is false. /// [ConfigurableRuleProperty(defaultValue:false)] public bool NoEmptyLineBefore { get; protected set; } @@ -42,11 +42,21 @@ public class PlaceCloseBrace : ConfigurableRule /// In the above example, if the property is set to true then the rule will /// not fire a violation. /// - /// Default value if true. + /// Default value is true. /// [ConfigurableRuleProperty(defaultValue: true)] public bool IgnoreOneLineBlock { get; protected set; } + /// + /// Indicates if a new line should follow a close brace. + /// + /// If set to true a close brace should be followed by a new line. + /// + /// Default value is true. + /// + [ConfigurableRuleProperty(defaultValue: true)] + public bool NewLineAfter { get; protected set; } + private HashSet tokensToIgnore; /// @@ -121,6 +131,13 @@ public override IEnumerable AnalyzeScript(Ast ast, string file GetViolationForBraceShouldNotFollowEmptyLine(tokens, k, openBracePos, fileName), ref diagnosticRecords); } + + if (NewLineAfter) + { + AddToDiagnosticRecords( + GetViolationForBraceShouldHaveNewLineAfter(tokens, k, openBracePos, fileName), + ref diagnosticRecords); + } } else { @@ -244,6 +261,22 @@ private List GetCorrectionsForBraceShouldNotFollowEmptyLine( return corrections; } + private List GetCorrectionsForBraceShouldHaveNewLineAfter( + Token[] tokens, + int closeBracePos, + int openBracePos, + string fileName) + { + var corrections = new List(); + var nextToken = tokens[closeBracePos + 1]; + var closeBraceToken = tokens[closeBracePos]; + corrections.Add(new CorrectionExtent( + closeBraceToken.Extent, + closeBraceToken.Text + Environment.NewLine, + fileName)); + return corrections; + } + private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePos) { // if open brace on a new line by itself, use its indentation @@ -271,7 +304,40 @@ private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePo return new String(' ', firstTokenOnOpenBraceLine.Extent.StartColumnNumber - 1); } - private DiagnosticRecord GetViolationForBraceShouldBeOnNewLine(Token[] tokens, int closeBracePos, int openBracePos, string fileName) + private DiagnosticRecord GetViolationForBraceShouldHaveNewLineAfter( + Token[] tokens, + int closeBracePos, + int openBracePos, + string fileName) + { + var expectedNewLinePos = closeBracePos + 1; + 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])) + { + + return new DiagnosticRecord( + GetError(Strings.PlaceCloseBraceErrorShouldFollowNewLine), + closeBraceToken.Extent, + GetName(), + GetDiagnosticSeverity(), + fileName, + null, + GetCorrectionsForBraceShouldHaveNewLineAfter(tokens, closeBracePos, openBracePos, fileName)); + } + } + + return null; + } + + private DiagnosticRecord GetViolationForBraceShouldBeOnNewLine( + Token[] tokens, + int closeBracePos, + int openBracePos, + string fileName) { if (tokens.Length > 1 && tokens.Length > closeBracePos) { diff --git a/Rules/Strings.resx b/Rules/Strings.resx index e48a6902c..bb4f9abbc 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -903,6 +903,9 @@ Close brace does not follow a non-empty line. + + Close brace does not follow a new line. + UseConsistentIndentation diff --git a/Tests/Rules/PlaceCloseBrace.tests.ps1 b/Tests/Rules/PlaceCloseBrace.tests.ps1 index 8fd9a4835..5e2f916db 100644 --- a/Tests/Rules/PlaceCloseBrace.tests.ps1 +++ b/Tests/Rules/PlaceCloseBrace.tests.ps1 @@ -1,8 +1,14 @@ -Import-Module PSScriptAnalyzer +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$testRootDirectory = Split-Path -Parent $directory + +Import-Module PSScriptAnalyzer +Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") + $ruleConfiguration = @{ Enable = $true NoEmptyLineBefore = $true IgnoreOneLineBlock = $true + NewLineAfter = $true } $settings = @{ @@ -19,6 +25,9 @@ Describe "PlaceCloseBrace" { function foo { Write-Output "close brace not on a new line"} '@ + $ruleConfiguration.'NoEmptyLineBefore' = $false + $ruleConfiguration.'IgnoreOneLineBlock' = $false + $ruleConfiguration.'NewLineAfter' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings } @@ -39,6 +48,9 @@ function foo { } '@ + $ruleConfiguration.'NoEmptyLineBefore' = $true + $ruleConfiguration.'IgnoreOneLineBlock' = $false + $ruleConfiguration.'NewLineAfter' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings } @@ -56,6 +68,9 @@ function foo { $def = @' $hashtable = @{a = 1; b = 2} '@ + $ruleConfiguration.'NoEmptyLineBefore' = $false + $ruleConfiguration.'IgnoreOneLineBlock' = $true + $ruleConfiguration.'NewLineAfter' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings } @@ -71,6 +86,9 @@ $hashtable = @{ a = 1 b = 2} '@ + $ruleConfiguration.'NoEmptyLineBefore' = $false + $ruleConfiguration.'IgnoreOneLineBlock' = $true + $ruleConfiguration.'NewLineAfter' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings } @@ -101,4 +119,41 @@ $x = if ($true) { "blah" } else { "blah blah" } $violations.Count | Should Be 0 } } + + Context "When a close brace should be follow a new line" { + BeforeAll { + $def = @' +if (Test-Path "blah") { + "blah" +} else { + "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 + $params = @{ + RawContent = $def + DiagnosticRecord = $violations[0] + CorrectionsCount = 1 + ViolationText = '}' + CorrectionText = '}' + [System.Environment]::NewLine + } + Test-CorrectionExtentFromContent @params + + $params = @{ + RawContent = $def + DiagnosticRecord = $violations[1] + CorrectionsCount = 1 + ViolationText = '}' + CorrectionText = '}' + [System.Environment]::NewLine + } + Test-CorrectionExtentFromContent @params + } + } }