Enhance UseDeclaredVarsMoreThanAssignments to detect also take into account the usage of Get-Variable with an array of variables and usage of named parameter -Name (#1310)

* Parse Array syntax of Get-Variable

* check named parameter better to be -Name or shortened versions and add more tests

* Fix last edge cases and refactor into method

* Apply PR suggestions: Use shorter pattern matching syntax and inline/simplify local method

Author: bergmeister

Rules/UseDeclaredVarsMoreThanAssignments.cs

@@ -208,33 +208,67 @@ private IEnumerable<DiagnosticRecord> AnalyzeScriptBlockAst(ScriptBlockAst scrip
                 }
             }
 
-            // Detect usages of Get-Variable
+            AnalyzeGetVariableCommands(scriptBlockAst, assignmentsDictionary_OrdinalIgnoreCase);
+
+            foreach (string key in assignmentsDictionary_OrdinalIgnoreCase.Keys)
+            {
+                yield return new DiagnosticRecord(
+                    string.Format(CultureInfo.CurrentCulture, Strings.UseDeclaredVarsMoreThanAssignmentsError, key),
+                    assignmentsDictionary_OrdinalIgnoreCase[key].Left.Extent,
+                    GetName(),
+                    DiagnosticSeverity.Warning,
+                    fileName,
+                    key);
+            }
+        }
+
+        /// <summary>
+        /// Detects variables retrieved by usage of Get-Variable and remove those
+        /// variables from the entries in <paramref name="assignmentsDictionary_OrdinalIgnoreCase"/>.
+        /// </summary>
+        /// <param name="scriptBlockAst"></param>
+        /// <param name="assignmentsDictionary_OrdinalIgnoreCase"></param>
+        private void AnalyzeGetVariableCommands(
+            ScriptBlockAst scriptBlockAst,
+            Dictionary<string, AssignmentStatementAst> assignmentsDictionary_OrdinalIgnoreCase)
+        {
             var getVariableCmdletNamesAndAliases = Helper.Instance.CmdletNameAndAliases("Get-Variable");
             IEnumerable<Ast> getVariableCommandAsts = scriptBlockAst.FindAll(testAst => testAst is CommandAst commandAst &&
                 getVariableCmdletNamesAndAliases.Contains(commandAst.GetCommandName(), StringComparer.OrdinalIgnoreCase), true);
+
             foreach (CommandAst getVariableCommandAst in getVariableCommandAsts)
             {
                 var commandElements = getVariableCommandAst.CommandElements.ToList();
-                // The following extracts the variable name only in the simplest possibe case 'Get-Variable variableName'
-                if (commandElements.Count == 2 && commandElements[1] is StringConstantExpressionAst constantExpressionAst)
+                // The following extracts the variable name(s) only in the simplest possible usage of Get-Variable.
+                // Usage of a named parameter and an array of variables is accounted for though.
+                if (commandElements.Count < 2 || commandElements.Count > 3) { continue; }
+
+                var commandElementAstOfVariableName = commandElements[commandElements.Count - 1];
+                if (commandElements.Count == 3)
                 {
-                    var variableName = constantExpressionAst.Value;
-                    if (assignmentsDictionary_OrdinalIgnoreCase.ContainsKey(variableName))
+                    if (!(commandElements[1] is CommandParameterAst commandParameterAst)) { continue; }
+                    // Check if the named parameter -Name is used (PowerShell does not need the full
+                    // parameter name and there is no other parameter of Get-Variable starting with n).
+                    if (!commandParameterAst.ParameterName.StartsWith("n", StringComparison.OrdinalIgnoreCase))
                     {
-                        assignmentsDictionary_OrdinalIgnoreCase.Remove(variableName);
+                        continue;
                     }
                 }
-            }
 
-            foreach (string key in assignmentsDictionary_OrdinalIgnoreCase.Keys)
-            {
-                yield return new DiagnosticRecord(
-                    string.Format(CultureInfo.CurrentCulture, Strings.UseDeclaredVarsMoreThanAssignmentsError, key),
-                    assignmentsDictionary_OrdinalIgnoreCase[key].Left.Extent,
-                    GetName(),
-                    DiagnosticSeverity.Warning,
-                    fileName,
-                    key);
+                if (commandElementAstOfVariableName is StringConstantExpressionAst constantExpressionAst)
+                {
+                    assignmentsDictionary_OrdinalIgnoreCase.Remove(constantExpressionAst.Value);
+                    continue;
+                }
+
+                if (!(commandElementAstOfVariableName is ArrayLiteralAst arrayLiteralAst)) { continue; }
+                foreach (var expressionAst in arrayLiteralAst.Elements)
+                {
+                    if (expressionAst is StringConstantExpressionAst constantExpressionAstOfArray)
+                    {
+                        assignmentsDictionary_OrdinalIgnoreCase.Remove(constantExpressionAstOfArray.Value);
+                    }
+                }
             }
         }
     }

Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1

@@ -72,7 +72,7 @@ function MyFunc2() {
         It "returns no violations" {
             $noViolations.Count | Should -Be 0
         }
-        
+
         It "Does not flag += operator" {
             $results = Invoke-ScriptAnalyzer -ScriptDefinition '$array=@(); $list | ForEach-Object { $array += $c }' | Where-Object { $_.RuleName -eq $violationName }
             $results.Count | Should -Be 0
@@ -88,9 +88,41 @@ function MyFunc2() {
             $results.Count | Should -Be 0
         }
 
-        It "Using a variable via 'Get-Variable' does not trigger a warning" {
-            $noViolations = Invoke-ScriptAnalyzer -ScriptDefinition '$a=4; get-variable a'
-            $noViolations.Count | Should -Be 0
+        It "No warning when using 'Get-Variable' with variables declaration '<DeclareVariables>' and command parameter <GetVariableCommandParameter>" -TestCases @(
+            @{
+                DeclareVariables = '$a = 1'; GetVariableCommandParameter = 'a';
+            }
+            @{
+                DeclareVariables = '$a = 1'; GetVariableCommandParameter = '-Name a';
+            }
+            @{
+                DeclareVariables = '$a = 1'; GetVariableCommandParameter = '-n a';
+            }
+            @{
+                DeclareVariables = '$a = 1; $b = 2'; GetVariableCommandParameter = 'a,b'
+            }
+            @{
+                DeclareVariables = '$a = 1; $b = 2'; GetVariableCommandParameter = '-Name a,b'
+            }
+            @{
+                DeclareVariables = '$a = 1; $b = 2'; GetVariableCommandParameter = '-n a,b'
+            }
+            @{
+                DeclareVariables = '$a = 1; $b = 2'; GetVariableCommandParameter = 'A,B'
+            }
+        ) {
+            Param(
+                $DeclareVariables,
+                $GetVariableCommandParameter
+            )
+            $scriptDefinition = "$DeclareVariables; Get-Variable $GetVariableCommandParameter"
+            $noViolations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition
+            $noViolations.Count | Should -Be 0 -Because $scriptDefinition
+        }
+
+        It "Does not misinterpret switch parameter of Get-Variable as variable" {
+            $scriptDefinition = '$ArbitrarySwitchParameter = 1; Get-Variable -ArbitrarySwitchParameter'
+            (Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition).Count | Should -Be 1 -Because $scriptDefinition
         }
     }
 }