Prevent duplicate rule ID suppression errors in `-Fix` and DSC scenarios by liamjpeters · Pull Request #2181 · PowerShell/PSScriptAnalyzer · GitHub

PR Summary

When using -Fix, only emit errors for unused rule suppressions IDs on the final pass - not intermediate Fix() passes.
Make the two DSC branches mutually exclusive (skip the module-layout branch when the class-based branch already handled the AST). Prevents multiple copies of unused rule suppression ID error.

Added regression tests covering these 2 scenarios.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

…r unapplied suppression errors

Thanks for picking this up — the repro in #2177 was solid and this fix matches it cleanly.

For the -Fix case, threading emitSuppressionErrors: false through the Fix() loop's AnalyzeSyntaxTree / AnalyzeScriptDefinition calls is the right shape: every iteration inside the loop suppresses, the post-loop AnalyzeFile emits once, no duplicates. For the DSC case, swapping the second if to else if so the class-DSC and module-DSC handler branches are mutually exclusive is exactly the right minimal fix. Both regression tests pass locally for me.

LGTM — approving pending CI passing.

— Drafted by Copilot (Claude Opus 4.7)

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).

Christoph Bergmeister (@bergmeister) I'll need your review since the super stringent code review requirements now bar me from merging after having Claude fix the failing test in CI.

Christoph Bergmeister (@bergmeister) I'll need your review since the super stringent code review requirements now bar me from merging after having Claude fix the failing test in CI.

Sure. Will look at this and others tomorrow

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR prevents duplicate “unapplied suppression” errors that can occur during -Fix reanalysis passes and in DSC scenarios where both class-based and module-layout DSC logic apply to the same file.

Changes:

Adds an emitSuppressionErrors switch to avoid emitting suppression errors during intermediate -Fix passes.
Makes DSC class-based vs module-layout analysis mutually exclusive to avoid duplicate suppression error emission.
Adds regression tests for -Fix reanalysis and mixed DSC layout/class scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
Tests/Rules/UseDSCResourceFunctions.tests.ps1	Adds regression test ensuring DSC suppression errors aren’t duplicated when both DSC detection paths apply.
Tests/Engine/RuleSuppression.tests.ps1	Adds regression test ensuring -Fix reanalysis doesn’t emit multiple unapplied suppression errors.
Engine/ScriptAnalyzer.cs	Introduces emitSuppressionErrors plumbing and makes DSC analysis branches mutually exclusive to prevent duplicate suppression errors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

I'm not sure how much of a real concern this is. While it's technically correct, I'm not sure anyone takes a package reference on PSScriptAnalyzer. PSES is the only tightly coupled .NET caller and that updates alongside PSSA.

These signatures have changed before (When adding skipVariable analysis) without shipping overloads.

But to appease the AI review gods, I've restored the original method signature and added an overload - there's no harm in doing so.

I'm not sure how much of a real concern this is. While it's technically correct, I'm not sure anyone takes a package reference on PSScriptAnalyzer. PSES is the only tightly coupled .NET caller and that updates alongside PSSA.

These signatures have changed before (When adding skipVariable analysis) without shipping overloads.

But to appease the AI review gods, I've restored the original method signature and added an overload.

The overload restoration in 0efd4876 is fine — EmitSuppressionErrors now preserves the old AnalyzeScriptDefinition and AnalyzeSyntaxTree signatures and forwards through to the new internal-bool form, which addresses the binary-compat concern without changing call-site shape. CI is green on all three platforms (run 25387220273). Re-affirming the approval — this is good to merge from my side.

Drafted by Copilot (Claude Sonnet 4.6)

We are all just appeasing the AI gods now aren't we 😆

Sorry, forgot. Looks good to me

Enhance suppression error handling in ScriptAnalyzer and add tests fo…

48e1d42

…r unapplied suppression errors

Liam Peters (liamjpeters) requested review from a team and Christoph Bergmeister (bergmeister) as code owners April 28, 2026 19:01

Andy Jordan (andyleejordan) approved these changes May 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings May 4, 2026 20:40

Copilot AI reviewed May 4, 2026

View reviewed changes

Copilot started reviewing on behalf of Andy Jordan (andyleejordan) May 4, 2026 20:48 View session

Liam Peters (liamjpeters) added 2 commits May 5, 2026 08:32

Merge branch 'main' into DuplicateFixSuppressionError

822c6af

Add copilot review suggestions

0efd487

Andy Jordan (andyleejordan) approved these changes May 7, 2026

View reviewed changes

Andy Jordan (andyleejordan) merged commit 940024a into PowerShell:main May 7, 2026
4 checks passed

Liam Peters (liamjpeters) deleted the DuplicateFixSuppressionError branch May 8, 2026 06:55

Prevent duplicate rule ID suppression errors in `-Fix` and DSC scenarios by liamjpeters · Pull Request #2181 · PowerShell/PSScriptAnalyzer · GitHub

Uh oh!

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

Prevent duplicate rule ID suppression errors in -Fix and DSC scenarios#2181
Andy Jordan (andyleejordan) merged 4 commits into
PowerShell:mainfrom
liamjpeters:DuplicateFixSuppressionError

Liam Peters (liamjpeters) commented Apr 28, 2026

Uh oh!

Andy Jordan (andyleejordan) left a comment

Uh oh!

Andy Jordan (andyleejordan) commented May 4, 2026

Uh oh!

Christoph Bergmeister (bergmeister) commented May 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Liam Peters (liamjpeters) May 5, 2026 •
edited
Loading

Uh oh!

Uh oh!

Liam Peters (liamjpeters) May 5, 2026

Uh oh!

Uh oh!

Andy Jordan (andyleejordan) left a comment

Uh oh!

Uh oh!

Andy Jordan (andyleejordan) commented May 7, 2026

Uh oh!

Christoph Bergmeister (bergmeister) commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		public List<DiagnosticRecord> AnalyzeScriptDefinition(string scriptDefinition, out ScriptBlockAst scriptAst, out Token[] scriptTokens, bool skipVariableAnalysis = false, bool emitSuppressionErrors = true)
		{

+                          bool skipVariableAnalysis = false,
+                          bool emitSuppressionErrors = true)
                       {

Prevent duplicate rule ID suppression errors in `-Fix` and DSC scenarios by liamjpeters · Pull Request #2181 · PowerShell/PSScriptAnalyzer · GitHub

Uh oh!

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

Conversation

Liam Peters (liamjpeters) commented Apr 28, 2026

PR Summary

PR Checklist

Uh oh!

Andy Jordan (andyleejordan) left a comment

Choose a reason for hiding this comment

Uh oh!

Andy Jordan (andyleejordan) commented May 4, 2026

Uh oh!

Christoph Bergmeister (bergmeister) commented May 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Liam Peters (liamjpeters) May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Liam Peters (liamjpeters) May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Andy Jordan (andyleejordan) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Andy Jordan (andyleejordan) commented May 7, 2026

Uh oh!

Christoph Bergmeister (bergmeister) commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Liam Peters (liamjpeters) May 5, 2026 •
edited
Loading