New rule - ReviewUnusedParameter

RuleDocumentation/README.md

@@ -43,6 +43,7 @@
 |[ProvideCommentHelp](./ProvideCommentHelp.md) | Information | Yes |
 |[ReservedCmdletChar](./ReservedCmdletChar.md) | Error | |
 |[ReservedParams](./ReservedParams.md) | Error | |
+|[ReviewUnusedParameter](./ReviewUnusedParameter.md) | Warning | |
 |[ShouldProcess](./ShouldProcess.md) | Error | |
 |[UseApprovedVerbs](./UseApprovedVerbs.md) | Warning | |
 |[UseBOMForUnicodeEncodedFile](./UseBOMForUnicodeEncodedFile.md) | Warning | |

RuleDocumentation/ReviewUnusedParameter.md

@@ -0,0 +1,45 @@
+# ReviewUnusedParameter
+
+**Severity Level: Warning**
+
+## Description
+
+This rule identifies parameters declared in a script, scriptblock, or function scope that have not been used in that scope. 
+
+## How
+
+Consider removing the unused parameter.
+
+## Example
+
+### Wrong
+
+``` PowerShell
+function Test-Parameter
+{
+	Param (
+		$Parameter1,
+
+		# this parameter is never called in the function
+		$Parameter2
+	)
+
+	Get-Something $Parameter1
+}
+```
+
+### Correct
+
+``` PowerShell
+function Test-Parameter
+{
+	Param (
+		$Parameter1,
+
+		# now this parameter is being called in the same scope
+		$Parameter2
+	)
+
+	Get-Something $Parameter1 $Parameter2
+}
+```

Rules/ReviewUnusedParameter.cs

@@ -0,0 +1,129 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Management.Automation.Language;
+using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+#if !CORECLR
+using System.ComponentModel.Composition;
+#endif
+using System.Globalization;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
+{
+    /// <summary>
+    /// ReviewUnusedParameter: Check that all declared parameters are used in the script body.
+    /// </summary>
+#if !CORECLR
+[Export(typeof(IScriptRule))]
+#endif
+    public class ReviewUnusedParameter : IScriptRule
+    {
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null)
+            {
+                throw new ArgumentNullException(Strings.NullAstErrorMessage);
+            }
+
+            var scriptBlockAsts = ast.FindAll(oneAst => oneAst is ScriptBlockAst, true);
+            if (scriptBlockAsts == null)
+            {
+                yield break;
+            }
+
+            foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts)
+            {
+                // find all declared parameters
+                IEnumerable<Ast> parameterAsts = scriptBlockAst.FindAll(oneAst => oneAst is ParameterAst, false);
+
+                // list all variables
+                IEnumerable<string> variables = scriptBlockAst.FindAll(oneAst => oneAst is VariableExpressionAst, false)
+                    .Cast<VariableExpressionAst>()
+                    .Select(variableExpressionAst => variableExpressionAst.VariablePath.ToString());
+
+                foreach (ParameterAst parameterAst in parameterAsts)
+                {
+                    // compare the list of variables to the parameter name
+                    // there should be at least two matches of the variable name since the parameter declaration counts as one
+                    int matchCount = variables
+                        .Where(variable => variable == parameterAst.Name.VariablePath.ToString())
+                        .Take(2).Count();
+                    if (matchCount == 2)
+                    {
+                        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,
+                        GetName(),
+                        DiagnosticSeverity.Warning,
+                        fileName,
+                        parameterAst.Name.VariablePath.UserPath
+                    );
+                }
+            }
+        }
+
+        /// <summary>
+        /// GetName: Retrieves the name of this rule.
+        /// </summary>
+        /// <returns>The name of this rule</returns>
+        public string GetName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.ReviewUnusedParameterName);
+        }
+
+        /// <summary>
+        /// GetCommonName: Retrieves the common name of this rule.
+        /// </summary>
+        /// <returns>The common name of this rule</returns>
+        public string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.ReviewUnusedParameterCommonName);
+        }
+
+        /// <summary>
+        /// GetDescription: Retrieves the description of this rule.
+        /// </summary>
+        /// <returns>The description of this rule</returns>
+        public string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.ReviewUnusedParameterDescription);
+        }
+
+        /// <summary>
+        /// GetSourceType: Retrieves the type of the rule, builtin, managed or module.
+        /// </summary>
+        public SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+
+        /// <summary>
+        /// GetSeverity: Retrieves the severity of the rule: error, warning of information.
+        /// </summary>
+        /// <returns></returns>
+        public RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        /// <summary>
+        /// GetSourceName: Retrieves the module/assembly name the rule is from.
+        /// </summary>
+        public string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+    }
+}

