New rule - ReviewUnusedParameter

Rules/ReviewUnusedParameter.cs

@@ -28,7 +28,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                 throw new ArgumentNullException(Strings.NullAstErrorMessage);
             }
 
-            var scriptBlockAsts = ast.FindAll(oneAst => oneAst is ScriptBlockAst, true);
+            IEnumerable<Ast> scriptBlockAsts = ast.FindAll(oneAst => oneAst is ScriptBlockAst, true);
             if (scriptBlockAsts == null)
             {
                 yield break;
@@ -42,7 +42,13 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                 // list all variables
                 IEnumerable<string> variables = scriptBlockAst.FindAll(oneAst => oneAst is VariableExpressionAst, false)
                     .Cast<VariableExpressionAst>()
-                    .Select(variableExpressionAst => variableExpressionAst.VariablePath.ToString());
+                    .Select(variableExpressionAst => variableExpressionAst.VariablePath.UserPath);
+
+                // all bets are off if the script uses PSBoundParameters
+                if (variables.Contains("PSBoundParameters"))
+                {
+                    continue;
+                }
 
                 foreach (ParameterAst parameterAst in parameterAsts)
                 {
@@ -56,12 +62,6 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                         continue;
                     }
 
-                    // all bets are off if the script uses PSBoundParameters
-                    if (variables.Contains("PSBoundParameters"))
-                    {
-                        continue;
-                    }
-
                     yield return new DiagnosticRecord(
                         string.Format(CultureInfo.CurrentCulture, Strings.ReviewUnusedParameterError, parameterAst.Name.VariablePath.UserPath),
                         parameterAst.Name.Extent,

Rules/Strings.resx

@@ -1111,7 +1111,7 @@
     <value>Ensure all parameters are used within the same script, scriptblock, or function where they are declared.</value>
   </data>
   <data name="ReviewUnusedParameterError" xml:space="preserve">
-    <value>The parameter {0} has been declared but not used. </value>
+    <value>The parameter '{0}' has been declared but not used. </value>
   </data>
   <data name="ReviewUnusedParameterName" xml:space="preserve">
     <value>ReviewUnusedParameter</value>

Tests/Rules/ReviewUnusedParameter.tests.ps1

@@ -5,63 +5,60 @@ Describe "ReviewUnusedParameter" {
     }
 
     Context "When there are violations" {
-        $Cases = @(
-            @{
-                ScriptDefinition   = 'function BadFunc1 { param ($Param1, $Param2) $Param1}'
-                Name               = "function with 1 unused parameter"
-                NumberOfViolations = 1
-            }
-            @{
-                ScriptDefinition   = 'function BadFunc1 { param ($Param1, $Param2) }'
-                Name               = "function with 2 unused parameters"
-                NumberOfViolations = 2
-            }
-            @{
-                ScriptDefinition   = '{ param ($Param1) }'
-                Name               = "scriptblock with unused parameter"
-                NumberOfViolations = 1
-            }
-            @{
-                ScriptDefinition   = '{ param ($Param1) }; { $Param1 }'
-                Name               = "parameter with same name in different scope"
-                NumberOfViolations = 1
-            }
-        )
-        It "has 1 violation for <Name>" -TestCases $Cases {
-            param ($ScriptDefinition, $Name, $NumberOfViolations)
+        It "has 1 violation - function with 1 unused parameter" {
+            $ScriptDefinition = 'function BadFunc1 { param ($Param1, $Param2) $Param1}'
+            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
+            $Violations.Count | Should -Be 1
+        }
+
+        It "has 2 violations - function with 2 unused parameters" {
+            $ScriptDefinition = 'function BadFunc1 { param ($Param1, $Param2) }'
+            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
+            $Violations.Count | Should -Be 2
+        }
+
+        It "has 1 violation - scriptblock with 1 unused parameter" {
+            $ScriptDefinition = '{ param ($Param1) }'
+            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
+            $Violations.Count | Should -Be 1
+        }
 
+        It "doesn't traverse scriptblock scope" {
+            $ScriptDefinition = '{ param ($Param1) }; { $Param1 }'
+            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
+            $Violations.Count | Should -Be 1
+        }
+
+        It "violations have correct rule and severity" {
+            $ScriptDefinition = 'function BadFunc1 { param ($Param1, $Param2) $Param1}'
             $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
-            $Violations.Count | Should -Be $NumberOfViolations
             $Violations.Severity | Select-Object -Unique | Should -Be $RuleSeverity
             $Violations.RuleName | Select-Object -Unique | Should -Be $RuleName
         }
     }
 
     Context "When there are no violations" {
-        $Cases = @(
-            @{
-                ScriptDefinition = 'function GoodFunc1 { param ($Param1, $Param2) $Param1; $Param2}'
-                Name             = "function with 0 unused parameters"
-            }
-            @{
-                ScriptDefinition = 'function GoodFunc1 { param ($Param1) $Splat = @{InputObject = $Param1}}'
-                Name             = "function with splatted parameter"
-            }
-        )
-        It "has no violations for function <Name>" -TestCases $Cases {
-            param ($ScriptDefinition, $Name)
+        It "has no violations - function that uses all parameters" {
+            $ScriptDefinition = 'function GoodFunc1 { param ($Param1, $Param2) $Param1; $Param2}'
+            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
+            $Violations.Count | Should -Be 0
+        }
 
+        It "has no violations - function with splatting" {
+            $ScriptDefinition = 'function GoodFunc1 { param ($Param1) $Splat = @{InputObject = $Param1}}'
             $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
             $Violations.Count | Should -Be 0
         }
 
         It "has no violations when using PSBoundParameters" {
-            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition 'function Bound { param ($Param1) Get-Foo @PSBoundParameters }' -IncludeRule $RuleName
+            $ScriptDefinition = 'function Bound { param ($Param1) Get-Foo @PSBoundParameters }'
+            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
             $Violations.Count | Should -Be 0
         }
 
-        It "has no violations when parameter is called in child scope" -Skip {
-            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition 'function Param { param ($Param1) function Child { $Param1 } }' -IncludeRule $RuleName
+        It "has no violations when parameter is called in child scope" -skip {
+            $ScriptDefinition = 'function Param { param ($Param1) function Child { $Param1 } }'
+            $Violations = Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName
             $Violations.Count | Should -Be 0
         }
     }