Skip to content

C#: Support suppression comments in XML files - #4948

Draft
rdmarsh2 wants to merge 3 commits into
github:mainfrom
rdmarsh2:rdmarsh2/csharp/xml-suppression-comments
Draft

C#: Support suppression comments in XML files#4948
rdmarsh2 wants to merge 3 commits into
github:mainfrom
rdmarsh2:rdmarsh2/csharp/xml-suppression-comments

Conversation

@rdmarsh2

Copy link
Copy Markdown
Contributor

This adds a new @kind alert-suppression query which handles XML comments and accepts the same format as the existing query for C# comments.

@rdmarsh2
rdmarsh2 requested a review from a team as a code owner January 12, 2021 22:00
@github-actions github-actions Bot added the C# label Jan 12, 2021

@hvitved hvitved left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except I think we should move the logic to the shared XML.qll library.

Comment thread csharp/ql/src/XMLAlertSuppression.ql Outdated
/**
* An alert suppression comment.
*/
class SuppressionComment extends XMLComment {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like this class and SuppressionScope should be in XML.qll, so it can be reused by other languages.

RasmusWL
RasmusWL previously approved these changes Feb 26, 2021

@RasmusWL RasmusWL left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python part 👍

MathiasVP
MathiasVP previously approved these changes Feb 26, 2021

@MathiasVP MathiasVP left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C++ 👍

@rdmarsh2
rdmarsh2 requested a review from hvitved February 26, 2021 19:03

@hvitved hvitved left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-format check fails.

@rdmarsh2
rdmarsh2 dismissed stale reviews from MathiasVP and RasmusWL via 04496f0 March 1, 2021 23:00
@MathiasVP

Copy link
Copy Markdown
Contributor

Should this change be documented in a change-note?

@aschackmull

Copy link
Copy Markdown
Contributor

Should this change be documented in a change-note?

It probably should, yes.

@tausbn

tausbn commented May 17, 2022

Copy link
Copy Markdown
Contributor

This one seems to have fallen through the cracks. Should it be resurrected, or is it no longer relevant?

@rdmarsh2

Copy link
Copy Markdown
Contributor Author

Probably worth resurrecting at some point - I think C# is still the only language where our queries report results in XML files, but it's still possible for custom queries to report results in XML for other languages.

@calumgrant

Copy link
Copy Markdown
Contributor

Is this still live or can it be closed/drafted?

@rdmarsh2
rdmarsh2 marked this pull request as draft June 27, 2022 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants