diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 3a09d2c56..7dccf0841 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -283,8 +283,9 @@ public bool HasSplattedVariable(CommandAst cmdAst) /// Given a commandast, checks whether positional parameters are used or not. /// /// + /// only return true if more than three positional parameters are used /// - public bool PositionalParameterUsed(CommandAst cmdAst) + public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositional = false) { if (cmdAst == null || cmdAst.GetCommandName() == null) { @@ -351,6 +352,11 @@ public bool PositionalParameterUsed(CommandAst cmdAst) arguments += 1; } + if (moreThanThreePositional && arguments < 3) + { + return false; + } + return arguments > parameters; } diff --git a/Rules/AvoidPositionalParameters.cs b/Rules/AvoidPositionalParameters.cs index a52ac550b..9744096bf 100644 --- a/Rules/AvoidPositionalParameters.cs +++ b/Rules/AvoidPositionalParameters.cs @@ -45,7 +45,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (cmdAst.GetCommandName() == null) continue; if (Helper.Instance.GetCommandInfo(cmdAst.GetCommandName()) != null - && Helper.Instance.PositionalParameterUsed(cmdAst)) + && Helper.Instance.PositionalParameterUsed(cmdAst, true)) { PipelineAst parent = cmdAst.Parent as PipelineAst; @@ -55,14 +55,14 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (parent.PipelineElements[0] == cmdAst) { yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), - cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + cmdAst.Extent, GetName(), DiagnosticSeverity.Information, fileName, cmdAst.GetCommandName()); } } // not in pipeline so just raise it normally else { yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), - cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + cmdAst.Extent, GetName(), DiagnosticSeverity.Information, fileName, cmdAst.GetCommandName()); } } } @@ -109,7 +109,7 @@ public SourceType GetSourceType() /// public RuleSeverity GetSeverity() { - return RuleSeverity.Warning; + return RuleSeverity.Information; } /// diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 2606aab63..63895955c 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -118,7 +118,7 @@ Describe "TestSeverity" { It "filters rules based on multiple severity inputs"{ $rules = Get-ScriptAnalyzerRule -Severity Error,Information - $rules.Count | Should be 13 + $rules.Count | Should be 14 } It "takes lower case inputs" { diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 05a008c48..deecaf6ca 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -143,12 +143,12 @@ Describe "Test IncludeRule" { Context "IncludeRule supports wild card" { It "includes 1 wildcard rule"{ $includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules - $includeWildcard.Count | Should be 3 + $includeWildcard.Count | Should be 0 } it "includes 2 wildcardrules" { $includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules, $useRules - $includeWildcard.Count | Should be 7 + $includeWildcard.Count | Should be 4 } } } @@ -174,12 +174,12 @@ Describe "Test Severity" { It "works with 2 arguments" { $errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity Information, Warning - $errors.Count | Should Be 2 + $errors.Count | Should Be 1 } It "works with lowercase argument"{ $errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity information, warning - $errors.Count | Should Be 2 + $errors.Count | Should Be 1 } } diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index de32e5a94..7902648d7 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -50,7 +50,7 @@ Describe "RuleSuppressionWithScope" { Context "FunctionScope" { It "Does not raise violations" { $suppression = $violations | Where-Object {$_.RuleName -eq "PSAvoidUsingPositionalParameters" } - $suppression.Count | Should Be 1 + $suppression.Count | Should Be 0 } } diff --git a/Tests/Rules/AvoidPositionalParameters.ps1 b/Tests/Rules/AvoidPositionalParameters.ps1 index f2cd77ede..93e3acc38 100644 --- a/Tests/Rules/AvoidPositionalParameters.ps1 +++ b/Tests/Rules/AvoidPositionalParameters.ps1 @@ -1,5 +1,2 @@ -Get-Content Test -Get-ChildItem Tests -Write-Output "I don't want to use positional parameters" -Split-Path "RandomPath" -Leaf -Get-Process | ForEach-Object {Write-Host $_.name -foregroundcolor cyan} \ No newline at end of file +# give it 3 positional parameters +Get-Command "abc" 4 4.3 \ No newline at end of file diff --git a/Tests/Rules/AvoidPositionalParameters.tests.ps1 b/Tests/Rules/AvoidPositionalParameters.tests.ps1 index 2e5156edd..fb7f151ea 100644 --- a/Tests/Rules/AvoidPositionalParameters.tests.ps1 +++ b/Tests/Rules/AvoidPositionalParameters.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Cmdlet 'Write-Host' has positional parameter. Please use named parameters instead of positional parameters when calling a command." +$violationMessage = "Cmdlet 'Get-Command' has positional parameter. Please use named parameters instead of positional parameters when calling a command." $violationName = "PSAvoidUsingPositionalParameters" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidPositionalParameters.ps1 | Where-Object {$_.RuleName -eq $violationName} @@ -8,8 +8,8 @@ $noViolationsDSC = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $director Describe "AvoidPositionalParameters" { Context "When there are violations" { - It "has 4 avoid positional parameters violation" { - $violations.Count | Should Be 5 + It "has 1 avoid positional parameters violation" { + $violations.Count | Should Be 1 } It "has the correct description message" { diff --git a/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 b/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 index 874ef0bd7..1c3841846 100644 --- a/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 +++ b/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 @@ -13,4 +13,11 @@ get-service-computername localhost | where {($_.status -eq "Running") -and ($_.C function TestExternalApplication { & "c:\Windows\System32\Calc.exe" parameter1 -} \ No newline at end of file +} + +# less than 3 arguments so rule won't trigger +Get-Content Test +Get-ChildItem Tests +Write-Output "I don't want to use positional parameters" +Split-Path "RandomPath" -Leaf +Get-Process | ForEach-Object {Write-Host $_.name -foregroundcolor cyan} \ No newline at end of file