Prevent duplicate rule ID suppression errors in -Fix and DSC scenarios (#2181)

* Enhance suppression error handling in ScriptAnalyzer and add tests for unapplied suppression errors

* Skip new -Fix suppression test under Library Usage harness

The `Library Usage` describe block in `LibraryUsage.tests.ps1` (only
active on Windows PowerShell 5.1, since it's gated `-Skip:$IsCoreCLR`)
re-runs `RuleSuppression.tests.ps1` against a hand-rolled
`Invoke-ScriptAnalyzer` wrapper that drives the analyzer as a .NET
library. That wrapper plugs in `PesterTestOutputWriter`, whose
`WriteError` is intentionally a no-op:

    public void WriteError(ErrorRecord error)
    {
        // We don't write errors to avoid misleading
        // error messages in test output
    }

So the unapplied-suppression `ErrorRecord` we now emit during the final
`-Fix` pass never reaches `-ErrorVariable`, and `$fixErr | Should
-HaveCount 1` fails with "Expected a collection with size 1, but got an
empty collection". The behaviour itself is correct - the assertion is
just unobservable through this test harness.

Mark the new `It` block `-Skip:$testingLibraryUsage`, matching the
existing pattern already used by the `Bad Rule Suppression` and
`External Rule Suppression` contexts in the same file for the same
reason. The regular pwsh and WinPS runs of `RuleSuppression.tests.ps1`
(which `[+]` in the failing CI log) continue to exercise the assertion.

The new `UseDSCResourceFunctions.tests.ps1` test isn't dot-sourced by
`LibraryUsage.tests.ps1`, so it doesn't need the same guard.

Drafted by Copilot (Claude Opus 4.7).

* Add copilot review suggestions

---------

Co-authored-by: Andy Jordan <2226434+andyleejordan@users.noreply.github.com>

Author: liamjpeters

Engine/ScriptAnalyzer.cs

@@ -1447,10 +1447,24 @@ public IEnumerable<DiagnosticRecord> AnalyzeAndFixPath(string path, Func<string,
         /// </summary>
         /// <param name="scriptDefinition">The script to be analyzed.</param>
         /// <param name="scriptAst">Parsed AST of <paramref name="scriptDefinition"/>.</param>
-        /// <param name="scriptTokens">Parsed tokens of <paramref name="scriptDefinition"/.></param>
+        /// <param name="scriptTokens">Parsed tokens of <paramref name="scriptDefinition"/>.</param>
         /// <param name="skipVariableAnalysis">Whether variable analysis can be skipped (applicable if rules do not use variable analysis APIs).</param>
         /// <returns></returns>
         public List<DiagnosticRecord> AnalyzeScriptDefinition(string scriptDefinition, out ScriptBlockAst scriptAst, out Token[] scriptTokens, bool skipVariableAnalysis = false)
+        {
+            return AnalyzeScriptDefinition(scriptDefinition, out scriptAst, out scriptTokens, skipVariableAnalysis, emitSuppressionErrors: true);
+        }
+
+        /// <summary>
+        /// Analyzes a script definition in the form of a string input.
+        /// </summary>
+        /// <param name="scriptDefinition">The script to be analysed.</param>
+        /// <param name="scriptAst">Parsed AST of <paramref name="scriptDefinition"/>.</param>
+        /// <param name="scriptTokens">Parsed tokens of <paramref name="scriptDefinition"/>.</param>
+        /// <param name="skipVariableAnalysis">Whether variable analysis can be skipped (applicable if rules do not use variable analysis APIs).</param>
+        /// <param name="emitSuppressionErrors">Whether to emit errors for unapplied rule suppression IDs.</param>
+        /// <returns>A list of diagnostics found by rules.</returns>
+        public List<DiagnosticRecord> AnalyzeScriptDefinition(string scriptDefinition, out ScriptBlockAst scriptAst, out Token[] scriptTokens, bool skipVariableAnalysis, bool emitSuppressionErrors)
         {
             scriptAst = null;
             scriptTokens = null;
@@ -1490,7 +1504,7 @@ public List<DiagnosticRecord> AnalyzeScriptDefinition(string scriptDefinition, o
             }
 
             // now, analyze the script definition
-            diagnosticRecords.AddRange(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, null, skipVariableAnalysis));
+            diagnosticRecords.AddRange(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, null, skipVariableAnalysis, emitSuppressionErrors));
             return diagnosticRecords;
         }
 
@@ -1549,11 +1563,11 @@ public EditableText Fix(EditableText text, Range range, bool skipParsing, out Ra
                 IEnumerable<DiagnosticRecord> records;
                 if (skipParsing && previousUnusedCorrections == 0)
                 {
-                    records = AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty, skipVariableAnalysis);
+                    records = AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty, skipVariableAnalysis, emitSuppressionErrors: false);
                 }
                 else
                 {
-                    records = AnalyzeScriptDefinition(text.ToString(), out scriptAst, out scriptTokens, skipVariableAnalysis);
+                    records = AnalyzeScriptDefinition(text.ToString(), out scriptAst, out scriptTokens, skipVariableAnalysis, emitSuppressionErrors: false);
                 }
                 var corrections = records
                     .Select(r => r.SuggestedCorrections)
