[DAM analyzer] Add support for attribute warning suppressions

[sbomer]

The analyzer needs to support suppressing warnings like the linker does. RUC already works, but we still need to add support for UnconditionalSuppressMessage and probably also adding both of these attributes via XML.

edit: UnconditionalSuppressMessage should already be working since roslyn supports it natively - so for non-attribute suppressions we should only need to turn on the existing tests.

@jtschuster is working on attribute XML support so you might want to sync with him @LakshanF

[sbomer]

These are the failing suppressions tests I noticed. I would start with the first two since they aren't about XML suppressions:

• Warnings.WarningSuppressionTests.SuppressWarningsInMembersAndTypesUsingTarget
  warning suppressions in code using Target = "..." syntax
• Warnings.WarningSuppressionTests.SuppressWarningsInCompilerGeneratedCode
  extra warnings in analyzer, needs investigation
• Warnings.WarningSuppressionTests.SuppressWarningsViaXml
  analyzer needs to support XML attributes

You'll probably need to apply the test infra fixes from 2a0cbf9 so these don't crash on LogContains attributes.

[MichalStrehovsky]

analyzer needs to support XML attributes

I know we're trying to add support for reading the XML to the analyzer, but I've internally had question marks around that for a while that I didn't voice.

I see that allowing to do suppressions by faking the attributes through XML will add extra work to this since Roslyn normally handless all the suppressions for us (we don't need to parse and bind the Target, etc.).

Do we really need the attribute XML support? I've always looked at the attribute XML as something that we have internally, but externally it's only used to remove attribute instances and that part is not even real attribute XML, it's just a hack on the file format. I don't see a compelling external use case for attribute XML that would warrant the support cost. When this was first introduced, it was only meant for testing (#1060).

It's kind of like if Roslyn provided an XML way to add nullable attributes to existing libraries for the purposes of compiling code against those libraries. Would some people find it useful? Probably. Would it be really fragile because it doesn't version with the assembly we're attributing? Definitely. Such feature would probably not meet the bar for Roslyn. Why does it meet the bar for us?

Cc @vitek-karas @agocke

[vitek-karas]

@MichalStrehovsky I think that is fair analysis. As far as I know we currently use them for two purposes:

• Suppress warnings for assemblies which we didn't annotate yet
• Attribute removal

From these two, only the attribute removal XMLs are actually embedded in the assemblies, the suppression XMLs are only passed on the command line and thus would not actually work with the analyzer (without additional, even more hacky work).

Attribute removal is something the analyzer will ignore - there's one warning tied to it, but that one is highly unlikely to be generated by the analyzer as it's typically only occurring for cross-assembly references, which the analyzer would not see anyway.

So that basically leaves the scenario which @MichalStrehovsky points out, adding annotations to libraries which don't have them. Again the analyzer in this case would only work if the XML is embedded into the assembly which is currently being compiled.

I do agree that the value of supporting that is relatively low and we should be fine shipping without such support.
Note: For the runtime repo case, I would argue that ignoring the suppression XMLs is a good thing, when in IDE I actually want to see these as they are effectively TODOs. And in the official build we can suppress the analyzer as a whole on assemblies which do have suppression XMLs, as those assemblies are not yet annotated anyway

[sbomer]

We have decided not to support XML for warning suppressions in the analyzer. The plan is to replace all permanent suppressions in dotnet/runtime with UnconditionalSuppressMessage in code, so they will just work with the analyzer. The "LibraryBuild" suppressions will continue being used by the linker but will not get analyzer support, and we will use existing mechanisms to suppress warnings in the analyzer (for example #pragmas).