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/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 e38696bdb..738ccfeb3 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() @@ -56,6 +57,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,6 +114,11 @@ public override IEnumerable AnalyzeScript(Ast ast, string file diagnosticRecords = diagnosticRecords.Concat(violationFinder(tokenOperations)); } + if (CheckParameter) + { + diagnosticRecords = diagnosticRecords.Concat(FindParameterViolations(ast)); + } + return diagnosticRecords.ToArray(); // force evaluation here } @@ -203,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); } @@ -364,6 +375,46 @@ private IEnumerable FindOpenParenViolations(TokenOperations to } } + private IEnumerable FindParameterViolations(Ast ast) + { + IEnumerable commandAsts = ast.FindAll( + testAst => testAst is CommandAst, true); + foreach (CommandAst commandAst in commandAsts) + { + List commandParameterAstElements = commandAst.FindAll(testAst => true, searchNestedScriptBlocks: false).ToList(); + for (int i = 0; i < commandParameterAstElements.Count - 1; i++) + { + 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) + { + int numberOfRedundantWhiteSpaces = rightExtent.StartColumnNumber - expectedStartColumnNumberOfRightExtent; + var correction = new CorrectionExtent( + 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), + leftExtent, + GetName(), + GetDiagnosticSeverity(), + leftExtent.File, + suggestedCorrections: new CorrectionExtent[] { correction }); + } + } + } + } + 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..fb7864d7a 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,69 @@ 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 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 @splattedVariable -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' + } + + 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" + } + } }