Fix test failures and use tasks instead of backgroundworkers

Author: Quoc Truong

Engine/Commands/InvokeScriptAnalyzerCommand.cs

@@ -271,14 +271,6 @@ private void ProcessPath(string path)
 
         }
 
-        ConcurrentBag<DiagnosticRecord> diagnostics;
-        ConcurrentBag<SuppressedRecord> suppressed;
-        Dictionary<string, List<RuleSuppression>> ruleSuppressions;
-        List<Regex> includeRegexList;
-        List<Regex> excludeRegexList;
-        CountdownEvent cde;
-        ConcurrentDictionary<string, List<object>> ruleDictionary;
-
         /// <summary>
         /// Analyzes a single script file.
         /// </summary>
@@ -287,16 +279,16 @@ private void AnalyzeFile(string filePath)
         {
             Token[] tokens = null;
             ParseError[] errors = null;
-            diagnostics = new ConcurrentBag<DiagnosticRecord>();
-            suppressed = new ConcurrentBag<SuppressedRecord>();
-            ruleDictionary = new ConcurrentDictionary<string, List<object>>();
+            ConcurrentBag<DiagnosticRecord> diagnostics = new ConcurrentBag<DiagnosticRecord>();
+            ConcurrentBag<SuppressedRecord> suppressed = new ConcurrentBag<SuppressedRecord>();
+            BlockingCollection<List<object>> verboseOrErrors = new BlockingCollection<List<object>>();
 
             // Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash
             List<KeyValuePair<CommandInfo, IScriptExtent>> cmdInfoTable = new List<KeyValuePair<CommandInfo, IScriptExtent>>();
 
             //Check wild card input for the Include/ExcludeRules and create regex match patterns
-            includeRegexList = new List<Regex>();
-            excludeRegexList = new List<Regex>();
+            List<Regex> includeRegexList = new List<Regex>();
+            List<Regex> excludeRegexList = new List<Regex>();
             if (includeRule != null)
             {
                 foreach (string rule in includeRule)
@@ -344,7 +336,7 @@ private void AnalyzeFile(string filePath)
                 return;
             }
 
-            ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
+            Dictionary<string, List<RuleSuppression>> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
 
             foreach (List<RuleSuppression> ruleSuppressionsList in ruleSuppressions.Values)
             {
@@ -373,24 +365,77 @@ private void AnalyzeFile(string filePath)
 
             if (ScriptAnalyzer.Instance.ScriptRules != null)
             {
-                cde = new CountdownEvent(ScriptAnalyzer.Instance.ScriptRules.Count());
+                var tasks = ScriptAnalyzer.Instance.ScriptRules.Select(scriptRule => Task.Factory.StartNew(() =>
+                    {
+                        bool includeRegexMatch = false;
+                        bool excludeRegexMatch = false;
 
-                foreach (var scriptRule in ScriptAnalyzer.Instance.ScriptRules)
-                {
-                    BackgroundWorker bg = new BackgroundWorker();
-                    bg.DoWork += bg_DoWork;
-                    bg.RunWorkerAsync(new object[] { scriptRule });
-                }
+                        foreach (Regex include in includeRegexList)
+                        {
+                            if (include.IsMatch(scriptRule.GetName()))
+                            {
+                                includeRegexMatch = true;
+                                break;
+                            }
+                        }
+
+                        foreach (Regex exclude in excludeRegexList)
+                        {
+                            if (exclude.IsMatch(scriptRule.GetName()))
+                            {
+                                excludeRegexMatch = true;
+                                break;
+                            }
+                        }
+
+                        if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
+                        {
+                            List<object> result = new List<object>();
+
+                            //WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName()));
+                            result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName()));
+
+                            // Ensure that any unhandled errors from Rules are converted to non-terminating errors
+                            // We want the Engine to continue functioning even if one or more Rules throws an exception
+                            try
+                            {
+                                var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList());
+                                foreach (var record in records.Item2)
+                                {
+                                    diagnostics.Add(record);
+                                }
+                                foreach (var suppressedRec in records.Item1)
+                                {
+                                    suppressed.Add(suppressedRec);
+                                }
+                            }
+                            catch (Exception scriptRuleException)
+                            {
+                                result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File));
+                            }
 
-                cde.Wait();
+                            verboseOrErrors.Add(result);
+                        }
+                }));
 
