Make ProgrammaticSuppressionInfo public

[mavasani]

Background and Motivation

We added support for DiagnosticSuppressor API some time back. This work added a new internal type ProgrammaticSuppressionInfo and an internal property Diagnostic.ProgrammaticSuppressionInfo to the compiler layer. This issue proposes making these two APIs public, without any changes or additions to the implementation with one minor change to the existing internal API shape: change the type of ProgrammaticSuppressionInfo.Suppressions property from ImmutableHashSet<(string Id, LocalizableString Justification)> to ImmutableArray<Suppression>. See existing definition of public type Suppression here

Motivation:

1. Testing support: From @sharwell:

   We need this public API for DiagnosticSuppressor unit testing support in the Microsoft.CodeAnalysis.Testing SDK. We tried two other approaches using public APIs but they caused problems for various edge cases. The library now uses the current property via reflection, which is fragile.

2. Tooling support: Tools built on top of Microsoft.CodeAnalysis that require to read/operate on the programmatic suppressions from DiagnosticSuppressors would need this public API.

Proposed API

Existing internal type ProgrammaticSuppressionInfo to be made public:

namespace Microsoft.CodeAnalysis.Diagnostics
{
+     public sealed class ProgrammaticSuppressionInfo : IEquatable<ProgrammaticSuppressionInfo?>
+     {
+        // NOTE: The below property is currently implemented in Roslyn with type `ImmutableHashSet<(string Id, LocalizableString Justification)>`, which might not be best suited for a public API.
+        //       Also note that this is proposed as an array and not a single Suppression instance because multiple diagnostic suppressors can suppress the same diagnostic. For most real world cases, the below array will have a single element.
+        public ImmutableArray<Suppression> Suppressions { get; }

+        public bool Equals(ProgrammaticSuppressionInfo? other);
+        public overrides bool Equals(object? other);
+        public overrides int GetHashCode();
+     }
}

Existing internal property Diagnostic.ProgrammaticSuppressionInfo to be made public:

namespace Microsoft.CodeAnalysis
{
      public abstract class Diagnostic
      {
+        public virtual ProgrammaticSuppressionInfo? ProgrammaticSuppressionInfo { get; };
      }
}

NOTE: Diagnostic type has internal abstract methods, and hence cannot be sub-typed outside the compiler layer:

roslyn/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs

Lines 448 to 461 in 86156fa

	/// <summary>
	/// Create a new instance of this diagnostic with the Location property changed.
	/// </summary>
	internal abstract Diagnostic WithLocation(Location location);
	/// <summary>
	/// Create a new instance of this diagnostic with the Severity property changed.
	/// </summary>
	internal abstract Diagnostic WithSeverity(DiagnosticSeverity severity);
	/// <summary>
	/// Create a new instance of this diagnostic with the suppression info changed.
	/// </summary>
	internal abstract Diagnostic WithIsSuppressed(bool isSuppressed);

[mavasani]

@sharwell @333fred

[sharwell]

Also note this API has existed in its current form for a long time. When making it public, it would benefit consumers of the API if we avoid making any changes to any names or return values.

[mavasani]

Also note this API has existed in its current form for a long time. When making it public, it would benefit consumers of the API if we avoid making any changes to any names or return values.

I think this should be possible for most of the existing API shape. The only thing that needs some discussion is the type of Suppressions property:

roslyn/src/Compilers/Core/Portable/Diagnostic/ProgrammaticSuppressionInfo.cs

Line 15 in eee6243

public ImmutableHashSet<(string Id, LocalizableString Justification)> Suppressions { get; }

Currently, it is an immutable hash set of tuple values. I believe we can consider following alternatives:

• Change the property type to be an ImmutableArray instead of ImmutableHashSet
• Introduce a new public struct type, say ProgrammaticSuppresion with 2 fields Putting more thought, we don't need to introduce a new type here, but instead can expose an immutable array of existing Suppression type itself

Basically:

public sealed class ProgrammaticSuppressionInfo : IEquatable<ProgrammaticSuppressionInfo?>
{
    public ImmutableArray<Suppression> Suppressions { get; }
    ...
}

I'll add this proposal to the original issue description

[333fred]

API Review

• Can we move Suppressions to SuppressionInfo instead and not have ProgrammaticSuppressionInfo at all?
  • Let's see if we can consolidate into SuppressionInfo. Passing a Compilation might be a problem?

Conclusion: See if we can do that move, and send email to the api review alias to follow up with the results. If it can't be done, we can make sure we like the names proposed and then approve over email.