Skip to content

Commit 23481da

Browse files
author
Kapil Borle
committed
Merge pull request #497 from PowerShell/FixSuppressRuleWriteError
Fix invoking WriteError from rule execution threads
2 parents cfe5cd4 + 0df1245 commit 23481da

3 files changed

Lines changed: 77 additions & 21 deletions

File tree

‎Engine/Helper.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,12 +1198,16 @@ internal List<RuleSuppression> GetSuppressionsClass(TypeDefinitionAst typeAst)
11981198
/// </summary>
11991199
/// <param name="ruleSuppressions"></param>
12001200
/// <param name="diagnostics"></param>
1201-
public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(string ruleName, Dictionary<string, List<RuleSuppression>> ruleSuppressionsDict, List<DiagnosticRecord> diagnostics)
1201+
public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
1202+
string ruleName,
1203+
Dictionary<string, List<RuleSuppression>> ruleSuppressionsDict,
1204+
List<DiagnosticRecord> diagnostics,
1205+
out List<ErrorRecord> errorRecords)
12021206
{
12031207
List<SuppressedRecord> suppressedRecords = new List<SuppressedRecord>();
12041208
List<DiagnosticRecord> unSuppressedRecords = new List<DiagnosticRecord>();
12051209
Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> result = Tuple.Create(suppressedRecords, unSuppressedRecords);
1206-
1210+
errorRecords = new List<ErrorRecord>();
12071211
if (diagnostics == null || diagnostics.Count == 0)
12081212
{
12091213
return result;
@@ -1269,8 +1273,8 @@ public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(string
12691273
ruleSuppression.Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, ruleSuppression.StartAttributeLine,
12701274
System.IO.Path.GetFileName(diagnostics.First().Extent.File), String.Format(Strings.RuleSuppressionIDError, ruleSuppression.RuleSuppressionID));
12711275
}
1272-
1273-
this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression));
1276+
errorRecords.Add(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression));
1277+
//this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression));
12741278
}
12751279
}
12761280

‎Engine/ScriptAnalyzer.cs‎

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,6 +1385,31 @@ bool IsRuleAllowed(IRule rule)
13851385
&& IsSeverityAllowed(allowedSeverities, rule);
13861386
}
13871387

