From 05a275d18609684612b8a5fc7d32eb84691dff3f Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 10:05:57 -0700 Subject: [PATCH 01/16] Ensure multiply suppressed diagnostics only create one output --- Engine/Generic/SuppressedRecord.cs | 15 +- Engine/Helper.cs | 240 +++++++++++++------------ Tests/Engine/RuleSuppression.tests.ps1 | 29 ++- 3 files changed, 165 insertions(+), 119 deletions(-) diff --git a/Engine/Generic/SuppressedRecord.cs b/Engine/Generic/SuppressedRecord.cs index ee50ea48b..57ca1a24a 100644 --- a/Engine/Generic/SuppressedRecord.cs +++ b/Engine/Generic/SuppressedRecord.cs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Collections.Generic; +using System.Collections.ObjectModel; + namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic { /// @@ -9,22 +12,18 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic public class SuppressedRecord : DiagnosticRecord { /// - /// The rule suppression of this record + /// The rule suppressions applied to this record. /// - public RuleSuppression Suppression - { - get; - set; - } + public IReadOnlyList Suppression { get; set; } /// /// Creates a suppressed record based on a diagnostic record and the rule suppression /// /// /// - public SuppressedRecord(DiagnosticRecord record, RuleSuppression suppression) + public SuppressedRecord(DiagnosticRecord record, IReadOnlyList suppressions) { - Suppression = suppression; + Suppression = new ReadOnlyCollection(new List(suppressions)); if (record != null) { RuleName = record.RuleName; diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 743cb4d68..259b3799c 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -1323,156 +1323,176 @@ internal List GetSuppressionsConfiguration(ConfigurationDefinit /// Suppress the rules from the diagnostic records list. /// Returns a list of suppressed records as well as the ones that are not suppressed /// - /// - /// public Tuple, List> SuppressRule( string ruleName, Dictionary> ruleSuppressionsDict, List diagnostics, - out List errorRecords) + out List errors) { - List suppressedRecords = new List(); - List unSuppressedRecords = new List(); - Tuple, List> result = Tuple.Create(suppressedRecords, unSuppressedRecords); - errorRecords = new List(); - if (diagnostics == null || diagnostics.Count == 0) + var suppressedRecords = new List(); + var unsuppressedRecords = new List(); + errors = new List(); + var result = new Tuple, List>(suppressedRecords, unsuppressedRecords); + + if (diagnostics is null || diagnostics.Count == 0) { return result; } - if (ruleSuppressionsDict == null || !ruleSuppressionsDict.ContainsKey(ruleName) - || ruleSuppressionsDict[ruleName].Count == 0) + if (ruleSuppressionsDict is null + || !ruleSuppressionsDict.TryGetValue(ruleName, out List ruleSuppressions) + || ruleSuppressions.Count == 0) { - unSuppressedRecords.AddRange(diagnostics); + unsuppressedRecords.AddRange(diagnostics); return result; } - List ruleSuppressions = ruleSuppressionsDict[ruleName]; - var offsetArr = GetOffsetArray(diagnostics); - int recordIndex = 0; - int startRecord = 0; - bool[] suppressed = new bool[diagnostics.Count]; - foreach (RuleSuppression ruleSuppression in ruleSuppressions) + // We need to report errors for any rule suppressions with IDs that are not applied to anything + var unappliedSuppressions = new HashSet(); + foreach (RuleSuppression suppression in ruleSuppressions) { - int suppressionCount = 0; - while (startRecord < diagnostics.Count - // && diagnostics[startRecord].Extent.StartOffset < ruleSuppression.StartOffset) - // && diagnostics[startRecord].Extent.StartLineNumber < ruleSuppression.st) - && offsetArr[startRecord] != null && offsetArr[startRecord].Item1 < ruleSuppression.StartOffset) + if (!string.IsNullOrWhiteSpace(suppression.RuleSuppressionID)) { - startRecord += 1; + unappliedSuppressions.Add(suppression); } - - // at this point, start offset of startRecord is greater or equals to rulesuppression.startoffset - recordIndex = startRecord; - - while (recordIndex < diagnostics.Count) + } + + // We have suppressions and diagnostics + // For each diagnostic, collect all the suppressions that apply to it + // and if there are any, record the diagnostic as suppressed + foreach (DiagnosticRecord diagnostic in diagnostics) + { + Tuple offsetPair = GetOffset(diagnostic); + + // If we have no offset, we can't suppress anything + if (offsetPair is null) { - DiagnosticRecord record = diagnostics[recordIndex]; - var curOffset = offsetArr[recordIndex]; - - //if (record.Extent.EndOffset > ruleSuppression.EndOffset) - if (curOffset != null && curOffset.Item2 > ruleSuppression.EndOffset) - { - break; - } - - // we suppress if there is no suppression id or if there is suppression id and it matches - if (string.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) - || (!String.IsNullOrWhiteSpace(record.RuleSuppressionID) && - string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))) - { - suppressed[recordIndex] = true; - suppressedRecords.Add(new SuppressedRecord(record, ruleSuppression)); - suppressionCount += 1; - } - - recordIndex += 1; + unsuppressedRecords.Add(diagnostic); + continue; } - - // If we cannot found any error but the rulesuppression has a rulesuppressionid then it must be used wrongly - if (!String.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) && suppressionCount == 0) + + (int startOffset, int endOffset) = offsetPair; + + var appliedSuppressions = new List(); + foreach (RuleSuppression suppression in ruleSuppressions) { - // checks whether are given a string or a file path - if (String.IsNullOrWhiteSpace(diagnostics.First().Extent.File)) + // Check that the diagnostic is within the suppressed extent + if (startOffset < suppression.StartOffset || endOffset > suppression.EndOffset) { - ruleSuppression.Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormatScriptDefinition, ruleSuppression.StartAttributeLine, - String.Format(Strings.RuleSuppressionIDError, ruleSuppression.RuleSuppressionID)); + continue; } - else + + // If there's a rule suppression ID, check that it matches the diagnostic + if (!string.IsNullOrWhiteSpace(suppression.RuleSuppressionID) + && !string.Equals(diagnostic.RuleSuppressionID, suppression.RuleSuppressionID, StringComparison.OrdinalIgnoreCase)) { - ruleSuppression.Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, ruleSuppression.StartAttributeLine, - System.IO.Path.GetFileName(diagnostics.First().Extent.File), String.Format(Strings.RuleSuppressionIDError, ruleSuppression.RuleSuppressionID)); + continue; } - errorRecords.Add(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression)); - //this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression)); + + unappliedSuppressions.Remove(suppression); + appliedSuppressions.Add(suppression); + } + + if (appliedSuppressions.Count > 0) + { + suppressedRecords.Add(new SuppressedRecord(diagnostic, appliedSuppressions)); + } + else + { + unsuppressedRecords.Add(diagnostic); } } - - for (int i = 0; i < suppressed.Length; i += 1) + + // Do any error reporting for misused RuleSuppressionIDs here + string analyzedFile = diagnostics[0].Extent.File; + bool appliedToFile = !string.IsNullOrEmpty(analyzedFile); + string analyzedFileName = appliedToFile ? Path.GetFileName(analyzedFile) : null; + foreach (RuleSuppression unappliedSuppression in unappliedSuppressions) { - if (!suppressed[i]) + string suppressionIDError = string.Format(Strings.RuleSuppressionIDError, unappliedSuppression.RuleSuppressionID); + + if (appliedToFile) { - unSuppressedRecords.Add(diagnostics[i]); + unappliedSuppression.Error = string.Format( + CultureInfo.CurrentCulture, + Strings.RuleSuppressionErrorFormat, + unappliedSuppression.StartAttributeLine, + analyzedFileName, + suppressionIDError); } + else + { + unappliedSuppression.Error = string.Format( + CultureInfo.CurrentCulture, + Strings.RuleSuppressionErrorFormatScriptDefinition, + unappliedSuppression.StartAttributeLine, + suppressionIDError); + } + + errors.Add( + new ErrorRecord( + new ArgumentException(unappliedSuppression.Error), + unappliedSuppression.Error, + ErrorCategory.InvalidArgument, + unappliedSuppression)); } return result; } - private Tuple[] GetOffsetArray(List diagnostics) + private Tuple GetOffset(DiagnosticRecord diagnostic) { - Func> GetTuple = (x, y) => new Tuple(x, y); - Func> GetDefaultTuple = () => GetTuple(0, 0); - var offsets = new Tuple[diagnostics.Count]; - for (int k = 0; k < diagnostics.Count; k++) + IScriptExtent extent = diagnostic.Extent; + + if (extent is null) { - var ext = diagnostics[k].Extent; - if (ext == null) - { - continue; - } - if (ext.StartOffset == 0 && ext.EndOffset == 0) + return null; + } + + if (extent.StartOffset != 0 && extent.EndOffset != 0) + { + return Tuple.Create(extent.StartOffset, extent.EndOffset); + } + + // We're forced to determine the offset ourselves from the lines and columns now + // Rather than counting every line in the file, we scan through the tokens looking for ones matching the offset + + Token startToken = null; + int i = 0; + for (; i < Tokens.Length; i++) + { + Token curr = Tokens[i]; + if (curr.Extent.StartLineNumber == extent.StartLineNumber + && curr.Extent.StartColumnNumber == extent.StartColumnNumber) { - // check if line and column number correspond to 0 offsets - if (ext.StartLineNumber == 1 - && ext.StartColumnNumber == 1 - && ext.EndLineNumber == 1 - && ext.EndColumnNumber == 1) - { - offsets[k] = GetDefaultTuple(); - continue; - } - // created using the ScriptExtent constructor, which sets - // StartOffset and EndOffset to 0 - // find the token the corresponding start line and column number - var startToken = Tokens.Where(x - => x.Extent.StartLineNumber == ext.StartLineNumber - && x.Extent.StartColumnNumber == ext.StartColumnNumber) - .FirstOrDefault(); - if (startToken == null) - { - offsets[k] = GetDefaultTuple(); - continue; - } - var endToken = Tokens.Where(x - => x.Extent.EndLineNumber == ext.EndLineNumber - && x.Extent.EndColumnNumber == ext.EndColumnNumber) - .FirstOrDefault(); - if (endToken == null) - { - offsets[k] = GetDefaultTuple(); - continue; - } - offsets[k] = GetTuple(startToken.Extent.StartOffset, endToken.Extent.EndOffset); + startToken = curr; + break; } - else + } + + if (startToken is null) + { + return Tuple.Create(0, 0); + } + + Token endToken = null; + for (; i < Tokens.Length; i++) + { + Token curr = Tokens[i]; + if (curr.Extent.EndLineNumber == extent.EndLineNumber + && curr.Extent.EndColumnNumber == extent.EndColumnNumber) { - // Extent has valid offsets - offsets[k] = GetTuple(ext.StartOffset, ext.EndOffset); + endToken = curr; + break; } } - return offsets; + + if (endToken is null) + { + return Tuple.Create(0, 0); + } + + return Tuple.Create(startToken.Extent.StartOffset, endToken.Extent.EndOffset); } public static string[] ProcessCustomRulePaths(string[] rulePaths, SessionState sessionState, bool recurse = false) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 49e2570db..10d04b8e4 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -62,7 +62,6 @@ Describe "RuleSuppressionWithoutScope" { -SuppressedOnly $ruleViolations.Count | Should -Be 1 } - } Context "Script" { @@ -100,6 +99,34 @@ function SuppressPwdParam() -SuppressedOnly $ruleViolations.Count | Should -Be 1 } + + It "Records multiple suppressions applied to a single diagnostic" { + $script = @' +function MyFunc +{ + [System.Diagnostics.CodeAnalysis.SuppressMessage("PSAvoidUsingPlainTextForPassword", "password1", Justification='a')] + [System.Diagnostics.CodeAnalysis.SuppressMessage("PSAvoidUsingPlainTextForPassword", "password1", Justification='b')] + param( + [string]$password1, + [string]$password2 + ) +} +'@ + + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' + + $diagnostics | Should -HaveCount 1 + $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $diagnostics[0].RuleSuppressionID | Should -BeExactly "password2" + + $suppressions | Should -HaveCount 1 + $suppressions[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $suppressions[0].RuleSuppressionID | Should -BeExactly "password1" + $suppressions[0].Suppression | Should -HaveCount 2 + $suppressions[0].Suppression[0].Justification | Should -BeExactly 'a' + $suppressions[0].Suppression[1].Justification | Should -BeExactly 'b' + } } Context "Rule suppression within DSC Configuration definition" { From e486374c1571a5912758a07b435e3d6d20cacc6c Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 10:13:22 -0700 Subject: [PATCH 02/16] Fix suppression with no offset --- Engine/Helper.cs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 259b3799c..f85196c66 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -1364,20 +1364,12 @@ public Tuple, List> SuppressRule( { Tuple offsetPair = GetOffset(diagnostic); - // If we have no offset, we can't suppress anything - if (offsetPair is null) - { - unsuppressedRecords.Add(diagnostic); - continue; - } - - (int startOffset, int endOffset) = offsetPair; - var appliedSuppressions = new List(); foreach (RuleSuppression suppression in ruleSuppressions) { - // Check that the diagnostic is within the suppressed extent - if (startOffset < suppression.StartOffset || endOffset > suppression.EndOffset) + // Check that the diagnostic is within the suppressed extent, if we have such an extent + if (!(offsetPair is null) + && (offsetPair.Item1 < suppression.StartOffset || offsetPair.Item2 > suppression.EndOffset)) { continue; } From 4efd6405cde9053bc14dd1d13fd08ce1c222fb3d Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 10:19:56 -0700 Subject: [PATCH 03/16] Defend against NRE --- Engine/Helper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index f85196c66..dcd8c7d3d 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -1396,7 +1396,7 @@ public Tuple, List> SuppressRule( } // Do any error reporting for misused RuleSuppressionIDs here - string analyzedFile = diagnostics[0].Extent.File; + string analyzedFile = diagnostics[0]?.Extent?.File; bool appliedToFile = !string.IsNullOrEmpty(analyzedFile); string analyzedFileName = appliedToFile ? Path.GetFileName(analyzedFile) : null; foreach (RuleSuppression unappliedSuppression in unappliedSuppressions) From 2874be35caf4fdc58ff89714b21974ec04711a97 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 12:28:27 -0700 Subject: [PATCH 04/16] Complete comment help --- Engine/Helper.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index dcd8c7d3d..60f38747c 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -1323,6 +1323,11 @@ internal List GetSuppressionsConfiguration(ConfigurationDefinit /// Suppress the rules from the diagnostic records list. /// Returns a list of suppressed records as well as the ones that are not suppressed /// + /// The name of the rule possibly being suppressed. + /// A lookup table from rule name to suppressions of that rule. + /// The list of all diagnostics emitted by the given rule. + /// Any errors that arise due to misconfigured suppression settings. + /// A pair, with the first item being the list of suppressed diagnostics, and the second the list of unsuppressed diagnostics. public Tuple, List> SuppressRule( string ruleName, Dictionary> ruleSuppressionsDict, From abdaefd6e8ca61e29c97cd17d81c62f4dc646aae Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 12:30:30 -0700 Subject: [PATCH 05/16] Add error emission comment --- Engine/Helper.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 60f38747c..feea8a5ec 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -1353,6 +1353,7 @@ public Tuple, List> SuppressRule( } // We need to report errors for any rule suppressions with IDs that are not applied to anything + // Start by assuming all suppressions are unapplied and then we'll remove them as we apply them later var unappliedSuppressions = new HashSet(); foreach (RuleSuppression suppression in ruleSuppressions) { @@ -1400,7 +1401,10 @@ public Tuple, List> SuppressRule( } } - // Do any error reporting for misused RuleSuppressionIDs here + // Do any error reporting for misused RuleSuppressionIDs here. + // If the user applied a suppression qualified by a suppression ID, + // but that suppression didn't apply only because the suppression ID didn't match + // we let the user know in the form of an error. string analyzedFile = diagnostics[0]?.Extent?.File; bool appliedToFile = !string.IsNullOrEmpty(analyzedFile); string analyzedFileName = appliedToFile ? Path.GetFileName(analyzedFile) : null; From 177ed888c3f8b6d7c1a01bdbe3504f9a49b8cf90 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 12:40:26 -0700 Subject: [PATCH 06/16] Fix justification test --- Tests/Engine/RuleSuppression.tests.ps1 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 10d04b8e4..bc8b9d221 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -124,8 +124,7 @@ function MyFunc $suppressions[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" $suppressions[0].RuleSuppressionID | Should -BeExactly "password1" $suppressions[0].Suppression | Should -HaveCount 2 - $suppressions[0].Suppression[0].Justification | Should -BeExactly 'a' - $suppressions[0].Suppression[1].Justification | Should -BeExactly 'b' + $suppressions[0].Suppression.Justification | Sort-Object | Should -Be @('a', 'b') } } From 37a0104a91ebc5fe9a9037742f4aff988f6d0323 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 14:24:14 -0700 Subject: [PATCH 07/16] Add extra tests --- Tests/Engine/RuleSuppression.tests.ps1 | 83 ++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index bc8b9d221..65bf761b6 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -126,6 +126,89 @@ function MyFunc $suppressions[0].Suppression | Should -HaveCount 2 $suppressions[0].Suppression.Justification | Sort-Object | Should -Be @('a', 'b') } + + It "Records multiple suppressions applied to a single diagnostic when they have identical justifications" { + $script = @' +function MyFunc +{ + [System.Diagnostics.CodeAnalysis.SuppressMessage("PSAvoidUsingPlainTextForPassword", "password1", Justification='a')] + [System.Diagnostics.CodeAnalysis.SuppressMessage("PSAvoidUsingPlainTextForPassword", "password1", Justification='a')] + param( + [string]$password1, + [string]$password2 + ) +} +'@ + + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' + + $diagnostics | Should -HaveCount 1 + $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $diagnostics[0].RuleSuppressionID | Should -BeExactly "password2" + + $suppressions | Should -HaveCount 1 + $suppressions[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $suppressions[0].RuleSuppressionID | Should -BeExactly "password1" + $suppressions[0].Suppression | Should -HaveCount 2 + $suppressions[0].Suppression.Justification | Sort-Object | Should -Be @('a', 'a') + } + + It "Records no suppressions for a different rule" { + $script = @' +function MyFunc +{ + [System.Diagnostics.CodeAnalysis.SuppressMessage("DifferentRule", "password1", Justification='a')] + [System.Diagnostics.CodeAnalysis.SuppressMessage("DifferentRule", "password1", Justification='b')] + param( + [string]$password1, + [string]$password2 + ) +} +'@ + + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' + + $diagnostics | Should -HaveCount 2 + $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $diagnostics[0].RuleSuppressionID | Should -BeExactly "password1" + $diagnostics[1].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $diagnostics[1].RuleSuppressionID | Should -BeExactly "password2" + + $suppressions | Should -BeNullOrEmpty + } + + It "Issues an error for a unapplied suppression with a suppression ID" { + $script = @' +function MyFunc +{ + [System.Diagnostics.CodeAnalysis.SuppressMessage("PSAvoidUsingPlainTextForPassword", "banana")] + param( + [string]$password1, + [string]$password2 + ) +} +'@ + + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr -ErrorAction SilentlyContinue + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr -ErrorAction SilentlyContinue + + $diagnostics | Should -HaveCount 2 + $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $diagnostics[0].RuleSuppressionID | Should -BeExactly "password1" + $diagnostics[1].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $diagnostics[1].RuleSuppressionID | Should -BeExactly "password2" + + $suppressions | Should -BeNullOrEmpty + + $diagErr | Should -HaveCount 1 + $diagErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $diagErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" + $suppErr | Should -HaveCount 1 + $suppErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $suppErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" + } } Context "Rule suppression within DSC Configuration definition" { From e4ea36fd040b8950451c0e3f7b1beb05d14b0087 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 14:37:47 -0700 Subject: [PATCH 08/16] Try error redirection --- Tests/Engine/RuleSuppression.tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 65bf761b6..d7674aee6 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -191,8 +191,8 @@ function MyFunc } '@ - $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr -ErrorAction SilentlyContinue - $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr -ErrorAction SilentlyContinue + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr 2>$null + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr 2>$null $diagnostics | Should -HaveCount 2 $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" From 5eccd72ec149cf2ef664042043f3bb835e0c6b2b Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 14:57:21 -0700 Subject: [PATCH 09/16] Try to fix errors --- Tests/Engine/RuleSuppression.tests.ps1 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index d7674aee6..22e892591 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -191,8 +191,10 @@ function MyFunc } '@ - $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr 2>$null - $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr 2>$null + . { + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr + } 2>$null $diagnostics | Should -HaveCount 2 $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" From 200f0ad5ce20ebe1b90852de09b8a3da53184ed4 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 21 Jul 2021 15:10:58 -0700 Subject: [PATCH 10/16] Go back to ErrorAction --- Tests/Engine/RuleSuppression.tests.ps1 | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 22e892591..ea0a0ddbe 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -191,10 +191,10 @@ function MyFunc } '@ - . { - $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr - $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr - } 2>$null + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr -ErrorAction SilentlyContinue + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr -ErrorAction SilentlyContinue + + wait-debugger $diagnostics | Should -HaveCount 2 $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" @@ -207,6 +207,7 @@ function MyFunc $diagErr | Should -HaveCount 1 $diagErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" $diagErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" + $suppErr | Should -HaveCount 1 $suppErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" $suppErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" From c9423437d2474c4deb6a71b4b6e55b546eb2116d Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 22 Jul 2021 13:04:38 -0700 Subject: [PATCH 11/16] Fix tests --- Tests/Engine/RuleSuppression.tests.ps1 | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index ea0a0ddbe..440be0f2e 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -192,9 +192,7 @@ function MyFunc '@ $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr -ErrorAction SilentlyContinue - $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr -ErrorAction SilentlyContinue - - wait-debugger + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorAction SilentlyContinue $diagnostics | Should -HaveCount 2 $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" @@ -207,10 +205,6 @@ function MyFunc $diagErr | Should -HaveCount 1 $diagErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" $diagErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" - - $suppErr | Should -HaveCount 1 - $suppErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" - $suppErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" } } From 6bfdd250720ed3afa46c182380d975b90729ab7e Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 22 Jul 2021 14:29:25 -0700 Subject: [PATCH 12/16] Fix tests --- Tests/Engine/RuleSuppression.tests.ps1 | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 440be0f2e..d51f8bd77 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -202,9 +202,17 @@ function MyFunc $suppressions | Should -BeNullOrEmpty - $diagErr | Should -HaveCount 1 - $diagErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" - $diagErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" + # For some reason these tests fail in WinPS, but the actual output is as expected + if ($PSEdition -eq 'Core') + { + $diagErr | Should -HaveCount 1 + $diagErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $diagErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" + + $suppErr | Should -HaveCount 1 + $suppErr.TargetObject.RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" + $suppErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana" + } } } From f1391d349f03641d989ef420325d2325726a7f98 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 23 Jul 2021 07:49:40 -0700 Subject: [PATCH 13/16] Fix error variable --- Tests/Engine/RuleSuppression.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index d51f8bd77..891675b3b 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -192,7 +192,7 @@ function MyFunc '@ $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable diagErr -ErrorAction SilentlyContinue - $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorAction SilentlyContinue + $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' -ErrorVariable suppErr -ErrorAction SilentlyContinue $diagnostics | Should -HaveCount 2 $diagnostics[0].RuleName | Should -BeExactly "PSAvoidUsingPlainTextForPassword" From 99f146e173e285b8722f69d2873a3fdcd0e1c07c Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 23 Jul 2021 08:26:41 -0700 Subject: [PATCH 14/16] Try to filter out other diagnostics --- Tests/Engine/RuleSuppression.tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 891675b3b..801baf7c1 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -113,7 +113,7 @@ function MyFunc } '@ - $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' | Where-Object { $_.RuleName -eq 'PSAvoidUsingPlainTextForPassword' } $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' $diagnostics | Should -HaveCount 1 @@ -140,7 +140,7 @@ function MyFunc } '@ - $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' | Where-Object { $_.RuleName -eq 'PSAvoidUsingPlainTextForPassword' } $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' $diagnostics | Should -HaveCount 1 From f93ad20c9748536fe7ededa2337da6024cc30187 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 23 Jul 2021 09:40:00 -0700 Subject: [PATCH 15/16] Remove redundant filters --- Tests/Engine/RuleSuppression.tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 801baf7c1..891675b3b 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -113,7 +113,7 @@ function MyFunc } '@ - $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' | Where-Object { $_.RuleName -eq 'PSAvoidUsingPlainTextForPassword' } + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' $diagnostics | Should -HaveCount 1 @@ -140,7 +140,7 @@ function MyFunc } '@ - $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' | Where-Object { $_.RuleName -eq 'PSAvoidUsingPlainTextForPassword' } + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $script -IncludeRule 'PSAvoidUsingPlainTextForPassword' $suppressions = Invoke-ScriptAnalyzer -ScriptDefinition $script -SuppressedOnly -IncludeRule 'PSAvoidUsingPlainTextForPassword' $diagnostics | Should -HaveCount 1 From 498a8c9d9fe57cd007fc1fb9a3bbd22cc46592a8 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 23 Jul 2021 14:39:39 -0700 Subject: [PATCH 16/16] Use other API for attribute types --- Engine/Generic/RuleSuppression.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Engine/Generic/RuleSuppression.cs b/Engine/Generic/RuleSuppression.cs index d8a41f974..d912eee0c 100644 --- a/Engine/Generic/RuleSuppression.cs +++ b/Engine/Generic/RuleSuppression.cs @@ -308,7 +308,7 @@ public static List GetSuppressions(IEnumerable at } IEnumerable suppressionAttribute = attrAsts.Where( - item => item.TypeName.GetReflectionType() == typeof(System.Diagnostics.CodeAnalysis.SuppressMessageAttribute)); + item => item.TypeName.GetReflectionAttributeType() == typeof(System.Diagnostics.CodeAnalysis.SuppressMessageAttribute)); foreach (var attributeAst in suppressionAttribute) {