Remove type from annotation ID

The purpose of this was to ensure that accessors used the correct
type for the annotation values. This originally applied to the
AnnotationAccessor<T> helper for symbol.With(help.Description, desc)
but also applied to later manually defined accessors.

This strong typing was not perfect, for example it was possible to
(incorrectly) define two annotations with the same ID and different
types, resulting in uninformative cast exceptions.

It also did not apply cleanly to more advanced cases, such as when
the value was a Func<T> or where the annotation value type was based
on the symbol value type. It is also not unreasonable to want to store
more than one type in an annotataion for othe reasons. All these
required removing the typing by using AnnotationId<object?>.

This miakes things a bit cleaner by removing the strong typing from
the annotation ID and instead adding TryGetAnnotation overloads
that take a type and throw an informative exception on mismatch.

This only affects folks defining new annotations. It does not change
anything for authors of CLIs or authors of subsystems, as they should
to use the strongly typed accessors that should be defined along with
every annotation ID.

Author: mhutch

src/System.CommandLine.Subsystems/HelpAnnotationExtensions.cs

@@ -45,6 +45,6 @@ public static void SetDescription<TSymbol>(this TSymbol symbol, string descripti
     /// </remarks>
     public static string? GetDescription<TSymbol>(this TSymbol symbol) where TSymbol : CliSymbol
     {
-        return symbol.GetAnnotationOrDefault(HelpAnnotations.Description);
+        return symbol.GetAnnotationOrDefault<string>(HelpAnnotations.Description);
     }
 }

src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationId.cs

@@ -6,7 +6,7 @@ namespace System.CommandLine.Subsystems.Annotations;
 /// <summary>
 /// Describes the ID and type of an annotation.
 /// </summary>
-public record struct AnnotationId<TValue>(string Prefix, string Id)
+public record struct AnnotationId(string Prefix, string Id)
 {
     public override readonly string ToString() => $"{Prefix}.{Id}";
 }

src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.AnnotationStorage.cs