@@ -1986,17 +2000,21 @@ bool IsRuleAllowed(IRule rule)
         private Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
             string ruleName,
             Dictionary<string, List<RuleSuppression>> ruleSuppressions,
-            List<DiagnosticRecord> ruleDiagnosticRecords)
+            List<DiagnosticRecord> ruleDiagnosticRecords,
+            bool emitSuppressionErrors = true)
         {
             List<ErrorRecord> suppressRuleErrors;
             var records = Helper.Instance.SuppressRule(
                 ruleName,
                 ruleSuppressions,
                 ruleDiagnosticRecords,
                 out suppressRuleErrors);
-            foreach (var error in suppressRuleErrors)
+            if (emitSuppressionErrors)
             {
-                this.outputWriter.WriteError(error);
+                foreach (var error in suppressRuleErrors)
+                {
+                    this.outputWriter.WriteError(error);
+                }
             }
             return records;
         }
@@ -2014,13 +2032,15 @@ private Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
         /// <returns>Returns a tuple of suppressed and diagnostic records</returns>
         private Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
             Dictionary<string, List<RuleSuppression>> ruleSuppressions,
-            DiagnosticRecord ruleDiagnosticRecord
+            DiagnosticRecord ruleDiagnosticRecord,
+            bool emitSuppressionErrors = true
             )
         {
             return SuppressRule(
                 ruleDiagnosticRecord.RuleName,
                 ruleSuppressions,
-                new List<DiagnosticRecord> { ruleDiagnosticRecord });
+                new List<DiagnosticRecord> { ruleDiagnosticRecord },
+                emitSuppressionErrors);
         }
 
         /// <summary>
@@ -2038,6 +2058,27 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
             Token[] scriptTokens,
             string filePath,
             bool skipVariableAnalysis = false)
+        {
+            return AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath, skipVariableAnalysis, emitSuppressionErrors: true);
+        }
+
+        /// <summary>
+        /// Analyzes the syntax tree of a script file that has already been parsed.
+        /// </summary>
+        /// <param name="scriptAst">The ScriptBlockAst from the parsed script.</param>
+        /// <param name="scriptTokens">The tokens found in the script.</param>
+        /// <param name="filePath">The path to the file that was parsed.
+        /// If AnalyzeSyntaxTree is called from an AST obtained via ParseInput, this field will be String.Empty.
+        /// </param>
+        /// <param name="skipVariableAnalysis">Whether to skip variable analysis.</param>
+        /// <param name="emitSuppressionErrors">Whether to emit errors for unapplied rule suppression IDs.</param>
+        /// <returns>An enumeration of DiagnosticRecords found by rules.</returns>
+        public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
+            ScriptBlockAst scriptAst,
+            Token[] scriptTokens,
+            string filePath,
+            bool skipVariableAnalysis,
+            bool emitSuppressionErrors)
         {
             Dictionary<string, List<RuleSuppression>> ruleSuppressions = new Dictionary<string,List<RuleSuppression>>();
             ConcurrentBag<DiagnosticRecord> diagnostics = new ConcurrentBag<DiagnosticRecord>();
@@ -2117,7 +2158,10 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
                                     ruleSuppressions,
                                     ruleRecords,
                                     out suppressRuleErrors);
