Merge BugFixes to Master

Engine/Commands/InvokeScriptAnalyzerCommand.cs

@@ -12,6 +12,7 @@
 
 using System.Text.RegularExpressions;
 using System;
+using System.ComponentModel;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
@@ -20,6 +21,9 @@
 using System.Management.Automation.Language;
 using System.IO;
 using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+using System.Threading.Tasks;
+using System.Collections.Concurrent;
+using System.Threading;
 
 namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands
 {
@@ -267,6 +271,13 @@ private void ProcessPath(string path)
 
         }
 
+        ConcurrentBag<DiagnosticRecord> diagnostics;
+        ConcurrentBag<SuppressedRecord> suppressed;
+        Dictionary<string, List<RuleSuppression>> ruleSuppressions;
+        List<Regex> includeRegexList;
+        List<Regex> excludeRegexList;
+        ConcurrentDictionary<string, List<object>> ruleDictionary;
+
         /// <summary>
         /// Analyzes a single script file.
         /// </summary>
@@ -275,15 +286,16 @@ private void AnalyzeFile(string filePath)
         {
             Token[] tokens = null;
             ParseError[] errors = null;
-            List<DiagnosticRecord> diagnostics = new List<DiagnosticRecord>();
-            List<SuppressedRecord> suppressed = new List<SuppressedRecord>();
+            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
-            List<Regex> includeRegexList = new List<Regex>();
-            List<Regex> excludeRegexList = new List<Regex>();
+            includeRegexList = new List<Regex>();
+            excludeRegexList = new List<Regex>();
             if (includeRule != null)
             {
                 foreach (string rule in includeRule)
@@ -331,7 +343,7 @@ private void AnalyzeFile(string filePath)
                 return;
             }
 
-            Dictionary<string, List<RuleSuppression>> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
+            ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
 
             foreach (List<RuleSuppression> ruleSuppressionsList in ruleSuppressions.Values)
             {
@@ -360,43 +372,75 @@ private void AnalyzeFile(string filePath)
 
             if (ScriptAnalyzer.Instance.ScriptRules != null)
             {
-                foreach (IScriptRule scriptRule in ScriptAnalyzer.Instance.ScriptRules)
-                {
-                    bool includeRegexMatch = false;
-                    bool excludeRegexMatch = false;
-                    foreach (Regex include in includeRegexList)
+                var tasks = ScriptAnalyzer.Instance.ScriptRules.Select(scriptRule => Task.Factory.StartNew(() =>
                     {
-                        if (include.IsMatch(scriptRule.GetName()))
+                        bool includeRegexMatch = false;
+                        bool excludeRegexMatch = false;
+
+                        foreach (Regex include in includeRegexList)
                         {
-                            includeRegexMatch = true;
-                            break;
+                            if (include.IsMatch(scriptRule.GetName()))
+                            {
+                                includeRegexMatch = true;
+                                break;
+                            }
                         }
-                    }
 
-                    foreach (Regex exclude in excludeRegexList)
-                    {
-                        if (exclude.IsMatch(scriptRule.GetName()))
+                        foreach (Regex exclude in excludeRegexList)
                         {
-                            excludeRegexMatch = true;
-                            break;
+                            if (exclude.IsMatch(scriptRule.GetName()))
+                            {
+                                excludeRegexMatch = true;
+                                break;
+                            }
                         }
-                    }
-
-                    if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
-                    {
-                        WriteVerbose(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
+                        if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
                         {
-                            var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList());
-                            diagnostics.AddRange(records.Item2);
-                            suppressed.AddRange(records.Item1);
+                            List<object> result = new List<object>();
+
+                            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));
+                            }
+
+                            verboseOrErrors.Add(result);
                         }
-                        catch (Exception scriptRuleException)
+                }));
+
+                Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding());
+
+                while (!verboseOrErrors.IsCompleted)
+                {
+                    List<object> data = null;
+                    try
+                    {
+                        data = verboseOrErrors.Take();
+                    }
+                    catch (InvalidOperationException) { }
+
+                    if (data != null)
+                    {
+                        WriteVerbose(data[0] as string);
+                        if (data.Count == 2)
                         {
-                            WriteError(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
+                            WriteError(data[1] as ErrorRecord);
                         }
                     }
                 }
@@ -437,8 +481,14 @@ private void AnalyzeFile(string filePath)
                         try
                         {
                             var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList());
-                            diagnostics.AddRange(records.Item2);
-                            suppressed.AddRange(records.Item1);
+                            foreach (var record in records.Item2)
+                            {
+                                diagnostics.Add(record);
+                            }
+                            foreach (var suppressedRec in records.Item1)
+                            {
+                                suppressed.Add(suppressedRec);
+                            }
                         }
                         catch (Exception tokenRuleException)
                         {
@@ -489,8 +539,14 @@ private void AnalyzeFile(string filePath)
                             try
                             {
                                 var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList());
-                                diagnostics.AddRange(records.Item2);
-                                suppressed.AddRange(records.Item1);
+                                foreach (var record in records.Item2)
+                                {
+                                    diagnostics.Add(record);
+                                }
+                                foreach (var suppressedRec in records.Item1)
+                                {
+                                    suppressed.Add(suppressedRec);
+                                }
                             }
                             catch (Exception dscResourceRuleException)
                             {
@@ -532,8 +588,14 @@ private void AnalyzeFile(string filePath)
                             try
                             {
                                 var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
-                                diagnostics.AddRange(records.Item2);
-                                suppressed.AddRange(records.Item1);
+                                foreach (var record in records.Item2)
+                                {
+                                    diagnostics.Add(record);
+                                }
+                                foreach (var suppressedRec in records.Item1)
+                                {
+                                    suppressed.Add(suppressedRec);
+                                }
                             }
                             catch (Exception dscResourceRuleException)
                             {
@@ -573,15 +635,20 @@ private void AnalyzeFile(string filePath)
                     }
                 }
 