1388+
/// <summary>
1389+
/// Wrapper around the Helper.SuppressRule method
1390+
/// </summary>
1391+
/// <param name="ruleName"></param>
1392+
/// <param name="ruleSuppressions"></param>
1393+
/// <param name="ruleDiagnosticRecords"></param>
1394+
/// <returns>Returns a tuple of suppressed and diagnostic records</returns>
1395+
private Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
1396+
string ruleName,
1397+
Dictionary<string, List<RuleSuppression>> ruleSuppressions,
1398+
List<DiagnosticRecord> ruleDiagnosticRecords)
1399+
{
1400+
List<ErrorRecord> suppressRuleErrors;
1401+
var records = Helper.Instance.SuppressRule(
1402+
ruleName,
1403+
ruleSuppressions,
1404+
ruleDiagnosticRecords,
1405+
out suppressRuleErrors);
1406+
foreach (var error in suppressRuleErrors)
1407+
{
1408+
this.outputWriter.WriteError(error);
1409+
}
1410+
return records;
1411+
}
1412+
13881413
/// <summary>
13891414
/// Analyzes the syntax tree of a script file that has already been parsed.
13901415
/// </summary>
@@ -1445,7 +1470,6 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
14451470
if (this.ScriptRules != null)
14461471
{
14471472
var allowedRules = this.ScriptRules.Where(IsRuleAllowed);
1448-
14491473
if (allowedRules.Any())
14501474
{
14511475
var tasks = allowedRules.Select(scriptRule => Task.Factory.StartNew(() =>
@@ -1468,7 +1492,14 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
14681492
}
14691493
else if (!helpRule && !helpFile)
14701494
{
1471-
var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(scriptAst, scriptAst.Extent.File).ToList());
1495+
List<ErrorRecord> suppressRuleErrors;
1496+
var ruleRecords = scriptRule.AnalyzeScript(scriptAst, scriptAst.Extent.File).ToList();
1497+
var records = Helper.Instance.SuppressRule(
1498+
scriptRule.GetName(),
1499+
ruleSuppressions,
1500+
ruleRecords,
1501+
out suppressRuleErrors);
1502+
result.AddRange(suppressRuleErrors);
14721503
foreach (var record in records.Item2)
14731504
{
14741505
diagnostics.Add(record);
@@ -1486,9 +1517,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
14861517

14871518
verboseOrErrors.Add(result);
14881519
}));
1489-
14901520
Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding());
1491-
14921521
while (!verboseOrErrors.IsCompleted)
14931522
{
14941523
List<object> data = null;
@@ -1501,9 +1530,12 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
15011530
if (data != null)
15021531
{
15031532
this.outputWriter.WriteVerbose(data[0] as string);
1504-
if (data.Count == 2)
1533+
if (data.Count > 1)
15051534
{
1506-
this.outputWriter.WriteError(data[1] as ErrorRecord);
1535+
for (int count = 1; count < data.Count; count++)
1536+
{
1537+
this.outputWriter.WriteError(data[count] as ErrorRecord);
1538+
}
15071539
}
15081540
}
15091541
}
@@ -1526,7 +1558,8 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
15261558
// We want the Engine to continue functioning even if one or more Rules throws an exception
15271559
try
15281560
{
1529-
var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(scriptTokens, filePath).ToList());
1561+
var ruleRecords = tokenRule.AnalyzeTokens(scriptTokens, filePath).ToList();
1562+
var records = SuppressRule(tokenRule.GetName(), ruleSuppressions, ruleRecords);
15301563
foreach (var record in records.Item2)
15311564
{
15321565
diagnostics.Add(record);
@@ -1584,16 +1617,12 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
15841617
// We want the Engine to continue functioning even if one or more Rules throws an exception
15851618
try
15861619
{
1587-
#if PSV3
1588-
1620+
#if PSV3
15891621
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, null);
1590-
1591-
#else
1592-
1593-
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(scriptAst, filePath).ToList());
1594-
1595-
#endif
1596-
1622+
#else
1623+
var ruleRecords = dscResourceRule.AnalyzeDSCClass(scriptAst, filePath).ToList();
1624+
var records = SuppressRule(dscResourceRule.GetName(), ruleSuppressions, ruleRecords);
1625+
#endif
15971626
foreach (var record in records.Item2)
15981627
{
15991628
diagnostics.Add(record);
@@ -1625,7 +1654,8 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
16251654
// We want the Engine to continue functioning even if one or more Rules throws an exception
16261655
try
16271656
{
1628-
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(scriptAst, filePath).ToList());
1657+
var ruleRecords = dscResourceRule.AnalyzeDSCResource(scriptAst, filePath).ToList();
1658+
var records = SuppressRule(dscResourceRule.GetName(), ruleSuppressions, ruleRecords);
16291659
foreach (var record in records.Item2)
16301660
{
16311661
diagnostics.Add(record);

‎Tests/Engine/RuleSuppression.tests.ps1‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
1010
$violationsUsingScriptDefinition = Invoke-ScriptAnalyzer -ScriptDefinition (Get-Content -Raw "$directory\RuleSuppression.ps1")
1111
$violations = Invoke-ScriptAnalyzer "$directory\RuleSuppression.ps1"
1212

13+
$ruleSuppressionBad = @'
14+
Function do-something
15+
{
16+
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingUserNameAndPassWordParams", "username")]
17+
Param(
18+
$username,
19+
$password
20+
)
21+
}
22+
'@
23+
1324
Describe "RuleSuppressionWithoutScope" {
1425
Context "Function" {
1526
It "Does not raise violations" {
@@ -37,6 +48,17 @@ Describe "RuleSuppressionWithoutScope" {
3748
$suppression.Count | Should Be 1
3849
}
3950
}
51+
52+
if (!$testingLibraryUsage)
53+
{
54+
Context "Bad Rule Suppression" {
55+
It "Throws a non-terminating error" {
56+
Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionBad -IncludeRule "PSAvoidUsingUserNameAndPassWordParams" -ErrorVariable errorRecord -ErrorAction SilentlyContinue
57+
$errorRecord.Count | Should Be 1
58+
$errorRecord.FullyQualifiedErrorId | Should match "suppression message attribute error"
59+
}
60+
}
61+
}
4062
}
4163

4264
Describe "RuleSuppressionWithScope" {

0 commit comments

Comments
 (0)