-                foreach (var rule in ruleDictionary.Keys)
+                Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding());
+
+                while (!verboseOrErrors.IsCompleted)
                 {
-                    List<object> verboseOrErrors = ruleDictionary[rule];
-                    WriteVerbose(verboseOrErrors[0] as string);
-                    if (verboseOrErrors.Count == 2)
+                    List<object> data = null;
+                    try
+                    {
+                        data = verboseOrErrors.Take();
+                    }
+                    catch (InvalidOperationException) { }
+
+                    if (data != null)
                     {
-                        WriteError(verboseOrErrors[1] as ErrorRecord);
+                        WriteVerbose(data[0] as string);
+                        if (data.Count == 2)
+                        {
+                            WriteError(data[1] as ErrorRecord);
+                        }
                     }
                 }
             }
@@ -620,65 +665,6 @@ private void AnalyzeFile(string filePath)
             }
         }
 
-        void bg_DoWork(object sender, DoWorkEventArgs e)
-        {
-            bool includeRegexMatch = false;
-            bool excludeRegexMatch = false;
-
-            object[] parameters = e.Argument as object[];
-
-            IScriptRule scriptRule = parameters[0] as IScriptRule;
-
-            foreach (Regex include in includeRegexList)
-            {
-                if (include.IsMatch(scriptRule.GetName()))
-                {
-                    includeRegexMatch = true;
-                    break;
-                }
-            }
-
-            foreach (Regex exclude in excludeRegexList)
-            {
-                if (exclude.IsMatch(scriptRule.GetName()))
-                {
-                    excludeRegexMatch = true;
-                    break;
-                }
-            }
-
-            List<object> result = new List<object>();
-
-            if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
-            {
-                //WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName()));
-                result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName()));
-
-                // Ensure that any unhandled errors from Rules are converted to non-terminating errors
-                // We want the Engine to continue functioning even if one or more Rules throws an exception
-                try
-                {
-                    var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList());
-                    foreach (var record in records.Item2)
-                    {
-                        diagnostics.Add(record);
-                    }
-                    foreach (var suppressedRec in records.Item1)
-                    {
-                        suppressed.Add(suppressedRec);
-                    }
-                }
-                catch (Exception scriptRuleException)
-                {
-                    result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File));
-                }
-            }
-
-            ruleDictionary[scriptRule.GetName()] = result;
-
-            cde.Signal();
-        }
-
         #endregion
     }
 }

Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1

@@ -2,14 +2,14 @@
 $globalMessage = "Found global variable 'Global:1'."
 $globalName = "PSAvoidGlobalVars"
 $nonInitializedName = "PSAvoidUninitializedVariable"
-$nonInitializedMessage = "Variable 'a' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
+$nonInitializedMessage = "Variable 'globalVars' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVars.ps1
 $dscResourceViolations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAny\MSFT_WaitForAny.psm1 | Where-Object {$_.RuleName -eq $nonInitializedName}
 $globalViolations = $violations | Where-Object {$_.RuleName -eq $globalName}
 $nonInitializedViolations = $violations | Where-Object {$_.RuleName -eq $nonInitializedName}
 $noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVarsNoViolations.ps1
-$noGlobalViolations = $noViolations | Where-Object {$_.RuleName -eq $violationName}
+$noGlobalViolations = $noViolations | Where-Object {$_.RuleName -eq $globalName}
 $noUninitializedViolations = $noViolations | Where-Object {$_.RuleName -eq $nonInitializedName}
 
 Describe "AvoidGlobalVars" {
@@ -23,7 +23,7 @@ Describe "AvoidGlobalVars" {
         }
 
         It "has the correct description message" {
-            $violations[0].Message | Should Match $globalMessage
+            $globalViolations[0].Message | Should Match $globalMessage
         }
     }
 

Tests/Rules/AvoidPositionalParameters.tests.ps1

@@ -1,5 +1,5 @@
 Import-Module PSScriptAnalyzer
-$violationMessage = "Cmdlet 'Get-Content' has positional parameter. Please use named parameters instead of positional parameters when calling a command."
+$violationMessage = "Cmdlet 'Write-Host' 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}

Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1

@@ -1,5 +1,5 @@
 Import-Module PSScriptAnalyzer
-$violationMessage = "Function 'Verb-Noun' in file 'AvoidShouldContinueWithoutForce.ps1' uses ShouldContinue but does not have a boolean force parameter. The force parameter will allow users of the script to bypass ShouldContinue prompt"
+$violationMessage = "Function 'Verb-Noun2' in file 'AvoidShouldContinueWithoutForce.ps1' uses ShouldContinue but does not have a boolean force parameter. The force parameter will allow users of the script to bypass ShouldContinue prompt"
 $violationName = "PSAvoidShouldContinueWithoutForce"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidShouldContinueWithoutForce.ps1 | Where-Object {$_.RuleName -eq $violationName}

Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1

@@ -1,6 +1,6 @@
 Import-Module PSScriptAnalyzer
 