@@ -11,25 +11,16 @@ class AnnotationStorage : IAnnotationProvider
     {
         record struct AnnotationKey(CliSymbol symbol, string prefix, string id)
         {
-            public static AnnotationKey Create<TAnnotation> (CliSymbol symbol, AnnotationId<TAnnotation> annotationId)
+            public static AnnotationKey Create (CliSymbol symbol, AnnotationId annotationId)
                 => new (symbol, annotationId.Prefix, annotationId.Id);
         }
 
         readonly Dictionary<AnnotationKey, object> annotations = [];
 
-        public bool TryGet<TValue>(CliSymbol symbol, AnnotationId<TValue> annotationId, [NotNullWhen(true)] out TValue? value)
-        {
-            if (annotations.TryGetValue(AnnotationKey.Create(symbol, annotationId), out var obj))
-            {
-                value = (TValue)obj;
-                return true;
-            }
-
-            value = default;
-            return false;
-        }
+        public bool TryGet(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value)
+            => annotations.TryGetValue(AnnotationKey.Create(symbol, annotationId), out value);
 
-        public void Set<TValue>(CliSymbol symbol, AnnotationId<TValue> annotationId, TValue value)
+        public void Set(CliSymbol symbol, AnnotationId annotationId, object? value)
         {
             var key = AnnotationKey.Create(symbol, annotationId);
             if (value is not null)

src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.cs

@@ -57,13 +57,12 @@ public static partial class AnnotationStorageExtensions
     /// <summary>
     /// Sets the value for the annotation <paramref name="id"/> associated with the <paramref name="symbol"/> in the internal annotation storage.
     /// </summary>
-    /// <typeparam name="TValue">The type of the annotation value</typeparam>
     /// <param name="symbol">The symbol that is annotated</param>
     /// <param name="id">
     /// The identifier for the annotation. For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
     /// </param>
     /// <param name="value">The annotation value</param>
-    public static void SetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<TValue> annotationId, TValue value)
+    public static void SetAnnotation(this CliSymbol symbol, AnnotationId annotationId, object? value)
     {
         var storage = symbolToAnnotationStorage.GetValue(symbol, static (CliSymbol _) => new AnnotationStorage());
         storage.Set(symbol, annotationId, value);
@@ -73,7 +72,6 @@ public static void SetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<TVa
     /// Sets the value for the annotation <paramref name="id"/> associated with the <paramref name="symbol"/> in the internal annotation storage,
     /// and returns the <paramref name="symbol"> to enable fluent construction of symbols with annotations.
     /// </summary>
-    /// <typeparam name="TValue">The type of the annotation value</typeparam>
     /// <param name="symbol">The symbol that is annotated</param>
     /// <param name="id">
     /// The identifier for the annotation. For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
@@ -82,32 +80,78 @@ public static void SetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<TVa
     /// <returns>
     /// The <paramref name="symbol">, to enable fluent construction of symbols with annotations.
     /// </returns>
-    public static TSymbol WithAnnotation<TSymbol, TValue>(this TSymbol symbol, AnnotationId<TValue> annotationId, TValue value) where TSymbol : CliSymbol
+    public static TSymbol WithAnnotation<TSymbol>(this TSymbol symbol, AnnotationId annotationId, object? value) where TSymbol : CliSymbol
     {
         symbol.SetAnnotation(annotationId, value);
         return symbol;
     }
 
     /// <summary>
     /// Attempts to get the value for the annotation <paramref name="annotationId"/> associated with the <paramref name="symbol"/> in the internal annotation
-    /// storage used to store values set via <see cref="SetAnnotation{TValue}(CliSymbol, AnnotationId{TValue}, TValue)"/>.
+    /// storage used to store values set via <see cref="SetAnnotation(CliSymbol, AnnotationId, object?)"/>.
     /// </summary>
-    /// <typeparam name="TValue">The type of the annotation value</typeparam>
+    /// <typeparam name="TValue">
+    /// The expected type of the annotation value. If the type does not match, a <see cref="AnnotationTypeException"/> will be thrown.
+    /// If the annotation allows multiple types for its values, and a type parameter cannot be determined statically,
+    /// use <see cref="TryGetAnnotation(CliSymbol, AnnotationId, out object?)"/> to access the annotation value without checking its type.
+    /// </typeparam>
     /// <param name="symbol">The symbol that is annotated</param>
     /// <param name="annotationId">
     /// The identifier for the annotation. For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
     /// </param>
     /// <param name="value">The annotation value, if successful, otherwise <c>default</c></param>
     /// <returns>True if successful</returns>
     /// <remarks>
-    /// This is intended to be called by specialized ID-specific accessors for CLI authors such as <see cref="HelpAnnotationExtensions.GetDescription{TSymbol}(TSymbol)"/>.
-    /// Subsystems should not call it, as it does not account for values from the subsystem's <see cref="IAnnotationProvider"/>. They should instead call
-    /// <see cref="CliSubsystem.TryGetAnnotation{TValue}(CliSymbol, AnnotationId{TValue}, out TValue?)"/> or an ID-specific accessor on the subsystem such
+    /// If the annotation value does not have a single expected type for this symbol, use the <see cref="TryGetAnnotation(CliSymbol, AnnotationId, out object?)"/> overload instead.
+    /// <para>
+    /// This is intended to be called by the implementation of specialized ID-specific accessors for CLI authors such as <see cref="HelpAnnotationExtensions.GetDescription{TSymbol}(TSymbol)"/>.
+    /// </para>
+    /// <para>
+    /// Subsystems should not call it directly, as it does not account for values from the subsystem's <see cref="IAnnotationProvider"/>. They should instead call
+    /// <see cref="CliSubsystem.TryGetAnnotation{TValue}(CliSymbol, AnnotationId, out TValue?)"/> or an ID-specific accessor on the subsystem such
+    /// <see cref="HelpSubsystem.TryGetDescription(CliSymbol, out string?)"/>.
+    /// </para>
+    /// </remarks>
+    public static bool TryGetAnnotation<TValue>(this CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out TValue? value)
+    {
+        if (TryGetAnnotation(symbol, annotationId, out object? rawValue))
+        {
+            if (rawValue is TValue expectedTypeValue)
+            {
+                value = expectedTypeValue;
+                return true;
+            }
+            throw new AnnotationTypeException(annotationId, typeof(TValue), rawValue?.GetType());
+        }
+
+        value = default;
+        return false;
+    }
+
+    /// <summary>
+    /// Attempts to get the value for the annotation <paramref name="annotationId"/> associated with the <paramref name="symbol"/> in the internal annotation
+    /// storage used to store values set via <see cref="SetAnnotation(CliSymbol, AnnotationId, object?)"/>.
+    /// </summary>
+    /// <param name="symbol">The symbol that is annotated</param>
+    /// <param name="annotationId">
+    /// The identifier for the annotation. For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
+    /// </param>
+    /// <param name="value">The annotation value, if successful, otherwise <c>default</c></param>
+    /// <returns>True if successful</returns>
+    /// <remarks>
+    /// If the expected type of the annotation value is known, use the <see cref="TryGetAnnotation{TValue}(CliSymbol, AnnotationId, out TValue?)"/> overload instead.
+    /// <para>
+    /// This is intended to be called by the implementation of specialized ID-specific accessors for CLI authors such as <see cref="HelpAnnotationExtensions.GetDescription{TSymbol}(TSymbol)"/>.
+    /// </para>
+    /// <para>
+    /// Subsystems should not call it directly, as it does not account for values from the subsystem's <see cref="IAnnotationProvider"/>. They should instead call
+    /// <see cref="CliSubsystem.TryGetAnnotation(CliSymbol, AnnotationId, out object?)"/> or an ID-specific accessor on the subsystem such
     /// <see cref="HelpSubsystem.TryGetDescription(CliSymbol, out string?)"/>.
+    /// </para>
     /// </remarks>
-    public static bool TryGetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<TValue> annotationId, [NotNullWhen(true)] out TValue? value)
+    public static bool TryGetAnnotation(this CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value)
     {
-        if (symbolToAnnotationStorage.TryGetValue(symbol, out var storage) && storage.TryGet (symbol, annotationId, out value))
+        if (symbolToAnnotationStorage.TryGetValue(symbol, out var storage) && storage.TryGet(symbol, annotationId, out value))
         {
             return true;
         }
@@ -118,7 +162,7 @@ public static bool TryGetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<
 
     /// <summary>
     /// Attempts to get the value for the annotation <paramref name="annotationId"/> associated with the <paramref name="symbol"/> in the internal annotation
-    /// storage used to store values set via <see cref="SetAnnotation{TValue}(CliSymbol, AnnotationId{TValue}, TValue)"/>.
+    /// storage used to store values set via <see cref="SetAnnotation{TValue}(CliSymbol, AnnotationId, TValue)"/>.
     /// </summary>
     /// <typeparam name="TValue">The type of the annotation value</typeparam>
     /// <param name="symbol">The symbol that is annotated</param>
@@ -129,10 +173,10 @@ public static bool TryGetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<
     /// <remarks>
     /// This is intended to be called by specialized ID-specific accessors for CLI authors such as <see cref="HelpAnnotationExtensions.GetDescription{TSymbol}(TSymbol)"/>.
     /// Subsystems should not call it, as it does not account for values from the subsystem's <see cref="IAnnotationProvider"/>. They should instead call
-    /// <see cref="CliSubsystem.TryGetAnnotation{TValue}(CliSymbol, AnnotationId{TValue}, out TValue?)"/> or an ID-specific accessor on the subsystem such
+    /// <see cref="CliSubsystem.TryGetAnnotation{TValue}(CliSymbol, AnnotationId, out TValue?)"/> or an ID-specific accessor on the subsystem such
     /// <see cref="HelpSubsystem.TryGetDescription(CliSymbol, out string?)"/>.
     /// </remarks>
-    public static TValue? GetAnnotationOrDefault<TValue>(this CliSymbol symbol, AnnotationId<TValue> annotationId)
+    public static TValue? GetAnnotationOrDefault<TValue>(this CliSymbol symbol, AnnotationId annotationId)
     {
         if (symbol.TryGetAnnotation(annotationId, out TValue? value))
         {

src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationTypeException.cs

@@ -0,0 +1,33 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace System.CommandLine.Subsystems.Annotations;
+
+/// <summary>
+/// Thrown when an annotation value does not match the expected type for that <see cref="AnnotationId"/>.
+/// </summary>
+public class AnnotationTypeException(AnnotationId annotationId, Type expectedType, Type? actualType, IAnnotationProvider? provider = null) : Exception
+{
+    public AnnotationId AnnotationId { get; } = annotationId;
+    public Type ExpectedType { get; } = expectedType;
+    public Type? ActualType { get; } = actualType;
+    public IAnnotationProvider? Provider = provider;
+
+    public override string Message
+    {
+        get
+        {
+            if (Provider is not null)
+            {
+                return
+                    $"Typed accessor for annotation '${AnnotationId}' expected type '{ExpectedType}' but the annotation provider returned an annotation of type '{ActualType?.ToString() ?? "[null]"}'. " +
+                    $"This may be an authoring error in in the annotation provider '{Provider.GetType()}' or in a typed annotation accessor.";
+
+            }
+
+            return
+                $"Typed accessor for annotation '${AnnotationId}' expected type '{ExpectedType}' but the stored annotation is of type '{ActualType?.ToString() ?? "[null]"}'. " +
+                $"This may be an authoring error in a typed annotation accessor, or the annotation may have be stored directly with the incorrect type, bypassing the typed accessors.";
+        }
+    }
+}

src/System.CommandLine.Subsystems/Subsystems/Annotations/HelpAnnotations.cs

@@ -10,5 +10,8 @@ public static class HelpAnnotations
 {
     public static string Prefix { get; } = nameof(SubsystemKind.Help);
 
-    public static AnnotationId<string> Description { get; } = new(Prefix, nameof(Description));
+    /// <summary>
+    /// The description of the symbol, as a plain text <see cref="string"/>.
+    /// </summary>
+    public static AnnotationId Description { get; } = new(Prefix, nameof(Description));
 }

src/System.CommandLine.Subsystems/Subsystems/Annotations/ValueAnnotations.cs

@@ -14,18 +14,20 @@ public static class ValueAnnotations
     /// Default value for an option or argument
     /// </summary>
     /// <remarks>
-    /// Although the type is <see cref="object?"/>, it must actually be the same type as the type
-    /// parameter of the <see cref="CliArgument{T}"/> or <see cref="CliOption{T}"/>.
+    /// Must be actually the same type as the type parameter of
+    /// the <see cref="CliArgument{T}"/> or <see cref="CliOption{T}"/>.
     /// </remarks>
-    public static AnnotationId<object?> DefaultValue { get; } = new(Prefix, nameof(DefaultValue));
+    public static AnnotationId DefaultValue { get; } = new(Prefix, nameof(DefaultValue));
 
     /// <summary>
     /// Default value calculation for an option or argument
     /// </summary>
     /// <remarks>
-    /// Although the type is <see cref="object?"/>, it must actually be a <see cref="Func{TResult}">
-    /// with a type parameter matching the the type parameter type of the <see cref="CliArgument{T}"/>
-    /// or <see cref="CliOption{T}"/>
+    /// Please use the extension methods and do not call this directly.
+    /// <para>
+    /// Must return a <see cref="Func{TValue}"> with the same type parameter as
+    /// the <see cref="CliArgument{T}"/> or <see cref="CliOption{T}"/>.
+    /// </para>
     /// </remarks>
-    public static AnnotationId<object> DefaultValueCalculation { get; } = new(Prefix, nameof(DefaultValueCalculation));
+    public static AnnotationId DefaultValueCalculation { get; } = new(Prefix, nameof(DefaultValueCalculation));
 }

src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs

@@ -36,7 +36,11 @@ protected CliSubsystem(string name, SubsystemKind subsystemKind, IAnnotationProv
     /// Attempt to retrieve the <paramref name="symbol"/>'s value for the annotation <paramref name="id"/>. This will check the
     /// annotation provider that was passed to the subsystem constructor, and the internal annotation storage.
     /// </summary>
-    /// <typeparam name="TValue">The value of the type to retrieve</typeparam>
+    /// <typeparam name="TValue">
+    /// The expected type of the annotation value. If the type does not match, a <see cref="AnnotationTypeException"/> will be thrown.
+    /// If the annotation allows multiple types for its values, and a type parameter cannot be determined statically,
+    /// use <see cref="TryGetAnnotation(CliSymbol, AnnotationId, out object?)"/> to access the annotation value without checking its type.
+    /// </typeparam>
     /// <param name="symbol">The symbol the value is attached to</param>
     /// <param name="id">
     /// The identifier for the annotation value to be retrieved.
@@ -45,10 +49,46 @@ protected CliSubsystem(string name, SubsystemKind subsystemKind, IAnnotationProv
     /// <param name="value">An out parameter to contain the result</param>
     /// <returns>True if successful</returns>
     /// <remarks>
+    /// If the annotation value does not have a single expected type for this symbol, use the <see cref="TryGetAnnotation(CliSymbol, AnnotationId, out object?)"/> overload instead.
+    /// <para>
     /// Subsystem authors must use this to access annotation values, as it respects the subsystem's <see cref="IAnnotationProvider"/> if it has one.
     /// This value is protected because it is intended for use only by subsystem authors. It calls <see cref="AnnotationStorageExtensions"/>
+    /// </para>
     /// </remarks>
-    protected internal bool TryGetAnnotation<TValue>(CliSymbol symbol, AnnotationId<TValue> annotationId, [NotNullWhen(true)] out TValue? value)
+    protected internal bool TryGetAnnotation<TValue>(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out TValue? value)
+    {
+        if (_annotationProvider is not null && _annotationProvider.TryGet(symbol, annotationId, out object? rawValue))
+        {
+            if (rawValue is TValue expectedTypeValue)
+            {
+                value = expectedTypeValue;
+                return true;
+            }
+            throw new AnnotationTypeException(annotationId, typeof(TValue), rawValue?.GetType(), _annotationProvider);
+        }
+
+        return symbol.TryGetAnnotation(annotationId, out value);
+    }
+
+    /// <summary>
+    /// Attempt to retrieve the <paramref name="symbol"/>'s value for the annotation <paramref name="id"/>. This will check the
+    /// annotation provider that was passed to the subsystem constructor, and the internal annotation storage.
+    /// </summary>
+    /// <param name="symbol">The symbol the value is attached to</param>
+    /// <param name="id">
+    /// The identifier for the annotation value to be retrieved.
+    /// For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
+    /// </param>
+    /// <param name="value">An out parameter to contain the result</param>
+    /// <returns>True if successful</returns>
+    /// <remarks>
+    /// If the expected type of the annotation value is known, use the <see cref="TryGetAnnotation{TValue}(CliSymbol, AnnotationId, out TValue?)"/> overload instead.
+    /// <para>
+    /// Subsystem authors must use this to access annotation values, as it respects the subsystem's <see cref="IAnnotationProvider"/> if it has one.
+    /// This value is protected because it is intended for use only by subsystem authors. It calls <see cref="AnnotationStorageExtensions"/>
+    /// </para>
+    /// </remarks>
+    protected internal bool TryGetAnnotation(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value)
     {
         if (_annotationProvider is not null && _annotationProvider.TryGet(symbol, annotationId, out value))
         {

src/System.CommandLine.Subsystems/Subsystems/IAnnotationProvider.cs

@@ -11,5 +11,5 @@ namespace System.CommandLine.Subsystems;
 /// </summary>
 public interface IAnnotationProvider
 {
-    bool TryGet<TValue>(CliSymbol symbol, AnnotationId<TValue> id, [NotNullWhen(true)] out TValue? value);
+    bool TryGet(CliSymbol symbol, AnnotationId id, [NotNullWhen(true)] out object? value);
 }