[API proposal]: Unseal DiagnosticDescriptor

[Youssef1313]

Background and Motivation

Currently, the only way to add custom information to a diagnostic descriptor is via custom tags, which is an IEnumerable<string>

Proposed API

namespace Microsoft.CodeAnalysis
{
-     public sealed class DiagnosticDescriptor : IEquatable<DiagnosticDescriptor?>
+     public class DiagnosticDescriptor : IEquatable<DiagnosticDescriptor?>

Usage Examples

The IDE could for example use an internal class IdeDiagnosticDescriptor : DiagnosticDescriptor that has extra information such as fading option, enforce on build values, etc.

Currently enforce on build values are encoded as custom tags. But fading options are set to the whole analyzer.
Regardless of whether the IDE team is interested to do things that way, I can see other 3rd party analyzers making use of custom descriptors.

Alternative Designs

Risks

@mavasani @333fred What do you think?

[Youssef1313]

It wasn't sealed originally, and was sealed in #1065.

[mavasani]

@Youssef1313 I don't think this is possible or even desirable. Descriptors are used to create Diagnostic instances, and Diagnostic.Create supports overloads which take individual fields of the descriptor, and we need to synthesize a Descriptor on the Diagnostic instance from these parameters, which won't be possible if entire content of the Descriptor is not known to the compiler layer (which will happen with your proposal).

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

Lines 295 to 298 in 87bf0d0

	/// <summary>
	/// Gets the diagnostic descriptor, which provides a description about a <see cref="Diagnostic"/>.
	/// </summary>
	public abstract DiagnosticDescriptor Descriptor { get; }

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

Lines 165 to 278 in 87bf0d0

