Skip to content

Commit dc59597

Browse files
author
Quoc Truong
committed
Merge pull request #247 from PowerShell/ImprovePerformanceScriptAnalyzer
Improve performance script analyzer
2 parents 113f7c8 + 295f9a3 commit dc59597

16 files changed

+127
-58
lines changed

‎Engine/Commands/InvokeScriptAnalyzerCommand.cs

Lines changed: 108 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
using System.Text.RegularExpressions;
1414
using System;
15+
using System.ComponentModel;
1516
using System.Collections.Generic;
1617
using System.Diagnostics.CodeAnalysis;
1718
using System.Globalization;
@@ -20,6 +21,9 @@
2021
using System.Management.Automation.Language;
2122
using System.IO;
2223
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
24+
using System.Threading.Tasks;
25+
using System.Collections.Concurrent;
26+
using System.Threading;
2327

2428
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands
2529
{
@@ -267,6 +271,13 @@ private void ProcessPath(string path)
267271

268272
}
269273

274+
ConcurrentBag<DiagnosticRecord> diagnostics;
275+
ConcurrentBag<SuppressedRecord> suppressed;
276+
Dictionary<string, List<RuleSuppression>> ruleSuppressions;
277+
List<Regex> includeRegexList;
278+
List<Regex> excludeRegexList;
279+
ConcurrentDictionary<string, List<object>> ruleDictionary;
280+
270281
/// <summary>
271282
/// Analyzes a single script file.
272283
/// </summary>
@@ -275,15 +286,16 @@ private void AnalyzeFile(string filePath)
275286
{
276287
Token[] tokens = null;
277288
ParseError[] errors = null;
278-
List<DiagnosticRecord> diagnostics = new List<DiagnosticRecord>();
279-
List<SuppressedRecord> suppressed = new List<SuppressedRecord>();
289+
ConcurrentBag<DiagnosticRecord> diagnostics = new ConcurrentBag<DiagnosticRecord>();
290+
ConcurrentBag<SuppressedRecord> suppressed = new ConcurrentBag<SuppressedRecord>();
291+
BlockingCollection<List<object>> verboseOrErrors = new BlockingCollection<List<object>>();
280292

281293
// Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash
282294
List<KeyValuePair<CommandInfo, IScriptExtent>> cmdInfoTable = new List<KeyValuePair<CommandInfo, IScriptExtent>>();
283295

284296
//Check wild card input for the Include/ExcludeRules and create regex match patterns
285-
List<Regex> includeRegexList = new List<Regex>();
286-
List<Regex> excludeRegexList = new List<Regex>();
297+
includeRegexList = new List<Regex>();
298+
excludeRegexList = new List<Regex>();
287299
if (includeRule != null)
288300
{
289301
foreach (string rule in includeRule)
@@ -331,7 +343,7 @@ private void AnalyzeFile(string filePath)
331343
return;
332344
}
333345

334-
Dictionary<string, List<RuleSuppression>> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
346+
ruleSuppressions = Helper.Instance.GetRuleSuppression(ast);
335347

336348
foreach (List<RuleSuppression> ruleSuppressionsList in ruleSuppressions.Values)
337349
{
@@ -360,43 +372,75 @@ private void AnalyzeFile(string filePath)
360372

361373
if (ScriptAnalyzer.Instance.ScriptRules != null)
362374
{
363-
foreach (IScriptRule scriptRule in ScriptAnalyzer.Instance.ScriptRules)
364-
{
365-
bool includeRegexMatch = false;
366-
bool excludeRegexMatch = false;
367-
foreach (Regex include in includeRegexList)
375+
var tasks = ScriptAnalyzer.Instance.ScriptRules.Select(scriptRule => Task.Factory.StartNew(() =>
368376
{
369-
if (include.IsMatch(scriptRule.GetName()))
377+
bool includeRegexMatch = false;
378+
bool excludeRegexMatch = false;
379+
380+
foreach (Regex include in includeRegexList)
370381
{
371-
includeRegexMatch = true;
372-
break;
382+
if (include.IsMatch(scriptRule.GetName()))
383+
{
384+
includeRegexMatch = true;
385+
break;
386+
}
373387
}
374-
}
375388

376-
foreach (Regex exclude in excludeRegexList)
377-
{
378-
if (exclude.IsMatch(scriptRule.GetName()))
389+
foreach (Regex exclude in excludeRegexList)
379390
{
380-
excludeRegexMatch = true;
381-
break;
391+
if (exclude.IsMatch(scriptRule.GetName()))
392+
{
393+
excludeRegexMatch = true;
394+
break;
395+
}
382396
}
383-
}
384-
385-
if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
386-
{
387-
WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName()));
388397

389-
// Ensure that any unhandled errors from Rules are converted to non-terminating errors
390-
// We want the Engine to continue functioning even if one or more Rules throws an exception
391-
try
398+
if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
392399
{
393-
var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList());
394-
diagnostics.AddRange(records.Item2);
395-
suppressed.AddRange(records.Item1);
400+
List<object> result = new List<object>();
401+
402+
result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName()));
403+
404+
// Ensure that any unhandled errors from Rules are converted to non-terminating errors
405+
// We want the Engine to continue functioning even if one or more Rules throws an exception
406+
try
407+
{
408+
var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList());
409+
foreach (var record in records.Item2)
410+
{
411+
diagnostics.Add(record);
412+
}
413+
foreach (var suppressedRec in records.Item1)
414+
{
415+
suppressed.Add(suppressedRec);
416+
}
417+
}
418+
catch (Exception scriptRuleException)
419+
{
420+
result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File));
421+
}
422+
423+
verboseOrErrors.Add(result);
396424
}
397-
catch (Exception scriptRuleException)
425+
}));
426+
427+
Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding());
428+
429+
while (!verboseOrErrors.IsCompleted)
430+
{
431+
List<object> data = null;
432+
try
433+
{
434+
data = verboseOrErrors.Take();
435+
}
436+
catch (InvalidOperationException) { }
437+
438+
if (data != null)
439+
{
440+
WriteVerbose(data[0] as string);
441+
if (data.Count == 2)
398442
{
399-
WriteError(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
443+
WriteError(data[1] as ErrorRecord);
400444
}
401445
}
402446
}
@@ -437,8 +481,14 @@ private void AnalyzeFile(string filePath)
437481
try
438482
{
439483
var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList());
440-
diagnostics.AddRange(records.Item2);
441-
suppressed.AddRange(records.Item1);
484+
foreach (var record in records.Item2)
485+
{
486+
diagnostics.Add(record);
487+
}
488+
foreach (var suppressedRec in records.Item1)
489+
{
490+
suppressed.Add(suppressedRec);
491+
}
442492
}
443493
catch (Exception tokenRuleException)
444494
{
@@ -489,8 +539,14 @@ private void AnalyzeFile(string filePath)
489539
try
490540
{
491541
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList());
492-
diagnostics.AddRange(records.Item2);
493-
suppressed.AddRange(records.Item1);
542+
foreach (var record in records.Item2)
543+
{
544+
diagnostics.Add(record);
545+
}
546+
foreach (var suppressedRec in records.Item1)
547+
{
548+
suppressed.Add(suppressedRec);
549+
}
494550
}
495551
catch (Exception dscResourceRuleException)
496552
{
@@ -532,8 +588,14 @@ private void AnalyzeFile(string filePath)
532588
try
533589
{
534590
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
535-
diagnostics.AddRange(records.Item2);
536-
suppressed.AddRange(records.Item1);
591+
foreach (var record in records.Item2)
592+
{
593+
diagnostics.Add(record);
594+
}
595+
foreach (var suppressedRec in records.Item1)
596+
{
597+
suppressed.Add(suppressedRec);
598+
}
537599
}
538600
catch (Exception dscResourceRuleException)
539601
{
@@ -573,15 +635,20 @@ private void AnalyzeFile(string filePath)
573635
}
574636
}
575637