-                                result.AddRange(suppressRuleErrors);
+                                if (emitSuppressionErrors)
+                                {
+                                    result.AddRange(suppressRuleErrors);
+                                }
                                 foreach (var record in records.Item2)
                                 {
                                     diagnostics.Add(record);
@@ -2177,7 +2221,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
                         try
                         {
                             var ruleRecords = tokenRule.AnalyzeTokens(scriptTokens, filePath).ToList();
-                            var records = SuppressRule(tokenRule.GetName(), ruleSuppressions, ruleRecords);
+                            var records = SuppressRule(tokenRule.GetName(), ruleSuppressions, ruleRecords, emitSuppressionErrors);
                             foreach (var record in records.Item2)
                             {
                                 diagnostics.Add(record);
@@ -2215,7 +2259,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
                             try
                             {
                                 var ruleRecords = dscResourceRule.AnalyzeDSCClass(scriptAst, filePath).ToList();
-                                var records = SuppressRule(dscResourceRule.GetName(), ruleSuppressions, ruleRecords);
+                                var records = SuppressRule(dscResourceRule.GetName(), ruleSuppressions, ruleRecords, emitSuppressionErrors);
                                 foreach (var record in records.Item2)
                                 {
                                     diagnostics.Add(record);
@@ -2234,7 +2278,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
                 }
 
                 // Check if the supplied artifact is indeed part of the DSC resource
-                if (!filePathIsNullOrWhiteSpace && Helper.Instance.IsDscResourceModule(filePath))
+                else if (!filePathIsNullOrWhiteSpace && Helper.Instance.IsDscResourceModule(filePath))
                 {
                     // Run all DSC Rules
                     foreach (IDSCResourceRule dscResourceRule in this.DSCResourceRules)
@@ -2248,7 +2292,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
                             try
                             {
                                 var ruleRecords = dscResourceRule.AnalyzeDSCResource(scriptAst, filePath).ToList();
-                                var records = SuppressRule(dscResourceRule.GetName(), ruleSuppressions, ruleRecords);
+                                var records = SuppressRule(dscResourceRule.GetName(), ruleSuppressions, ruleRecords, emitSuppressionErrors);
                                 foreach (var record in records.Item2)
                                 {
                                     diagnostics.Add(record);
@@ -2297,7 +2341,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeSyntaxTree(
 
                 foreach (var ruleRecord in this.GetExternalRecord(scriptAst, scriptTokens, exRules.ToArray(), filePath))
                 {
-                    var records = SuppressRule(ruleSuppressions, ruleRecord);
+                    var records = SuppressRule(ruleSuppressions, ruleRecord, emitSuppressionErrors);
                     foreach (var record in records.Item2)
                     {
                         diagnostics.Add(record);

Tests/Engine/RuleSuppression.tests.ps1

@@ -244,6 +244,30 @@ function MyFunc
                 $suppErr.TargetObject.RuleSuppressionID | Should -BeExactly "banana"
             }
         }
+
+        It "Issues one unapplied suppression error when -Fix reanalyzes a file" -Skip:$testingLibraryUsage {
+            $scriptPath = Join-Path $TestDrive 'SuppressionFix.ps1'
+            $script = @(
+                'function Test-Function1 {'
+                "    [System.Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingWriteHost','NonExistentID123')]"
+                '    param() ; Write-Host ''x'''
+                '}'
+            ) -join "`n"
+
+            [System.IO.File]::WriteAllText($scriptPath, $script + "`n")
+
+            $diagnostics = Invoke-ScriptAnalyzer `
+                -Path $scriptPath `
+                -Fix `
+                -ErrorVariable fixErr `
+                -ErrorAction SilentlyContinue
+
+            $diagnostics | Should -HaveCount 1
+            $diagnostics[0].RuleName | Should -BeExactly 'PSAvoidUsingWriteHost'
+            $fixErr | Should -HaveCount 1
+            $fixErr[0].TargetObject.RuleName | Should -BeExactly 'PSAvoidUsingWriteHost'
+            $fixErr[0].TargetObject.RuleSuppressionID | Should -BeExactly 'NonExistentID123'
+        }
     }
 
     Context "RuleSuppressionID with named arguments" {

Tests/Rules/UseDSCResourceFunctions.tests.ps1

@@ -46,4 +46,34 @@ Describe "StandardDSCFunctionsInClass" {
             $noClassViolations.Count | Should -Be 0
         }
     }
+
+    Context "When a class-based DSC resource is also in DSC resource module layout" {
+        It "does not duplicate the unapplied suppression error" {
+            $resourceRoot = Join-Path $TestDrive 'DSCResources'
+            $resourceDir = Join-Path $resourceRoot 'MyRes'
+            $resourcePath = Join-Path $resourceDir 'MyRes.psm1'
+            $schemaPath = Join-Path $resourceDir 'MyRes.schema.mof'
+
+            New-Item -ItemType Directory -Path $resourceDir -Force | Out-Null
+            [System.IO.File]::WriteAllText($resourcePath, @'
+[System.Diagnostics.CodeAnalysis.SuppressMessage('PSDSCStandardDSCFunctionsInResource', 'BadDscId', Scope='Class', Target='MyRes')]
+[DscResource()]
+class MyRes {
+    [DscProperty(Key)] [string] $Name
+    [MyRes] Get() { return $this }
+}
+'@.TrimStart() + "`n")
+            Set-Content -Path $schemaPath -Value ''
+
+            Invoke-ScriptAnalyzer `
+                -Path $resourcePath `
+                -ErrorVariable dscErr `
+                -ErrorAction SilentlyContinue |
+                Out-Null
+
+            $dscErr | Should -HaveCount 1
+            $dscErr[0].TargetObject.RuleName | Should -BeExactly $violationName
+            $dscErr[0].TargetObject.RuleSuppressionID | Should -BeExactly 'BadDscId'
+        }
+    }
 }