From aada802638d4999fcf3a190613571a6d45a82a66 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 4 Jan 2020 22:42:12 +0000 Subject: [PATCH 1/9] find extents of parameters elements --- Rules/UseConsistentWhitespace.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index e38696bdb..3db4cd5d9 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -110,6 +110,23 @@ public override IEnumerable AnalyzeScript(Ast ast, string file diagnosticRecords = diagnosticRecords.Concat(violationFinder(tokenOperations)); } + var commandParameterAsts = ast.FindAll( + testAst => testAst is CommandParameterAst, true); + foreach (CommandParameterAst commandParameterAst in commandParameterAsts) + { + var commandParameterAstElements = commandParameterAst.FindAll(testAst => true, false).ToList(); // no recursive option to avoid getting children from the colon syntax + for (int i = 1; i < commandParameterAstElements.Count - 1; i++) + { + var leftExtent = commandParameterAstElements[i].Extent; + var rightExtent = commandParameterAstElements[i + 1].Extent; + var extentsAreOnSameLine = leftExtent.EndLineNumber == rightExtent.StartLineNumber; + if (!extentsAreOnSameLine) + { + continue; + } + } + } + return diagnosticRecords.ToArray(); // force evaluation here } From a240bd5ead16c043a32502e9edc3311c30ccfb87 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 5 Jan 2020 10:31:30 +0000 Subject: [PATCH 2/9] first working prototype --- Rules/UseConsistentWhitespace.cs | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 3db4cd5d9..0c76518ee 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -110,12 +110,12 @@ public override IEnumerable AnalyzeScript(Ast ast, string file diagnosticRecords = diagnosticRecords.Concat(violationFinder(tokenOperations)); } - var commandParameterAsts = ast.FindAll( - testAst => testAst is CommandParameterAst, true); - foreach (CommandParameterAst commandParameterAst in commandParameterAsts) + var commandAsts = ast.FindAll( + testAst => testAst is CommandAst, true).ToArray(); + foreach (CommandAst commandAst in commandAsts) { - var commandParameterAstElements = commandParameterAst.FindAll(testAst => true, false).ToList(); // no recursive option to avoid getting children from the colon syntax - for (int i = 1; i < commandParameterAstElements.Count - 1; i++) + var commandParameterAstElements = commandAst.FindAll(testAst => true, searchNestedScriptBlocks: false).ToList(); + for (int i = 0; i < commandParameterAstElements.Count - 1; i++) { var leftExtent = commandParameterAstElements[i].Extent; var rightExtent = commandParameterAstElements[i + 1].Extent; @@ -124,6 +124,28 @@ public override IEnumerable AnalyzeScript(Ast ast, string file { continue; } + var expectedStartColumnNumberOfRightExtent = leftExtent.EndColumnNumber + 1; + if (rightExtent.StartColumnNumber > expectedStartColumnNumberOfRightExtent) + { + int numberOfRedundantWhiteSpaces = rightExtent.StartColumnNumber - expectedStartColumnNumberOfRightExtent; + var correction = new CorrectionExtent( + leftExtent.StartLineNumber, + leftExtent.EndLineNumber, + leftExtent.EndColumnNumber + 1, + leftExtent.EndColumnNumber + 1 + numberOfRedundantWhiteSpaces, + string.Empty, + rightExtent.File, + "description TODO"); + + diagnosticRecords = diagnosticRecords.Concat(Enumerable.Repeat(new DiagnosticRecord( + GetError(ErrorKind.BeforeOpeningBrace), + rightExtent, + GetName(), + GetDiagnosticSeverity(), + tokenOperations.Ast.Extent.File, + null, + Enumerable.Repeat(correction, 1)), 1)); + } } } From 5d3384e67d0bfab06ba259c8b02ef53c99079a03 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 5 Jan 2020 21:02:44 +0000 Subject: [PATCH 3/9] Productionize, add tests, docs and add to config files. TODO: message names --- Engine/Settings/CodeFormatting.psd1 | 1 + Engine/Settings/CodeFormattingAllman.psd1 | 1 + Engine/Settings/CodeFormattingOTBS.psd1 | 1 + Engine/Settings/CodeFormattingStroustrup.psd1 | 1 + RuleDocumentation/UseConsistentWhitespace.md | 6 ++ Rules/UseConsistentWhitespace.cs | 83 +++++++++++-------- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 43 ++++++++++ 7 files changed, 100 insertions(+), 36 deletions(-) diff --git a/Engine/Settings/CodeFormatting.psd1 b/Engine/Settings/CodeFormatting.psd1 index 3a0b3bd4c..24b4d52e3 100644 --- a/Engine/Settings/CodeFormatting.psd1 +++ b/Engine/Settings/CodeFormatting.psd1 @@ -38,6 +38,7 @@ CheckOperator = $true CheckPipe = $true CheckSeparator = $true + CheckParameter = $false } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingAllman.psd1 b/Engine/Settings/CodeFormattingAllman.psd1 index af11307c9..bb398e8f9 100644 --- a/Engine/Settings/CodeFormattingAllman.psd1 +++ b/Engine/Settings/CodeFormattingAllman.psd1 @@ -38,6 +38,7 @@ CheckOperator = $true CheckPipe = $true CheckSeparator = $true + CheckParameter = $false } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingOTBS.psd1 b/Engine/Settings/CodeFormattingOTBS.psd1 index 771313f0a..c75ba18bd 100644 --- a/Engine/Settings/CodeFormattingOTBS.psd1 +++ b/Engine/Settings/CodeFormattingOTBS.psd1 @@ -38,6 +38,7 @@ CheckOperator = $true CheckPipe = $true CheckSeparator = $true + CheckParameter = $false } PSAlignAssignmentStatement = @{ diff --git a/Engine/Settings/CodeFormattingStroustrup.psd1 b/Engine/Settings/CodeFormattingStroustrup.psd1 index 9ef08d800..1d13df6c5 100644 --- a/Engine/Settings/CodeFormattingStroustrup.psd1 +++ b/Engine/Settings/CodeFormattingStroustrup.psd1 @@ -39,6 +39,7 @@ CheckOperator = $true CheckPipe = $true CheckSeparator = $true + CheckParameter = $false } PSAlignAssignmentStatement = @{ diff --git a/RuleDocumentation/UseConsistentWhitespace.md b/RuleDocumentation/UseConsistentWhitespace.md index 3070aac5c..f24267a85 100644 --- a/RuleDocumentation/UseConsistentWhitespace.md +++ b/RuleDocumentation/UseConsistentWhitespace.md @@ -20,6 +20,7 @@ CheckOperator = $true CheckPipe = $true CheckSeparator = $true + CheckParameter = $false } } ``` @@ -53,3 +54,8 @@ Checks if a comma or a semicolon is followed by a space. E.g. `@(1, 2, 3)` or `@ #### CheckPipe: bool (Default value is `$true`) Checks if a pipe is surrounded on both sides by a space. E.g. `foo | bar` instead of `foo|bar`. + +#### CheckParameter: bool (Default value is `$false` at the moment due to the setting being new) + +Checks if there is more than one space between parameters and values. E.g. `foo -bar $baz -bat` instead of `foo -bar $baz -bat`. This eliminates redundant whitespace that was probably added unintentionally. +The rule does not check for whitespace between parameter and value when the colon syntax `-ParameterName:$ParameterValue` is used as some users prefer either 0 or 1 whitespace in this case. diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 0c76518ee..5dc5cc7e1 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -56,6 +56,9 @@ private List>> violationFind [ConfigurableRuleProperty(defaultValue: true)] public bool CheckSeparator { get; protected set; } + [ConfigurableRuleProperty(defaultValue: true)] + public bool CheckParameter { get; protected set; } + public override void ConfigureRule(IDictionary paramValueMap) { base.ConfigureRule(paramValueMap); @@ -110,43 +113,9 @@ public override IEnumerable AnalyzeScript(Ast ast, string file diagnosticRecords = diagnosticRecords.Concat(violationFinder(tokenOperations)); } - var commandAsts = ast.FindAll( - testAst => testAst is CommandAst, true).ToArray(); - foreach (CommandAst commandAst in commandAsts) + if (CheckParameter) { - var commandParameterAstElements = commandAst.FindAll(testAst => true, searchNestedScriptBlocks: false).ToList(); - for (int i = 0; i < commandParameterAstElements.Count - 1; i++) - { - var leftExtent = commandParameterAstElements[i].Extent; - var rightExtent = commandParameterAstElements[i + 1].Extent; - var extentsAreOnSameLine = leftExtent.EndLineNumber == rightExtent.StartLineNumber; - if (!extentsAreOnSameLine) - { - continue; - } - var expectedStartColumnNumberOfRightExtent = leftExtent.EndColumnNumber + 1; - if (rightExtent.StartColumnNumber > expectedStartColumnNumberOfRightExtent) - { - int numberOfRedundantWhiteSpaces = rightExtent.StartColumnNumber - expectedStartColumnNumberOfRightExtent; - var correction = new CorrectionExtent( - leftExtent.StartLineNumber, - leftExtent.EndLineNumber, - leftExtent.EndColumnNumber + 1, - leftExtent.EndColumnNumber + 1 + numberOfRedundantWhiteSpaces, - string.Empty, - rightExtent.File, - "description TODO"); - - diagnosticRecords = diagnosticRecords.Concat(Enumerable.Repeat(new DiagnosticRecord( - GetError(ErrorKind.BeforeOpeningBrace), - rightExtent, - GetName(), - GetDiagnosticSeverity(), - tokenOperations.Ast.Extent.File, - null, - Enumerable.Repeat(correction, 1)), 1)); - } - } + diagnosticRecords = diagnosticRecords.Concat(FindParameterViolations(ast)); } return diagnosticRecords.ToArray(); // force evaluation here @@ -403,6 +372,48 @@ private IEnumerable FindOpenParenViolations(TokenOperations to } } + private IEnumerable FindParameterViolations(Ast ast) + { + var commandAsts = ast.FindAll( + testAst => testAst is CommandAst, true).ToArray(); + foreach (CommandAst commandAst in commandAsts) + { + var commandParameterAstElements = commandAst.FindAll(testAst => true, searchNestedScriptBlocks: false).ToList(); + for (int i = 0; i < commandParameterAstElements.Count - 1; i++) + { + var leftExtent = commandParameterAstElements[i].Extent; + var rightExtent = commandParameterAstElements[i + 1].Extent; + var extentsAreOnSameLine = leftExtent.EndLineNumber == rightExtent.StartLineNumber; + if (!extentsAreOnSameLine) + { + continue; + } + var expectedStartColumnNumberOfRightExtent = leftExtent.EndColumnNumber + 1; + if (rightExtent.StartColumnNumber > expectedStartColumnNumberOfRightExtent) + { + int numberOfRedundantWhiteSpaces = rightExtent.StartColumnNumber - expectedStartColumnNumberOfRightExtent; + var correction = new CorrectionExtent( + leftExtent.StartLineNumber, + leftExtent.EndLineNumber, + leftExtent.EndColumnNumber + 1, + leftExtent.EndColumnNumber + 1 + numberOfRedundantWhiteSpaces, + string.Empty, + leftExtent.File, + "description TODO"); + + yield return new DiagnosticRecord( + GetError(ErrorKind.BeforeOpeningBrace), + leftExtent, + GetName(), + GetDiagnosticSeverity(), + leftExtent.File, + null, + Enumerable.Repeat(correction, 1)); + } + } + } + } + private bool IsSeparator(Token token) { return token.Kind == TokenKind.Comma || token.Kind == TokenKind.Semi; diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index fa6be6769..62209e2c7 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -12,6 +12,7 @@ $ruleConfiguration = @{ CheckOperator = $false CheckPipe = $false CheckSeparator = $false + CheckParameter = $false } $settings = @{ @@ -388,4 +389,46 @@ if ($true) { Get-Item ` } } + + Context "CheckParameter" { + BeforeAll { + $ruleConfiguration.CheckInnerBrace = $true + $ruleConfiguration.CheckOpenBrace = $false + $ruleConfiguration.CheckOpenParen = $false + $ruleConfiguration.CheckOperator = $false + $ruleConfiguration.CheckPipe = $false + $ruleConfiguration.CheckSeparator = $false + $ruleConfiguration.CheckParameter = $true + } + + It "Should not find no violation if there is always 1 space between parameters except when using colon syntax" { + $def = 'foo -bar $baz -bat -parameterName:$parameterValue' + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null + } + + It "Should find 1 violation if there is 1 space too much before a parameter" { + $def = 'foo -bar' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'foo' + $violations[0].SuggestedCorrections[0].Text | Should -Be ([string]::Empty) + } + + It "Should find 1 violation if there is 1 space too much before a parameter value" { + $def = 'foo $bar' + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'foo' + $violations[0].SuggestedCorrections[0].Text | Should -Be ([string]::Empty) + } + + It "Should fix script to always have 1 space between parameters except when using colon syntax but not by default" { + $def = 'foo -bar $baz -ParameterName: $ParameterValue' + Invoke-Formatter -ScriptDefinition $def | + Should -BeExactly $def -Because 'CheckParameter configuration is not turned on by default (yet) as the setting is new' + Invoke-Formatter -ScriptDefinition $def -Settings $settings | + Should -BeExactly 'foo -bar $baz -ParameterName: $ParameterValue' + } + + } } From df26814a8c1e206ffb1c751a9afe8c6cb7b34435 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 5 Jan 2020 21:12:51 +0000 Subject: [PATCH 4/9] tidy up --- Rules/UseConsistentWhitespace.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 5dc5cc7e1..25efd56af 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -375,7 +375,7 @@ private IEnumerable FindOpenParenViolations(TokenOperations to private IEnumerable FindParameterViolations(Ast ast) { var commandAsts = ast.FindAll( - testAst => testAst is CommandAst, true).ToArray(); + testAst => testAst is CommandAst, true); foreach (CommandAst commandAst in commandAsts) { var commandParameterAstElements = commandAst.FindAll(testAst => true, searchNestedScriptBlocks: false).ToList(); From 7b7d4fa2a015cd418e0e938a1b83fe6035e7eee9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 5 Jan 2020 21:26:01 +0000 Subject: [PATCH 5/9] add messages --- Rules/Strings.Designer.cs | 89 ++++++++++++++++---------------- Rules/Strings.resx | 5 +- Rules/UseConsistentWhitespace.cs | 10 ++-- 3 files changed, 55 insertions(+), 49 deletions(-) diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 54cba38eb..8fbcbc368 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -492,6 +492,42 @@ internal static string AvoidNullOrEmptyHelpMessageAttributeName { } } + /// + /// Looks up a localized string similar to Avoid overwriting built in cmdlets. + /// + internal static string AvoidOverwritingBuiltInCmdletsCommonName { + get { + return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Do not overwrite the definition of a cmdlet that is included with PowerShell. + /// + internal static string AvoidOverwritingBuiltInCmdletsDescription { + get { + return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to '{0}' is a cmdlet that is included with PowerShell (version {1}) whose definition should not be overridden. + /// + internal static string AvoidOverwritingBuiltInCmdletsError { + get { + return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to AvoidOverwritingBuiltInCmdlets. + /// + internal static string AvoidOverwritingBuiltInCmdletsName { + get { + return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Avoid Using ShouldContinue Without Boolean Force Parameter. /// @@ -2084,50 +2120,6 @@ internal static string UseCompatibleCmdletsName { return ResourceManager.GetString("UseCompatibleCmdletsName", resourceCulture); } } - - /// - /// Looks up a localized string similar to Avoid overwriting built in cmdlets. - /// - internal static string AvoidOverwritingBuiltInCmdletsCommonName - { - get - { - return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsCommonName", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Avoid overwriting built in cmdlets. - /// - internal static string AvoidOverwritingBuiltInCmdletsDescription - { - get - { - return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsDescription", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to '{0}' is a cmdlet that is included with PowerShell whose definition should not be overridden. - /// - internal static string AvoidOverwritingBuiltInCmdletsError - { - get - { - return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsError", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to AvoidOverwritingBuiltInCmdlets. - /// - internal static string AvoidOverwritingBuiltInCmdletsName - { - get - { - return ResourceManager.GetString("AvoidOverwritingBuiltInCmdletsName", resourceCulture); - } - } /// /// Looks up a localized string similar to The command '{0}' is not available by default in PowerShell version '{1}' on platform '{2}'. @@ -2417,6 +2409,15 @@ internal static string UseConsistentWhitespaceErrorSpaceBeforePipe { } } + /// + /// Looks up a localized string similar to Use only 1 whitespace between parameter names or values.. + /// + internal static string UseConsistentWhitespaceErrorSpaceBetweenParameter { + get { + return ResourceManager.GetString("UseConsistentWhitespaceErrorSpaceBetweenParameter", resourceCulture); + } + } + /// /// Looks up a localized string similar to UseConsistentWhitespace. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 50362080f..c2bf6c943 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1104,4 +1104,7 @@ UseProcessBlockForPipelineCommand - + + Use only 1 whitespace between parameter names or values. + + \ No newline at end of file diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 25efd56af..438bef396 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -22,7 +22,8 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class UseConsistentWhitespace : ConfigurableRule { - private enum ErrorKind { BeforeOpeningBrace, Paren, Operator, SeparatorComma, SeparatorSemi, AfterOpeningBrace, BeforeClosingBrace, BeforePipe, AfterPipe }; + private enum ErrorKind { BeforeOpeningBrace, Paren, Operator, SeparatorComma, SeparatorSemi, + AfterOpeningBrace, BeforeClosingBrace, BeforePipe, AfterPipe, BetweenParameter }; private const int whiteSpaceSize = 1; private const string whiteSpace = " "; private readonly SortedSet openParenKeywordWhitelist = new SortedSet() @@ -211,6 +212,8 @@ private string GetError(ErrorKind kind) return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSeparatorComma); case ErrorKind.SeparatorSemi: return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSeparatorSemi); + case ErrorKind.BetweenParameter: + return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorSpaceBetweenParameter); default: return string.Format(CultureInfo.CurrentCulture, Strings.UseConsistentWhitespaceErrorBeforeParen); } @@ -398,11 +401,10 @@ private IEnumerable FindParameterViolations(Ast ast) leftExtent.EndColumnNumber + 1, leftExtent.EndColumnNumber + 1 + numberOfRedundantWhiteSpaces, string.Empty, - leftExtent.File, - "description TODO"); + leftExtent.File); yield return new DiagnosticRecord( - GetError(ErrorKind.BeforeOpeningBrace), + GetError(ErrorKind.BetweenParameter), leftExtent, GetName(), GetDiagnosticSeverity(), From 96b62d62d236974a84fc38a46007d41c39a329dd Mon Sep 17 00:00:00 2001 From: "Christoph Bergmeister [MVP]" Date: Mon, 6 Jan 2020 21:39:57 +0000 Subject: [PATCH 6/9] Apply suggestions from code review Co-Authored-By: Robert Holt --- Rules/UseConsistentWhitespace.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 438bef396..5bf4ce56d 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -377,22 +377,21 @@ private IEnumerable FindOpenParenViolations(TokenOperations to private IEnumerable FindParameterViolations(Ast ast) { - var commandAsts = ast.FindAll( + IEnumerable commandAsts = ast.FindAll( testAst => testAst is CommandAst, true); foreach (CommandAst commandAst in commandAsts) { - var commandParameterAstElements = commandAst.FindAll(testAst => true, searchNestedScriptBlocks: false).ToList(); + List commandParameterAstElements = commandAst.FindAll(testAst => true, searchNestedScriptBlocks: false).ToList(); for (int i = 0; i < commandParameterAstElements.Count - 1; i++) { - var leftExtent = commandParameterAstElements[i].Extent; - var rightExtent = commandParameterAstElements[i + 1].Extent; - var extentsAreOnSameLine = leftExtent.EndLineNumber == rightExtent.StartLineNumber; - if (!extentsAreOnSameLine) + IScriptExtent leftExtent = commandParameterAstElements[i].Extent; + IScriptExtent rightExtent = commandParameterAstElements[i + 1].Extent; + if (leftExtent.EndLineNumber != rightExtent.StartLineNumber) { continue; } - var expectedStartColumnNumberOfRightExtent = leftExtent.EndColumnNumber + 1; - if (rightExtent.StartColumnNumber > expectedStartColumnNumberOfRightExtent) + + if (rightExtent.StartColumnNumber > leftExtent.EndColumnNumber + 1) { int numberOfRedundantWhiteSpaces = rightExtent.StartColumnNumber - expectedStartColumnNumberOfRightExtent; var correction = new CorrectionExtent( From e150a2f516e0c6248a68c2c863ceb8f79fada795 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 6 Jan 2020 21:43:51 +0000 Subject: [PATCH 7/9] revert suggestion around removal around expectedStartColumnNumberOfRightExtent variable, which broke build. Variable is used 2 times and makes code more readable, therefore keeping it --- Rules/UseConsistentWhitespace.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 5bf4ce56d..2e9621348 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -391,7 +391,8 @@ private IEnumerable FindParameterViolations(Ast ast) continue; } - if (rightExtent.StartColumnNumber > leftExtent.EndColumnNumber + 1) + var expectedStartColumnNumberOfRightExtent = leftExtent.EndColumnNumber + 1; + if (rightExtent.StartColumnNumber > expectedStartColumnNumberOfRightExtent) { int numberOfRedundantWhiteSpaces = rightExtent.StartColumnNumber - expectedStartColumnNumberOfRightExtent; var correction = new CorrectionExtent( From 8f6625cbbd77b9fc4cfbf07908bdaa284bbed037 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 6 Jan 2020 22:26:58 +0000 Subject: [PATCH 8/9] Tweak DiagnosticRecord creation and add more tests --- Rules/UseConsistentWhitespace.cs | 3 +-- Tests/Rules/UseConsistentWhitespace.tests.ps1 | 25 ++++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 2e9621348..04e65de39 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -409,8 +409,7 @@ private IEnumerable FindParameterViolations(Ast ast) GetName(), GetDiagnosticSeverity(), leftExtent.File, - null, - Enumerable.Repeat(correction, 1)); + suggestedCorrections: new CorrectionExtent[] { correction }); } } } diff --git a/Tests/Rules/UseConsistentWhitespace.tests.ps1 b/Tests/Rules/UseConsistentWhitespace.tests.ps1 index 62209e2c7..fb7864d7a 100644 --- a/Tests/Rules/UseConsistentWhitespace.tests.ps1 +++ b/Tests/Rules/UseConsistentWhitespace.tests.ps1 @@ -401,8 +401,17 @@ if ($true) { Get-Item ` $ruleConfiguration.CheckParameter = $true } + It "Should not find no violation when newlines are involved" { + $def = {foo -a $b ` +-c d -d $e -f g ` +-h i | +bar -h i ` +-switch} + Invoke-ScriptAnalyzer -ScriptDefinition "$def" -Settings $settings | Should -Be $null + } + It "Should not find no violation if there is always 1 space between parameters except when using colon syntax" { - $def = 'foo -bar $baz -bat -parameterName:$parameterValue' + $def = 'foo -bar $baz @splattedVariable -bat -parameterName:$parameterValue' Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null } @@ -430,5 +439,19 @@ if ($true) { Get-Item ` Should -BeExactly 'foo -bar $baz -ParameterName: $ParameterValue' } + It "Should fix script when newlines are involved" { + $def = {foo -a $b ` +-c d -d $e -f g ` +-h i | +bar -h i ` +-switch} + $expected = {foo -a $b ` +-c d -d $e -f g ` +-h i | +bar -h i ` +-switch} + Invoke-Formatter -ScriptDefinition "$def" -Settings $settings | + Should -Be "$expected" + } } } From 0e004cd2770a70f45d424f955eb61fb27ce53681 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 6 Jan 2020 22:30:20 +0000 Subject: [PATCH 9/9] use named parameters for CorrectionExtent constructor --- Rules/UseConsistentWhitespace.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Rules/UseConsistentWhitespace.cs b/Rules/UseConsistentWhitespace.cs index 04e65de39..738ccfeb3 100644 --- a/Rules/UseConsistentWhitespace.cs +++ b/Rules/UseConsistentWhitespace.cs @@ -396,12 +396,12 @@ private IEnumerable FindParameterViolations(Ast ast) { int numberOfRedundantWhiteSpaces = rightExtent.StartColumnNumber - expectedStartColumnNumberOfRightExtent; var correction = new CorrectionExtent( - leftExtent.StartLineNumber, - leftExtent.EndLineNumber, - leftExtent.EndColumnNumber + 1, - leftExtent.EndColumnNumber + 1 + numberOfRedundantWhiteSpaces, - string.Empty, - leftExtent.File); + startLineNumber: leftExtent.StartLineNumber, + endLineNumber: leftExtent.EndLineNumber, + startColumnNumber: leftExtent.EndColumnNumber + 1, + endColumnNumber: leftExtent.EndColumnNumber + 1 + numberOfRedundantWhiteSpaces, + text: string.Empty, + file: leftExtent.File); yield return new DiagnosticRecord( GetError(ErrorKind.BetweenParameter),