Fix XML suppression comments in CLI suppress command

[Copilot]

• Explore and understand the codebase and issue CLI Suppress Command Does not properly comment XML suppressions #588
• Review PR Fix related to XML suppressions not working #705 to understand the attempted fix
• Build and verify existing tests pass (299 tests passing)
• Add XML suppression comment syntax to comments.json
• Add unit tests for XML suppression generation
• Add integration tests for XML suppression via CLI
• Manually verify XML suppression works with CLI suppress command
• Restore original nuget.config (keep private feed configuration)
• Run code review and address feedback
• Refactor foreach loops to use explicit .Where() filtering
• Move DateTime calculation before command execution
• Add try-finally for proper temp file cleanup
• Use .Select() for mapping in foreach loop

Summary

This PR fixes issue #588 where XML suppressions were not properly commented when using the DevSkim CLI suppress command.

Changes Made:

1. Added XML comment syntax to comments.json

   • Added new entry for xml language with prefix: "<!--" and suffix: "-->"
   • This enables proper XML comment formatting for suppression comments

2. Added comprehensive test coverage (13 new tests)

   • Unit tests for XML suppression generation (basic, with duration, with reviewer)
   • Unit tests for XML suppression parsing (basic, expired, with reviewer, with expiration)
   • Integration tests for CLI suppress command with XML files
   • Integration tests with duration and reviewer options
   • Test to verify suppressions are respected during analysis

Verification:

• All 312 tests pass (299 existing + 13 new XML-related tests)
• Manually verified CLI suppress workflow works with XML files
• Code review passed with no issues

Original prompt

Author a new PR in this repo based on #705 attempting to complete the same task of fixing issues with XML suppressions previously not functioning properly. Ensure that there is new, comprehensive test coverage for the changes and that the tests pass. You should also validate that the devskim 'suppress' CLI flow works with this new kind of suppression.

If you needed you may modify the nuget.config to fix restore issues while working, but do not commit changes to the config - the checked in config should remain with the private feed.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[gfs]

Based on the work initially done in #705 but in a branch on the main repo so it can be iterated on by Copilot and the PR gates run.

[gfs]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

This PR fixes XML suppression comments in the DevSkim CLI to use proper XML comment delimiters (<!-- -->). Previously, XML files were receiving suppression text without comment delimiters, making the suppressions invalid and non-functional.

Changes:

• Added XML language entry to comments.json with proper prefix (<!--) and suffix (-->) for multi-line comments
• Added 13 comprehensive tests covering XML suppression generation, parsing, expiration handling, reviewer attribution, and CLI integration

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
DevSkim-DotNet/Microsoft.DevSkim/resources/comments.json	Added XML language configuration with correct comment delimiters
DevSkim-DotNet/Microsoft.DevSkim.Tests/SuppressionsTest.cs	Added comprehensive test suite for XML suppression functionality

Comments suppressed due to low confidence (1)

DevSkim-DotNet/Microsoft.DevSkim.Tests/SuppressionsTest.cs:538

• This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

            foreach (string line in lines)
            {
                if (line.Contains("<!-- DevSkim: ignore"))
                {
                    Suppression suppression = new Suppression(line);
                    Assert.IsTrue(suppression.IsInEffect, "Suppression should be in effect");
                    
                    if (!string.IsNullOrEmpty(reviewerName))
                    {
                        Assert.AreEqual(reviewerName, suppression.Reviewer, "Reviewer name should match");
                    }
                    
                    if (duration > 0)
                    {
                        DateTime expectedExpiration = DateTime.Now.AddDays(duration);
                        Assert.AreEqual(expectedExpiration.Date, suppression.ExpirationDate, "Expiration date should match");
                    }
                }
            }

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

DevSkim-DotNet/Microsoft.DevSkim.Tests/SuppressionsTest.cs:532

• This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

            foreach (string line in suppressionLines)
            {
                Suppression suppression = new Suppression(line);
                Assert.IsTrue(suppression.IsInEffect, "Suppression should be in effect");
                
                if (!string.IsNullOrEmpty(reviewerName))
                {
                    Assert.AreEqual(reviewerName, suppression.Reviewer, "Reviewer name should match");
                }
                
                if (duration > 0)
                {
                    DateTime expectedExpiration = DateTime.Now.AddDays(duration);
                    Assert.AreEqual(expectedExpiration.Date, suppression.ExpirationDate, "Expiration date should match");
                }
            }

---

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

[Copilot]

The expected expiration date should be calculated before calling the suppression command to match the pattern used elsewhere in this test file and to avoid potential timing issues. Move the line DateTime expectedExpiration = DateTime.Now.AddDays(duration); to before line 507 where new SuppressionCommand(opts).Run() is called, similar to how it's done in ExecuteSuppressionsWithReviewerNameAndDate (line 30) and ExecuteSuppressionsWithExpiration (line 166).

[gfs]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

Applied all three changes from the code review:

1. Moved DateTime expectedExpiration calculation before SuppressionCommand.Run() to avoid timing issues
2. Wrapped test logic in try-finally for proper temp file cleanup
3. Used .Select() to map lines to Suppression objects explicitly

Commit: b0d611f

[gfs]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).