576-
diagnostics.AddRange(ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName));
638+
foreach (var record in ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName))
639+
{
640+
diagnostics.Add(record);
641+
}
577642
}
578643

579644
#endregion
580645

646+
IEnumerable<DiagnosticRecord> diagnosticsList = diagnostics;
647+
581648
if (severity != null)
582649
{
583650
var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true));
584-
diagnostics = diagnostics.Where(item => diagSeverity.Contains(item.Severity)).ToList();
651+
diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity));
585652
}
586653

587654
//Output through loggers
@@ -596,7 +663,7 @@ private void AnalyzeFile(string filePath)
596663
}
597664
else
598665
{
599-
foreach (DiagnosticRecord diagnostic in diagnostics)
666+
foreach (DiagnosticRecord diagnostic in diagnosticsList)
600667
{
601668
logger.LogObject(diagnostic, this);
602669
}

‎Rules/AvoidUsingDeprecatedManifestFields.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
4141

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

76+
ps.Dispose();
7677
}
7778

7879
}

‎Rules/MissingModuleManifestField.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
3838

3939
if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase))
4040
{
41-
var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace);
41+
var ps = System.Management.Automation.PowerShell.Create();
4242

4343
try
4444
{
@@ -68,6 +68,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
6868
}
6969
}
7070