Rules/Strings.resx

@@ -1104,4 +1104,16 @@
   <data name="UseProcessBlockForPipelineCommandName" xml:space="preserve">
     <value>UseProcessBlockForPipelineCommand</value>
   </data>
+  <data name="ReviewUnusedParameterCommonName" xml:space="preserve">
+    <value>ReviewUnusedParameter</value>
+  </data>
+  <data name="ReviewUnusedParameterDescription" xml:space="preserve">
+    <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>
+  </data>
+  <data name="ReviewUnusedParameterName" xml:space="preserve">
+    <value>ReviewUnusedParameter</value>
+  </data>
 </root>

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

@@ -59,7 +59,7 @@ Describe "Test Name parameters" {
 
         It "get Rules with no parameters supplied" {
             $defaultRules = Get-ScriptAnalyzerRule
-            $expectedNumRules = 62
+            $expectedNumRules = 63
             if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4))
             {
                 # for PSv3 PSAvoidGlobalAliases is not shipped because

Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1

@@ -43,7 +43,7 @@ Describe "AvoidAssignmentToAutomaticVariables" {
         It "Using Variable <VariableName> as parameter name produces warning of Severity error" -TestCases $testCases_ReadOnlyVariables {
             param ($VariableName, $ExpectedSeverity)
 
-            [System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo{Param(`$$VariableName)}"
+            [System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo{Param(`$$VariableName)}" -ExcludeRule PSReviewUnusedParameter
             $warnings.Count | Should -Be 1
             $warnings.Severity | Should -Be $ExpectedSeverity
             $warnings.RuleName | Should -Be $ruleName
@@ -59,7 +59,7 @@ Describe "AvoidAssignmentToAutomaticVariables" {
         }
 
         It "Does not flag parameter attributes" {
-            [System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'function foo{Param([Parameter(Mandatory=$true)]$param1)}'
+            [System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'function foo{Param([Parameter(Mandatory=$true)]$param1)}' -ExcludeRule PSReviewUnusedParameter
             $warnings.Count | Should -Be 0
         }
 
@@ -86,7 +86,7 @@ Describe "AvoidAssignmentToAutomaticVariables" {
                 Set-Variable -Name $VariableName -Value 'foo'
                 continue
             }
-            
+
             # Setting the $Error variable has the side effect of the ErrorVariable to contain only the exception message string, therefore exclude this case.
             # For the library test in WMF 4, assigning a value $PSEdition does not seem to throw an error, therefore this special case is excluded as well.
             if ($VariableName -ne 'Error' -and ($VariableName -ne 'PSEdition' -and $PSVersionTable.PSVersion.Major -ne 4))

Tests/Rules/AvoidPositionalParameters.tests.ps1

@@ -45,7 +45,7 @@ Describe "AvoidPositionalParameters" {
                     [Parameter(Position=3)]$C)
                 }
                 Foo "a" "b" "c"}
-            $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "$sb"
+            $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "$sb" -ExcludeRule PSReviewUnusedParameter
             $warnings.Count | Should -Be 1
             $warnings.RuleName | Should -BeExactly $violationName
         }