-$violationMessage = "Function 'TestFunction' has both username and password parameters. A credential parameter of type PSCredential should be used."
+$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential should be used."
 $violationName = "PSAvoidUsingUserNameAndPasswordParams"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName}

Tests/Rules/AvoidUsingAlias.tests.ps1

@@ -1,5 +1,5 @@
 Import-Module PSScriptAnalyzer
-$violationMessage = "'iex' is an alias of 'Invoke-Expression'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content."
+$violationMessage = "'cls' is an alias of 'Clear-Host'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content."
 $violationName = "PSAvoidUsingCmdletAliases"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingAlias.ps1 | Where-Object {$_.RuleName -eq $violationName}

Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1

@@ -1,6 +1,6 @@
 Import-Module PSScriptAnalyzer
 
-$violationMessage = [regex]::Escape("Parameter '`$passphrases' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.")
+$violationMessage = [regex]::Escape("Parameter '`$password' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.")
 $violationName = "PSAvoidUsingPlainTextForPassword"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingPlainTextForPassword.ps1 | Where-Object {$_.RuleName -eq $violationName}

Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1

@@ -1,6 +1,6 @@
 Import-Module PSScriptAnalyzer
 $AvoidUninitializedVariable = "PSAvoidUninitializedVariable"
-$violationMessage = "Variable 'MyProgressPreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
+$violationMessage = "Variable 'MyVerbosePreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariable.ps1 -IncludeRule $AvoidUninitializedVariable
 $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariableNoViolations.ps1 -IncludeRule $AvoidUninitializedVariable

Tests/Rules/ProvideCommentHelp.tests.ps1

@@ -1,5 +1,5 @@
 Import-Module PSScriptAnalyzer
-$violationMessage = "The cmdlet 'Verb-Files' does not have a help comment."
+$violationMessage = "The cmdlet 'Comment' does not have a help comment."
 $violationName = "PSProvideCommentHelp"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName}

Tests/Rules/ProvideDefaultParameterValue.tests.ps1

@@ -1,6 +1,6 @@
 Import-Module PSScriptAnalyzer
 $violationName = "PSProvideDefaultParameterValue"
-$violationMessage = "Parameter 'Param2' is not initialized. Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters"
+$violationMessage = "Parameter 'Param1' is not initialized. Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValue.ps1 | Where-Object {$_.RuleName -match $violationName}
 $noViolations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValueNoViolations.ps1

Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1

@@ -1,7 +1,7 @@
 Import-Module -Verbose PSScriptAnalyzer
 
 $violationMessageDSCResource = "Test-TargetResource function in DSC Resource should return object of type System.Boolean instead of System.Collections.Hashtable"
-$violationMessageDSCClass = "Test function in DSC Class FileResource should return object of type System.Boolean instead of type System.Int32"
+$violationMessageDSCClass = "Get function in DSC Class FileResource should return object of type FileResource instead of type System.Collections.Hashtable"
 $violationName = "PSDSCReturnCorrectTypesForDSCFunctions"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAll\MSFT_WaitForAll.psm1 | Where-Object {$_.RuleName -eq $violationName}

Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1

@@ -1,5 +1,5 @@
 Import-Module PSScriptAnalyzer
-$violationMessage = "The variable 'declaredVar' is assigned but never used."
+$violationMessage = "The variable 'declaredVar2' is assigned but never used."
 $violationName = "PSUseDeclaredVarsMoreThanAssigments"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\UseDeclaredVarsMoreThanAssignments.ps1 | Where-Object {$_.RuleName -eq $violationName}

Tests/Rules/UseOutputTypeCorrectly.tests.ps1

@@ -1,5 +1,5 @@
 Import-Module PSScriptAnalyzer
-$violationMessage = "The cmdlet 'Verb-Files' returns an object of type 'System.Double' but this type is not declared in the OutputType attribute."
+$violationMessage = "The cmdlet 'Verb-Files' returns an object of type 'System.Collections.Hashtable' but this type is not declared in the OutputType attribute."
 $violationName = "PSUseOutputTypeCorrectly"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName}

Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1

@@ -1,6 +1,6 @@
 Import-Module PSScriptAnalyzer
 
-$violationMessage = "There is no call to Write-Verbose in DSC function ‘Set-TargetResource’. If you are using Write-Verbose in a helper function, suppress this rule application."
+$violationMessage = "There is no call to Write-Verbose in DSC function ‘Test-TargetResource’. If you are using Write-Verbose in a helper function, suppress this rule application."
 $violationName = "PSDSCUseVerboseMessageInDSCResource"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAll\MSFT_WaitForAll.psm1 | Where-Object {$_.RuleName -eq $violationName}