[API Compat] Problematic developer experience of compatibility suppressions

[bitbonk]

Is your feature request related to a problem? Please describe.

The package validation of the .NET SDK allows me to selectively accept/ignore a breaking changes using CompatibilitySuppressions.xml files.

Unfortunately, for a human being, this file is very hard to parse and almost impossible to write.
This has the following problems during daily development of libraries:

1. When faced with a PR that contains new entries in CompatibilitySuppressions.xml it is very hard to understand what the breaking change actually is. The way we currently often examine these suppression in a code review is, we checkout the PR branch locally, temporarily remove the new suppression entries and then pack the projects in question, so we can see a human readable error message. so instead of the somewhat incomprehensible suppression

     <Suppression>
       <DiagnosticId>CP0008</DiagnosticId>
       <Target>T:Foo.Bar.MyClass</Target>
       <Left>lib/net8.0/MyAssembly.dll</Left>
       <Right>lib/net8.0/MyAssembly.dll</Right>
       <IsBaselineSuppression>true</IsBaselineSuppression>
    </Suppression>

   the reviewer now has the proper error message again:
   error CP0008: Type 'Foo.Bar.MyClass' does not implement interface 'Foo.Bar.IMyInterface' on lib/net8.0/MyAssembly.dll but it does on [Baseline] lib/net8.0/MyAssembly.dll

2. There is no good way for the PR author to document in the source code, why this particular breaking change is acceptable here.
   Adding an XML comment into the suppression file would be overwritten when the suppression file is regenerated the next time a new breaking change is accepted.

     <Suppression>
       <!--  This is OK here, because .... -->
       <DiagnosticId>CP0008</DiagnosticId>
       <Target>T:Foo.Bar.MyClass</Target>
       <Left>lib/net8.0/MyAssembly.dll</Left>
       <Right>lib/net8.0/MyAssembly.dll</Right>
       <IsBaselineSuppression>true</IsBaselineSuppression>
    </Suppression>

Describe the solution you'd like

1. Also include the actual error message right in the supression entry. This would solve problem 1. Something similar like the example below

     <Suppression>
       <DiagnosticId>CP0008</DiagnosticId>
       <DiagnosticMessage>Type 'Foo.Bar.MyClass' does not implement interface 'Foo.Bar.IMyInterface' on lib/net8.0/MyAssembly.dll but it does on [Baseline] lib/net8.0/MyAssembly.dll</DiagnosticMessage>
       <Target>T:Foo.Bar.MyClass</Target>
       <Left>lib/net8.0/MyAssembly.dll</Left>
       <Right>lib/net8.0/MyAssembly.dll</Right>
       <IsBaselineSuppression>true</IsBaselineSuppression>
    </Suppression>

2. When regenerating the CompatibilitySuppressions.xml file, leave all existing modifications to that file unchanged as long as it does not change the sematics of the supressions. This could solve problem 1 and 2 because we can now add helpful comments to the suppressions, like this:

     <Suppression>
       <!--  Type 'Foo.Bar.MyClass' does not implement interface 'Foo.Bar.IMyInterface' on lib/net8.0/MyAssembly.dll but it does on [Baseline] lib/net8.0/MyAssembly.dll -->
       <!--  This is OK here, because .... -->
       <DiagnosticId>CP0008</DiagnosticId>
       <Target>T:Foo.Bar.MyClass</Target>
       <Left>lib/net8.0/MyAssembly.dll</Left>
       <Right>lib/net8.0/MyAssembly.dll</Right>
       <IsBaselineSuppression>true</IsBaselineSuppression>
    </Suppression>

[dotnet-policy-service]

@dotnet/area-infrastructure-libraries a new issue has been filed in the ApiCompat area, please triage

[ViktorHofer]

Agreed, we aren't happy with the readiability of suppression files either. Thank you for filing this issue.

We discussed adding diagnostic messages to the suppression file. There are some problems in doing so:

1. The suppression file becomes localization specific (even if the diagnostic message is ignored during the equality check, the file itself would still be language specific).
2. Suppression files would become significantly larger in size.

Regarding your second point: Comments that are manually added to the suppression file really should be preserved when re-generating the suppression file. That's not the case today and we treat that as a bug. I might look into this when I have some time. We use XmlSerializer underneath which doesn't support this out-of-the box:

sdk/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Logging/SuppressionEngine.cs

Line 34 in 818246b

XmlSerializer serializer = CreateXmlSerializer();

[bitbonk]

@ViktorHofer

The suppression file becomes localization specific (even if the diagnostic message is ignored during the equality check, the file itself would still be language specific).

Yes, that would indeed be problematic, especially when you work in a multi language team, each regeneration of the suppression file might change the messages language.

But would it be possible to always add the english version of the message (NeutralResourcesLanguage or something)? I think this would be good enough or at least better than nothing.

I am not sure why the file size might matter.