71+
ps.Dispose();
7172
}
7273

7374
}

‎Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22
$globalMessage = "Found global variable 'Global:1'."
33
$globalName = "PSAvoidGlobalVars"
44
$nonInitializedName = "PSAvoidUninitializedVariable"
5-
$nonInitializedMessage = "Variable 'a' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
5+
$nonInitializedMessage = "Variable 'globalVars' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
66
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
77
$violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVars.ps1
88
$dscResourceViolations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAny\MSFT_WaitForAny.psm1 | Where-Object {$_.RuleName -eq $nonInitializedName}
99
$globalViolations = $violations | Where-Object {$_.RuleName -eq $globalName}
1010
$nonInitializedViolations = $violations | Where-Object {$_.RuleName -eq $nonInitializedName}
1111
$noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVarsNoViolations.ps1
12-
$noGlobalViolations = $noViolations | Where-Object {$_.RuleName -eq $violationName}
12+
$noGlobalViolations = $noViolations | Where-Object {$_.RuleName -eq $globalName}
1313
$noUninitializedViolations = $noViolations | Where-Object {$_.RuleName -eq $nonInitializedName}
1414

1515
Describe "AvoidGlobalVars" {
@@ -23,7 +23,7 @@ Describe "AvoidGlobalVars" {
2323
}
2424

2525
It "has the correct description message" {
26-
$violations[0].Message | Should Match $globalMessage
26+
$globalViolations[0].Message | Should Match $globalMessage
2727
}
2828
}
2929

‎Tests/Rules/AvoidPositionalParameters.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Import-Module PSScriptAnalyzer
2-
$violationMessage = "Cmdlet 'Get-Content' has positional parameter. Please use named parameters instead of positional parameters when calling a command."
2+
$violationMessage = "Cmdlet 'Write-Host' has positional parameter. Please use named parameters instead of positional parameters when calling a command."
33
$violationName = "PSAvoidUsingPositionalParameters"
44
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
55
$violations = Invoke-ScriptAnalyzer $directory\AvoidPositionalParameters.ps1 | Where-Object {$_.RuleName -eq $violationName}

‎Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Import-Module PSScriptAnalyzer
2-
$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"
2+
$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"
33
$violationName = "PSAvoidShouldContinueWithoutForce"
44
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
55
$violations = Invoke-ScriptAnalyzer $directory\AvoidShouldContinueWithoutForce.ps1 | Where-Object {$_.RuleName -eq $violationName}

‎Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Import-Module PSScriptAnalyzer
22

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

‎Tests/Rules/AvoidUsingAlias.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Import-Module PSScriptAnalyzer
2-
$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."
2+
$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."
33
$violationName = "PSAvoidUsingCmdletAliases"
44
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
55
$violations = Invoke-ScriptAnalyzer $directory\AvoidUsingAlias.ps1 | Where-Object {$_.RuleName -eq $violationName}

‎Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Import-Module PSScriptAnalyzer
22

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

‎Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Import-Module PSScriptAnalyzer
22
$AvoidUninitializedVariable = "PSAvoidUninitializedVariable"
3-
$violationMessage = "Variable 'MyProgressPreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
3+
$violationMessage = "Variable 'MyVerbosePreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
44
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
55
$violations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariable.ps1 -IncludeRule $AvoidUninitializedVariable
66
$noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariableNoViolations.ps1 -IncludeRule $AvoidUninitializedVariable

‎Tests/Rules/ProvideCommentHelp.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Import-Module PSScriptAnalyzer
2-
$violationMessage = "The cmdlet 'Verb-Files' does not have a help comment."
2+
$violationMessage = "The cmdlet 'Comment' does not have a help comment."
33
$violationName = "PSProvideCommentHelp"
44
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
55
$violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName}

0 commit comments

Comments
 (0)