Merge pull request #306 from PowerShell/FixPositionalParameter

Change positional parameter rule so it will only be triggered if we h…

Author: Quoc Truong

Engine/Helper.cs

@@ -283,8 +283,9 @@ public bool HasSplattedVariable(CommandAst cmdAst)
         /// Given a commandast, checks whether positional parameters are used or not.
         /// </summary>
         /// <param name="cmdAst"></param>
+        /// <param name="moreThanThreePositional">only return true if more than three positional parameters are used</param>
         /// <returns></returns>
-        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;
         }
 

Rules/AvoidPositionalParameters.cs

@@ -45,7 +45,7 @@ public IEnumerable<DiagnosticRecord> 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<DiagnosticRecord> 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()
         /// <returns></returns>
         public RuleSeverity GetSeverity()
         {
-            return RuleSeverity.Warning;
+            return RuleSeverity.Information;
         }
 
         /// <summary>

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" {

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
         }
     }
 

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
         }
     }
 

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}
+# give it 3 positional parameters
+Get-Command "abc" 4 4.3

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" {

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
-}
+}
+
+# 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}