-                diagnostics.AddRange(ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName));
+                foreach (var record in ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName))
+                {
+                    diagnostics.Add(record);
+                }
             }
 
             #endregion
 
+            IEnumerable<DiagnosticRecord> diagnosticsList = diagnostics;
+
             if (severity != null)
             {
                 var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true));
-                diagnostics = diagnostics.Where(item => diagSeverity.Contains(item.Severity)).ToList();
+                diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity));
             }
 
             //Output through loggers
@@ -596,7 +663,7 @@ private void AnalyzeFile(string filePath)
                 }
                 else
                 {
-                    foreach (DiagnosticRecord diagnostic in diagnostics)
+                    foreach (DiagnosticRecord diagnostic in diagnosticsList)
                     {
                         logger.LogObject(diagnostic, this);
                     }

Engine/Helper.cs

@@ -249,8 +249,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
             CommandInfo commandInfo = GetCommandInfo(GetCmdletNameFromAlias(cmdAst.GetCommandName())) ?? GetCommandInfo(cmdAst.GetCommandName());
 
             IEnumerable<ParameterMetadata> switchParams = null;
-            IEnumerable<CommandParameterSetInfo> scriptBlocks = null;
-            bool hasScriptBlockSet = false;
 
             if (HasSplattedVariable(cmdAst))
             {
@@ -262,15 +260,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
                 try
                 {
                     switchParams = commandInfo.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter);
-                    scriptBlocks = commandInfo.ParameterSets;
-                    foreach (CommandParameterSetInfo cmdParaset in scriptBlocks)
-                    {
-                        if (String.Equals(cmdParaset.Name, "ScriptBlockSet", StringComparison.OrdinalIgnoreCase))
-                        {
-                            hasScriptBlockSet = true;
-                        }
-                    }
-
                 }
                 catch (Exception)
                 {
@@ -284,8 +273,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
 
             foreach (CommandElementAst ceAst in cmdAst.CommandElements)
             {
-                if (!hasScriptBlockSet)
-                {
                     if (ceAst is CommandParameterAst)
                     {
                         // Skip if it's a switch parameter
@@ -308,7 +295,7 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
                     {
                         arguments += 1;
                     }
-                }
+
             }
 
             // if not the first element in a pipeline, increase the number of arguments by 1

Rules/AvoidDefaultTrueValueSwitchParameter.cs

@@ -39,7 +39,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
             // Iterrates all ParamAsts and check if any are switch.
             foreach (ParameterAst paramAst in paramAsts)
             {
-                if (paramAst.Attributes.Any(attr => string.Equals(attr.TypeName.GetReflectionType().FullName, "system.management.automation.switchparameter", StringComparison.OrdinalIgnoreCase))
+                if (paramAst.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(System.Management.Automation.SwitchParameter))
                     && paramAst.DefaultValue != null && String.Equals(paramAst.DefaultValue.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase))
                 {
                     yield return new DiagnosticRecord(

Rules/AvoidUsingDeprecatedManifestFields.cs

@@ -41,7 +41,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
 
             if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase))
             {
-                var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace);
+                var ps = System.Management.Automation.PowerShell.Create();
                 IEnumerable<PSObject> result = null;
                 try
                 {
@@ -73,6 +73,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                     }
                 }
 
+                ps.Dispose();
             }
 
         }

Rules/MissingModuleManifestField.cs

@@ -38,7 +38,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
 
             if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase))
             {
-                var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace);
+                var ps = System.Management.Automation.PowerShell.Create();
 
                 try
                 {
@@ -68,6 +68,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                     }
                 }
 
+                ps.Dispose();
             }
 
         }

Rules/UseOutputTypeCorrectly.cs

@@ -102,15 +102,20 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
 
             List<Tuple<string, StatementAst>> returnTypes = FindPipelineOutput.OutputTypes(funcAst, _classes);
 
+            HashSet<string> specialTypes = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+            specialTypes.Add(typeof(Unreached).FullName);
+            specialTypes.Add(typeof(Undetermined).FullName);
+            specialTypes.Add(typeof(object).FullName);
+            specialTypes.Add(typeof(void).FullName);
+            specialTypes.Add(typeof(PSCustomObject).FullName);
+            specialTypes.Add(typeof(PSObject).FullName);
+
             foreach (Tuple<string, StatementAst> returnType in returnTypes)
             {
                 string typeName = returnType.Item1;
 
                 if (String.IsNullOrEmpty(typeName)
-                    || String.Equals(typeof(Unreached).FullName, typeName, StringComparison.OrdinalIgnoreCase)
-                    || String.Equals(typeof(Undetermined).FullName, typeName, StringComparison.OrdinalIgnoreCase)
-                    || String.Equals(typeof(object).FullName, typeName, StringComparison.OrdinalIgnoreCase)
-                    || String.Equals(typeof(void).FullName, typeName, StringComparison.OrdinalIgnoreCase)
+                    || specialTypes.Contains(typeName)
                     || outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase))
                 {
                     continue;

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}