Skip to content

Commit 0805538

Browse files
raghushantharaghushantha
committed
Merge pull request #248 from PowerShell/BugFixes
Merge BugFixes to Master
2 parents 967e956 + dc59597 commit 0805538

21 files changed

+147
-79
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
}

‎Engine/Helper.cs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
249249
CommandInfo commandInfo = GetCommandInfo(GetCmdletNameFromAlias(cmdAst.GetCommandName())) ?? GetCommandInfo(cmdAst.GetCommandName());
250250

251251
IEnumerable<ParameterMetadata> switchParams = null;
252-
IEnumerable<CommandParameterSetInfo> scriptBlocks = null;
253-
bool hasScriptBlockSet = false;
254252

255253
if (HasSplattedVariable(cmdAst))
256254
{
@@ -262,15 +260,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
262260
try
263261
{
264262
switchParams = commandInfo.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter);
265-
scriptBlocks = commandInfo.ParameterSets;
266-
foreach (CommandParameterSetInfo cmdParaset in scriptBlocks)
267-
{
268-
if (String.Equals(cmdParaset.Name, "ScriptBlockSet", StringComparison.OrdinalIgnoreCase))
269-
{
270-
hasScriptBlockSet = true;
271-
}
272-
}
273-
274263
}
275264
catch (Exception)
276265
{
@@ -284,8 +273,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
284273

285274
foreach (CommandElementAst ceAst in cmdAst.CommandElements)
286275
{
287-
if (!hasScriptBlockSet)
288-
{
289276
if (ceAst is CommandParameterAst)
290277
{
291278
// Skip if it's a switch parameter
@@ -308,7 +295,7 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
308295
{
309296
arguments += 1;
310297
}
311-
}
298+
312299
}
313300

314301
// if not the first element in a pipeline, increase the number of arguments by 1

‎Rules/AvoidDefaultTrueValueSwitchParameter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
3939
// Iterrates all ParamAsts and check if any are switch.
4040
foreach (ParameterAst paramAst in paramAsts)
4141
{
42-
if (paramAst.Attributes.Any(attr => string.Equals(attr.TypeName.GetReflectionType().FullName, "system.management.automation.switchparameter", StringComparison.OrdinalIgnoreCase))
42+
if (paramAst.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(System.Management.Automation.SwitchParameter))
4343
&& paramAst.DefaultValue != null && String.Equals(paramAst.DefaultValue.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase))
4444
{
4545
yield return new DiagnosticRecord(

‎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
}

‎Rules/UseOutputTypeCorrectly.cs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,20 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
102102

103103
List<Tuple<string, StatementAst>> returnTypes = FindPipelineOutput.OutputTypes(funcAst, _classes);
104104

105+
HashSet<string> specialTypes = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
106+
specialTypes.Add(typeof(Unreached).FullName);
107+
specialTypes.Add(typeof(Undetermined).FullName);
108+
specialTypes.Add(typeof(object).FullName);
109+
specialTypes.Add(typeof(void).FullName);
110+
specialTypes.Add(typeof(PSCustomObject).FullName);
111+
specialTypes.Add(typeof(PSObject).FullName);
112+
105113
foreach (Tuple<string, StatementAst> returnType in returnTypes)
106114
{
107115
string typeName = returnType.Item1;
108116

109117
if (String.IsNullOrEmpty(typeName)
110-
|| String.Equals(typeof(Unreached).FullName, typeName, StringComparison.OrdinalIgnoreCase)
111-
|| String.Equals(typeof(Undetermined).FullName, typeName, StringComparison.OrdinalIgnoreCase)
112-
|| String.Equals(typeof(object).FullName, typeName, StringComparison.OrdinalIgnoreCase)
113-
|| String.Equals(typeof(void).FullName, typeName, StringComparison.OrdinalIgnoreCase)
118+
|| specialTypes.Contains(typeName)
114119
|| outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase))
115120
{
116121
continue;

‎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}

0 commit comments

Comments
 (0)