	/// <summary>
	/// Creates a <see cref="Diagnostic"/> instance which is localizable.
	/// </summary>
	/// <param name="id">An identifier for the diagnostic. For diagnostics generated by the compiler, this will be a numeric code with a prefix such as "CS1001".</param>
	/// <param name="category">The category of the diagnostic. For diagnostics generated by the compiler, the category will be "Compiler".</param>
	/// <param name="message">The diagnostic message text.</param>
	/// <param name="severity">The diagnostic's effective severity.</param>
	/// <param name="defaultSeverity">The diagnostic's default severity.</param>
	/// <param name="isEnabledByDefault">True if the diagnostic is enabled by default</param>
	/// <param name="warningLevel">The warning level, greater than 0 if severity is <see cref="DiagnosticSeverity.Warning"/>; otherwise 0.</param>
	/// <param name="title">An optional short localizable title describing the diagnostic.</param>
	/// <param name="description">An optional longer localizable description for the diagnostic.</param>
	/// <param name="helpLink">An optional hyperlink that provides more detailed information regarding the diagnostic.</param>
	/// <param name="location">An optional primary location of the diagnostic. If null, <see cref="Location"/> will return <see cref="Location.None"/>.</param>
	/// <param name="additionalLocations">
	/// An optional set of additional locations related to the diagnostic.
	/// Typically, these are locations of other items referenced in the message.
	/// If null, <see cref="AdditionalLocations"/> will return an empty list.
	/// </param>
	/// <param name="customTags">
	/// An optional set of custom tags for the diagnostic. See <see cref="WellKnownDiagnosticTags"/> for some well known tags.
	/// If null, <see cref="CustomTags"/> will return an empty list.
	/// </param>
	/// <param name="properties">
	/// An optional set of name-value pairs by means of which the analyzer that creates the diagnostic
	/// can convey more detailed information to the fixer. If null, <see cref="Properties"/> will return
	/// <see cref="ImmutableDictionary{TKey, TValue}.Empty"/>.
	/// </param>
	/// <returns>The <see cref="Diagnostic"/> instance.</returns>
	public static Diagnostic Create(
	string id,
	string category,
	LocalizableString message,
	DiagnosticSeverity severity,
	DiagnosticSeverity defaultSeverity,
	bool isEnabledByDefault,
	int warningLevel,
	LocalizableString? title = null,
	LocalizableString? description = null,
	string? helpLink = null,
	Location? location = null,
	IEnumerable<Location>? additionalLocations = null,
	IEnumerable<string>? customTags = null,
	ImmutableDictionary<string, string?>? properties = null)
	{
	return Create(id, category, message, severity, defaultSeverity, isEnabledByDefault, warningLevel, false,
	title, description, helpLink, location, additionalLocations, customTags, properties);
	}
	/// <summary>
	/// Creates a <see cref="Diagnostic"/> instance which is localizable.
	/// </summary>
	/// <param name="id">An identifier for the diagnostic. For diagnostics generated by the compiler, this will be a numeric code with a prefix such as "CS1001".</param>
	/// <param name="category">The category of the diagnostic. For diagnostics generated by the compiler, the category will be "Compiler".</param>
	/// <param name="message">The diagnostic message text.</param>
	/// <param name="severity">The diagnostic's effective severity.</param>
	/// <param name="defaultSeverity">The diagnostic's default severity.</param>
	/// <param name="isEnabledByDefault">True if the diagnostic is enabled by default</param>
	/// <param name="warningLevel">The warning level, greater than 0 if severity is <see cref="DiagnosticSeverity.Warning"/>; otherwise 0.</param>
	/// <param name="isSuppressed">Flag indicating whether the diagnostic is suppressed by a source suppression.</param>
	/// <param name="title">An optional short localizable title describing the diagnostic.</param>
	/// <param name="description">An optional longer localizable description for the diagnostic.</param>
	/// <param name="helpLink">An optional hyperlink that provides more detailed information regarding the diagnostic.</param>
	/// <param name="location">An optional primary location of the diagnostic. If null, <see cref="Location"/> will return <see cref="Location.None"/>.</param>
	/// <param name="additionalLocations">
	/// An optional set of additional locations related to the diagnostic.
	/// Typically, these are locations of other items referenced in the message.
	/// If null, <see cref="AdditionalLocations"/> will return an empty list.
	/// </param>
	/// <param name="customTags">
	/// An optional set of custom tags for the diagnostic. See <see cref="WellKnownDiagnosticTags"/> for some well known tags.
	/// If null, <see cref="CustomTags"/> will return an empty list.
	/// </param>
	/// <param name="properties">
	/// An optional set of name-value pairs by means of which the analyzer that creates the diagnostic
	/// can convey more detailed information to the fixer. If null, <see cref="Properties"/> will return
	/// <see cref="ImmutableDictionary{TKey, TValue}.Empty"/>.
	/// </param>
	/// <returns>The <see cref="Diagnostic"/> instance.</returns>
	public static Diagnostic Create(
	string id,
	string category,
	LocalizableString message,
	DiagnosticSeverity severity,
	DiagnosticSeverity defaultSeverity,
	bool isEnabledByDefault,
	int warningLevel,
	bool isSuppressed,
	LocalizableString? title = null,
	LocalizableString? description = null,
	string? helpLink = null,
	Location? location = null,
	IEnumerable<Location>? additionalLocations = null,
	IEnumerable<string>? customTags = null,
	ImmutableDictionary<string, string?>? properties = null)
	{
	if (id == null)
	{
	throw new ArgumentNullException(nameof(id));
	}
	if (category == null)
	{
	throw new ArgumentNullException(nameof(category));
	}
	if (message == null)
	{
	throw new ArgumentNullException(nameof(message));
	}
	return SimpleDiagnostic.Create(id, title ?? string.Empty, category, message, description ?? string.Empty, helpLink ?? string.Empty,
	severity, defaultSeverity, isEnabledByDefault, warningLevel, location ?? Location.None, additionalLocations, customTags, properties, isSuppressed);
	}

This also ensures that the DiagnosticDescriptor can be round-tripped across processes by all Roslyn layers, including the compiler layer.

[Youssef1313]

Diagnostic.Create supports overloads which take individual fields of the descriptor, and we need to synthesize a Descriptor on the Diagnostic instance from these parameters, which won't be possible if entire content of the Descriptor is not known to the compiler layer (which will happen with your proposal).

When using custom descriptors, we'll use the Diagnostic.Create overload that takes a DiagnosticDescriptor, not those that take the individual fields. Does this cause a problem?