From 70b13221e20c3d4d846acc1a397bc237859ed49a Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 16:19:11 -0800 Subject: [PATCH 1/9] Add a constructor to CorrectionExtent class --- Engine/Generic/CorrectionExtent.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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"); From a91cf7c9a0b3a70d0a86bab5522e4ed21270f47b Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 16:20:25 -0800 Subject: [PATCH 2/9] Add switch to enforce new line after close brace --- Rules/PlaceCloseBrace.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Rules/PlaceCloseBrace.cs b/Rules/PlaceCloseBrace.cs index 671f496f3..45ca516cc 100644 --- a/Rules/PlaceCloseBrace.cs +++ b/Rules/PlaceCloseBrace.cs @@ -47,6 +47,16 @@ public class PlaceCloseBrace : ConfigurableRule [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 not be followed by any keyword. + /// + /// Default value if true. + /// + [ConfigurableRuleProperty(defaultValue: true)] + public bool NewLineAfter { get; protected set; } + private HashSet tokensToIgnore; /// From 761076b97fef692562a341bddd537ae2126ae112 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 16:20:55 -0800 Subject: [PATCH 3/9] Implement logic to enforce line after close brace --- Rules/PlaceCloseBrace.cs | 44 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/Rules/PlaceCloseBrace.cs b/Rules/PlaceCloseBrace.cs index 45ca516cc..ab79bb053 100644 --- a/Rules/PlaceCloseBrace.cs +++ b/Rules/PlaceCloseBrace.cs @@ -131,6 +131,13 @@ public override IEnumerable AnalyzeScript(Ast ast, string file GetViolationForBraceShouldNotFollowEmptyLine(tokens, k, openBracePos, fileName), ref diagnosticRecords); } + + if (NewLineAfter) + { + AddToDiagnosticRecords( + GetViolationForNewLineShouldFollowBrace(tokens, k, openBracePos, fileName), + ref diagnosticRecords); + } } else { @@ -254,6 +261,18 @@ private List GetCorrectionsForBraceShouldNotFollowEmptyLine( return corrections; } + private List GetCorrectionsForNewLineShouldFollowBrace(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 + TokenKind.NewLine.Text(), + fileName)); + return corrections; + } + private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePos) { // if open brace on a new line by itself, use its indentation @@ -281,6 +300,31 @@ private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePo return new String(' ', firstTokenOnOpenBraceLine.Extent.StartColumnNumber - 1); } + private DiagnosticRecord GetViolationForNewLineShouldFollowBrace(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, + GetCorrectionsForNewLineShouldFollowBrace(tokens, closeBracePos, openBracePos, fileName)); + } + } + + return null; + } + private DiagnosticRecord GetViolationForBraceShouldBeOnNewLine(Token[] tokens, int closeBracePos, int openBracePos, string fileName) { if (tokens.Length > 1 && tokens.Length > closeBracePos) From 5616269cbcec7374fc0d2621a583ec730a0a898f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 16:22:01 -0800 Subject: [PATCH 4/9] Add error string for new line after close brace violation --- Rules/Strings.resx | 3 +++ 1 file changed, 3 insertions(+) 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 From f028f7d26bc693d31d18b9569ffb6f0eb2171bd0 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 16:22:40 -0800 Subject: [PATCH 5/9] Add tests to check new line after close brace violation --- Tests/Rules/PlaceCloseBrace.tests.ps1 | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/Tests/Rules/PlaceCloseBrace.tests.ps1 b/Tests/Rules/PlaceCloseBrace.tests.ps1 index 8fd9a4835..b164f0726 100644 --- a/Tests/Rules/PlaceCloseBrace.tests.ps1 +++ b/Tests/Rules/PlaceCloseBrace.tests.ps1 @@ -3,6 +3,7 @@ $ruleConfiguration = @{ Enable = $true NoEmptyLineBefore = $true IgnoreOneLineBlock = $true + NewLineAfter = $true } $settings = @{ @@ -19,6 +20,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 +43,9 @@ function foo { } '@ + $ruleConfiguration.'NoEmptyLineBefore' = $true + $ruleConfiguration.'IgnoreOneLineBlock' = $false + $ruleConfiguration.'NewLineAfter' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings } @@ -56,6 +63,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 +81,9 @@ $hashtable = @{ a = 1 b = 2} '@ + $ruleConfiguration.'NoEmptyLineBefore' = $false + $ruleConfiguration.'IgnoreOneLineBlock' = $true + $ruleConfiguration.'NewLineAfter' = $false $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings } @@ -101,4 +114,24 @@ $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 + } + } } From c0e8c435155fe85a6cf9a518c3153cdfadba4561 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 17:07:34 -0800 Subject: [PATCH 6/9] Fix correction for new line after close brace --- Rules/PlaceCloseBrace.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/PlaceCloseBrace.cs b/Rules/PlaceCloseBrace.cs index ab79bb053..1aa9949bf 100644 --- a/Rules/PlaceCloseBrace.cs +++ b/Rules/PlaceCloseBrace.cs @@ -268,7 +268,7 @@ private List GetCorrectionsForNewLineShouldFollowBrace(Token[] var closeBraceToken = tokens[closeBracePos]; corrections.Add(new CorrectionExtent( closeBraceToken.Extent, - closeBraceToken.Text + TokenKind.NewLine.Text(), + closeBraceToken.Text + Environment.NewLine, fileName)); return corrections; } From 95a3be4415cc7169be3e80afa1b4c2491753a0a3 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 17:08:20 -0800 Subject: [PATCH 7/9] Add correction tests to new line after close brace --- Tests/Rules/PlaceCloseBrace.tests.ps1 | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/Tests/Rules/PlaceCloseBrace.tests.ps1 b/Tests/Rules/PlaceCloseBrace.tests.ps1 index b164f0726..5e2f916db 100644 --- a/Tests/Rules/PlaceCloseBrace.tests.ps1 +++ b/Tests/Rules/PlaceCloseBrace.tests.ps1 @@ -1,4 +1,9 @@ -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 @@ -132,6 +137,23 @@ if (Test-Path "blah") { 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 } } } From 4bfa3027a9f0f51f3dfe8028b11563e485851af9 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 17:13:35 -0800 Subject: [PATCH 8/9] Update close brace rule documentation --- RuleDocumentation/PlaceCloseBrace.md | 6 +++++- Rules/PlaceCloseBrace.cs | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) 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 1aa9949bf..ab6bd2f50 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,7 +42,7 @@ 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; } @@ -50,9 +50,9 @@ public class PlaceCloseBrace : ConfigurableRule /// /// Indicates if a new line should follow a close brace. /// - /// If set to true a close brace should not be followed by any keyword. + /// If set to true a close brace should be followed by a new line. /// - /// Default value if true. + /// Default value is true. /// [ConfigurableRuleProperty(defaultValue: true)] public bool NewLineAfter { get; protected set; } From c26a08b7c94e8cb984dfb59ba055e5cf687a06e4 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Thu, 16 Feb 2017 23:22:24 -0800 Subject: [PATCH 9/9] Update method names of new line after close brace --- Rules/PlaceCloseBrace.cs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/Rules/PlaceCloseBrace.cs b/Rules/PlaceCloseBrace.cs index ab6bd2f50..bc58131ad 100644 --- a/Rules/PlaceCloseBrace.cs +++ b/Rules/PlaceCloseBrace.cs @@ -135,7 +135,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file if (NewLineAfter) { AddToDiagnosticRecords( - GetViolationForNewLineShouldFollowBrace(tokens, k, openBracePos, fileName), + GetViolationForBraceShouldHaveNewLineAfter(tokens, k, openBracePos, fileName), ref diagnosticRecords); } } @@ -261,7 +261,11 @@ private List GetCorrectionsForBraceShouldNotFollowEmptyLine( return corrections; } - private List GetCorrectionsForNewLineShouldFollowBrace(Token[] tokens, int closeBracePos, int openBracePos, string fileName) + private List GetCorrectionsForBraceShouldHaveNewLineAfter( + Token[] tokens, + int closeBracePos, + int openBracePos, + string fileName) { var corrections = new List(); var nextToken = tokens[closeBracePos + 1]; @@ -300,7 +304,11 @@ private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePo return new String(' ', firstTokenOnOpenBraceLine.Extent.StartColumnNumber - 1); } - private DiagnosticRecord GetViolationForNewLineShouldFollowBrace(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) @@ -318,14 +326,18 @@ private DiagnosticRecord GetViolationForNewLineShouldFollowBrace(Token[] tokens, GetDiagnosticSeverity(), fileName, null, - GetCorrectionsForNewLineShouldFollowBrace(tokens, closeBracePos, openBracePos, fileName)); + GetCorrectionsForBraceShouldHaveNewLineAfter(tokens, closeBracePos, openBracePos, fileName)); } } return null; } - private DiagnosticRecord GetViolationForBraceShouldBeOnNewLine(Token[] tokens, int closeBracePos, int openBracePos, string fileName) + private DiagnosticRecord GetViolationForBraceShouldBeOnNewLine( + Token[] tokens, + int closeBracePos, + int openBracePos, + string fileName) { if (tokens.Length > 1 && tokens.Length > closeBracePos) {