Skip to content

Commit dd15949

Browse files
mhutchmhutch
authored
Merge pull request #2468 from mhutch/untyped-annotation-id
Remove type from annotation ID
2 parents cf68185 + 5c656bc commit dd15949

11 files changed

+163
-50
lines changed

‎src/System.CommandLine.Subsystems/HelpAnnotationExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,6 @@ public static void SetDescription<TSymbol>(this TSymbol symbol, string descripti
4545
/// </remarks>
4646
public static string? GetDescription<TSymbol>(this TSymbol symbol) where TSymbol : CliSymbol
4747
{
48-
return symbol.GetAnnotationOrDefault(HelpAnnotations.Description);
48+
return symbol.GetAnnotationOrDefault<string>(HelpAnnotations.Description);
4949
}
5050
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace System.CommandLine.Subsystems.Annotations;
66
/// <summary>
77
/// Describes the ID and type of an annotation.
88
/// </summary>
9-
public record struct AnnotationId<TValue>(string Prefix, string Id)
9+
public record struct AnnotationId(string Prefix, string Id)
1010
{
1111
public override readonly string ToString() => $"{Prefix}.{Id}";
1212
}

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,16 @@ class AnnotationStorage : IAnnotationProvider
1111
{
1212
record struct AnnotationKey(CliSymbol symbol, string prefix, string id)
1313
{
14-
public static AnnotationKey Create<TAnnotation> (CliSymbol symbol, AnnotationId<TAnnotation> annotationId)
14+
public static AnnotationKey Create (CliSymbol symbol, AnnotationId annotationId)
1515
=> new (symbol, annotationId.Prefix, annotationId.Id);
1616
}
1717

1818
readonly Dictionary<AnnotationKey, object> annotations = [];
1919

20-
public bool TryGet<TValue>(CliSymbol symbol, AnnotationId<TValue> annotationId, [NotNullWhen(true)] out TValue? value)
21-
{
22-
if (annotations.TryGetValue(AnnotationKey.Create(symbol, annotationId), out var obj))
23-
{
24-
value = (TValue)obj;
25-
return true;
26-
}
27-
28-
value = default;
29-
return false;
30-
}
20+
public bool TryGet(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value)
21+
=> annotations.TryGetValue(AnnotationKey.Create(symbol, annotationId), out value);
3122

32-
public void Set<TValue>(CliSymbol symbol, AnnotationId<TValue> annotationId, TValue value)
23+
public void Set(CliSymbol symbol, AnnotationId annotationId, object? value)
3324
{
3425
var key = AnnotationKey.Create(symbol, annotationId);
3526
if (value is not null)

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

Lines changed: 58 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,12 @@ public static partial class AnnotationStorageExtensions
5757
/// <summary>
5858
/// Sets the value for the annotation <paramref name="id"/> associated with the <paramref name="symbol"/> in the internal annotation storage.
5959
/// </summary>
60-
/// <typeparam name="TValue">The type of the annotation value</typeparam>
6160
/// <param name="symbol">The symbol that is annotated</param>
6261
/// <param name="id">
6362
/// The identifier for the annotation. For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
6463
/// </param>
6564
/// <param name="value">The annotation value</param>
66-
public static void SetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<TValue> annotationId, TValue value)
65+
public static void SetAnnotation(this CliSymbol symbol, AnnotationId annotationId, object? value)
6766
{
6867
var storage = symbolToAnnotationStorage.GetValue(symbol, static (CliSymbol _) => new AnnotationStorage());
6968
storage.Set(symbol, annotationId, value);
@@ -73,7 +72,6 @@ public static void SetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<TVa
7372
/// Sets the value for the annotation <paramref name="id"/> associated with the <paramref name="symbol"/> in the internal annotation storage,
7473
/// and returns the <paramref name="symbol"> to enable fluent construction of symbols with annotations.
7574
/// </summary>
76-
/// <typeparam name="TValue">The type of the annotation value</typeparam>
7775
/// <param name="symbol">The symbol that is annotated</param>
7876
/// <param name="id">
7977
/// 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
8280
/// <returns>
8381
/// The <paramref name="symbol">, to enable fluent construction of symbols with annotations.
8482
/// </returns>
85-
public static TSymbol WithAnnotation<TSymbol, TValue>(this TSymbol symbol, AnnotationId<TValue> annotationId, TValue value) where TSymbol : CliSymbol
83+
public static TSymbol WithAnnotation<TSymbol>(this TSymbol symbol, AnnotationId annotationId, object? value) where TSymbol : CliSymbol
8684
{
8785
symbol.SetAnnotation(annotationId, value);
8886
return symbol;
8987
}
9088

9189
/// <summary>
9290
/// Attempts to get the value for the annotation <paramref name="annotationId"/> associated with the <paramref name="symbol"/> in the internal annotation
93-
/// storage used to store values set via <see cref="SetAnnotation{TValue}(CliSymbol, AnnotationId{TValue}, TValue)"/>.
91+
/// storage used to store values set via <see cref="SetAnnotation(CliSymbol, AnnotationId, object?)"/>.
9492
/// </summary>
95-
/// <typeparam name="TValue">The type of the annotation value</typeparam>
93+
/// <typeparam name="TValue">
94+
/// The expected type of the annotation value. If the type does not match, a <see cref="AnnotationTypeException"/> will be thrown.
95+
/// If the annotation allows multiple types for its values, and a type parameter cannot be determined statically,
96+
/// use <see cref="TryGetAnnotation(CliSymbol, AnnotationId, out object?)"/> to access the annotation value without checking its type.
97+
/// </typeparam>
9698
/// <param name="symbol">The symbol that is annotated</param>
9799
/// <param name="annotationId">
98100
/// The identifier for the annotation. For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
99101
/// </param>
100102
/// <param name="value">The annotation value, if successful, otherwise <c>default</c></param>
101103
/// <returns>True if successful</returns>
102104
/// <remarks>
103-
/// This is intended to be called by specialized ID-specific accessors for CLI authors such as <see cref="HelpAnnotationExtensions.GetDescription{TSymbol}(TSymbol)"/>.
104-
/// Subsystems should not call it, as it does not account for values from the subsystem's <see cref="IAnnotationProvider"/>. They should instead call
105-
/// <see cref="CliSubsystem.TryGetAnnotation{TValue}(CliSymbol, AnnotationId{TValue}, out TValue?)"/> or an ID-specific accessor on the subsystem such
105+
/// 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.
106+
/// <para>
107+
/// 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)"/>.
108+
/// </para>
109+
/// <para>
110+
/// Subsystems should not call it directly, as it does not account for values from the subsystem's <see cref="IAnnotationProvider"/>. They should instead call
111+
/// <see cref="CliSubsystem.TryGetAnnotation{TValue}(CliSymbol, AnnotationId, out TValue?)"/> or an ID-specific accessor on the subsystem such
112+
/// <see cref="HelpSubsystem.TryGetDescription(CliSymbol, out string?)"/>.
113+
/// </para>
114+
/// </remarks>
115+
public static bool TryGetAnnotation<TValue>(this CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out TValue? value)
116+
{
117+
if (TryGetAnnotation(symbol, annotationId, out object? rawValue))
118+
{
119+
if (rawValue is TValue expectedTypeValue)
120+
{
121+
value = expectedTypeValue;
122+
return true;
123+
}
124+
throw new AnnotationTypeException(annotationId, typeof(TValue), rawValue?.GetType());
125+
}
126+
127+
value = default;
128+
return false;
129+
}
130+
131+
/// <summary>
132+
/// Attempts to get the value for the annotation <paramref name="annotationId"/> associated with the <paramref name="symbol"/> in the internal annotation
133+
/// storage used to store values set via <see cref="SetAnnotation(CliSymbol, AnnotationId, object?)"/>.
134+
/// </summary>
135+
/// <param name="symbol">The symbol that is annotated</param>
136+
/// <param name="annotationId">
137+
/// The identifier for the annotation. For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
138+
/// </param>
139+
/// <param name="value">The annotation value, if successful, otherwise <c>default</c></param>
140+
/// <returns>True if successful</returns>
141+
/// <remarks>
142+
/// If the expected type of the annotation value is known, use the <see cref="TryGetAnnotation{TValue}(CliSymbol, AnnotationId, out TValue?)"/> overload instead.
143+
/// <para>
144+
/// 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)"/>.
145+
/// </para>
146+
/// <para>
147+
/// Subsystems should not call it directly, as it does not account for values from the subsystem's <see cref="IAnnotationProvider"/>. They should instead call
148+
/// <see cref="CliSubsystem.TryGetAnnotation(CliSymbol, AnnotationId, out object?)"/> or an ID-specific accessor on the subsystem such
106149
/// <see cref="HelpSubsystem.TryGetDescription(CliSymbol, out string?)"/>.
150+
/// </para>
107151
/// </remarks>
108-
public static bool TryGetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<TValue> annotationId, [NotNullWhen(true)] out TValue? value)
152+
public static bool TryGetAnnotation(this CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value)
109153
{
110-
if (symbolToAnnotationStorage.TryGetValue(symbol, out var storage) && storage.TryGet (symbol, annotationId, out value))
154+
if (symbolToAnnotationStorage.TryGetValue(symbol, out var storage) && storage.TryGet(symbol, annotationId, out value))
111155
{
112156
return true;
113157
}
@@ -118,7 +162,7 @@ public static bool TryGetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<
118162

119163
/// <summary>
120164
/// Attempts to get the value for the annotation <paramref name="annotationId"/> associated with the <paramref name="symbol"/> in the internal annotation
121-
/// storage used to store values set via <see cref="SetAnnotation{TValue}(CliSymbol, AnnotationId{TValue}, TValue)"/>.
165+
/// storage used to store values set via <see cref="SetAnnotation{TValue}(CliSymbol, AnnotationId, TValue)"/>.
122166
/// </summary>
123167
/// <typeparam name="TValue">The type of the annotation value</typeparam>
124168
/// <param name="symbol">The symbol that is annotated</param>
@@ -129,10 +173,10 @@ public static bool TryGetAnnotation<TValue>(this CliSymbol symbol, AnnotationId<
129173
/// <remarks>
130174
/// This is intended to be called by specialized ID-specific accessors for CLI authors such as <see cref="HelpAnnotationExtensions.GetDescription{TSymbol}(TSymbol)"/>.
131175
/// Subsystems should not call it, as it does not account for values from the subsystem's <see cref="IAnnotationProvider"/>. They should instead call
132-
/// <see cref="CliSubsystem.TryGetAnnotation{TValue}(CliSymbol, AnnotationId{TValue}, out TValue?)"/> or an ID-specific accessor on the subsystem such
176+
/// <see cref="CliSubsystem.TryGetAnnotation{TValue}(CliSymbol, AnnotationId, out TValue?)"/> or an ID-specific accessor on the subsystem such
133177
/// <see cref="HelpSubsystem.TryGetDescription(CliSymbol, out string?)"/>.
134178
/// </remarks>
135-
public static TValue? GetAnnotationOrDefault<TValue>(this CliSymbol symbol, AnnotationId<TValue> annotationId)
179+
public static TValue? GetAnnotationOrDefault<TValue>(this CliSymbol symbol, AnnotationId annotationId)
136180
{
137181
if (symbol.TryGetAnnotation(annotationId, out TValue? value))
138182
{

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
namespace System.CommandLine.Subsystems.Annotations;
5+
6+
/// <summary>
7+
/// Thrown when an annotation value does not match the expected type for that <see cref="AnnotationId"/>.
8+
/// </summary>
9+
public class AnnotationTypeException(AnnotationId annotationId, Type expectedType, Type? actualType, IAnnotationProvider? provider = null) : Exception
10+
{
11+
public AnnotationId AnnotationId { get; } = annotationId;
12+
public Type ExpectedType { get; } = expectedType;
13+
public Type? ActualType { get; } = actualType;
14+
public IAnnotationProvider? Provider = provider;
15+
16+
public override string Message
17+
{
18+
get
19+
{
20+
if (Provider is not null)
21+
{
22+
return
23+
$"Typed accessor for annotation '${AnnotationId}' expected type '{ExpectedType}' but the annotation provider returned an annotation of type '{ActualType?.ToString() ?? "[null]"}'. " +
24+
$"This may be an authoring error in in the annotation provider '{Provider.GetType()}' or in a typed annotation accessor.";
25+
26+
}
27+
28+
return
29+
$"Typed accessor for annotation '${AnnotationId}' expected type '{ExpectedType}' but the stored annotation is of type '{ActualType?.ToString() ?? "[null]"}'. " +
30+
$"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.";
31+
}
32+
}
33+
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,8 @@ public static class HelpAnnotations
1010
{
1111
public static string Prefix { get; } = nameof(SubsystemKind.Help);
1212

13-
public static AnnotationId<string> Description { get; } = new(Prefix, nameof(Description));
13+
/// <summary>
14+
/// The description of the symbol, as a plain text <see cref="string"/>.
15+
/// </summary>
16+
public static AnnotationId Description { get; } = new(Prefix, nameof(Description));
1417
}

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,20 @@ public static class ValueAnnotations
1414
/// Default value for an option or argument
1515
/// </summary>
1616
/// <remarks>
17-
/// Although the type is <see cref="object?"/>, it must actually be the same type as the type
18-
/// parameter of the <see cref="CliArgument{T}"/> or <see cref="CliOption{T}"/>.
17+
/// Must be actually the same type as the type parameter of
18+
/// the <see cref="CliArgument{T}"/> or <see cref="CliOption{T}"/>.
1919
/// </remarks>
20-
public static AnnotationId<object?> DefaultValue { get; } = new(Prefix, nameof(DefaultValue));
20+
public static AnnotationId DefaultValue { get; } = new(Prefix, nameof(DefaultValue));
2121

2222
/// <summary>
2323
/// Default value calculation for an option or argument
2424
/// </summary>
2525
/// <remarks>
26-
/// Although the type is <see cref="object?"/>, it must actually be a <see cref="Func{TResult}">
27-
/// with a type parameter matching the the type parameter type of the <see cref="CliArgument{T}"/>
28-
/// or <see cref="CliOption{T}"/>
26+
/// Please use the extension methods and do not call this directly.
27+
/// <para>
28+
/// Must return a <see cref="Func{TValue}"> with the same type parameter as
29+
/// the <see cref="CliArgument{T}"/> or <see cref="CliOption{T}"/>.
30+
/// </para>
2931
/// </remarks>
30-
public static AnnotationId<object> DefaultValueCalculation { get; } = new(Prefix, nameof(DefaultValueCalculation));
32+
public static AnnotationId DefaultValueCalculation { get; } = new(Prefix, nameof(DefaultValueCalculation));
3133
}

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

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ protected CliSubsystem(string name, SubsystemKind subsystemKind, IAnnotationProv
3636
/// Attempt to retrieve the <paramref name="symbol"/>'s value for the annotation <paramref name="id"/>. This will check the
3737
/// annotation provider that was passed to the subsystem constructor, and the internal annotation storage.
3838
/// </summary>
39-
/// <typeparam name="TValue">The value of the type to retrieve</typeparam>
39+
/// <typeparam name="TValue">
40+
/// The expected type of the annotation value. If the type does not match, a <see cref="AnnotationTypeException"/> will be thrown.
41+
/// If the annotation allows multiple types for its values, and a type parameter cannot be determined statically,
42+
/// use <see cref="TryGetAnnotation(CliSymbol, AnnotationId, out object?)"/> to access the annotation value without checking its type.
43+
/// </typeparam>
4044
/// <param name="symbol">The symbol the value is attached to</param>
4145
/// <param name="id">
4246
/// The identifier for the annotation value to be retrieved.
@@ -45,10 +49,46 @@ protected CliSubsystem(string name, SubsystemKind subsystemKind, IAnnotationProv
4549
/// <param name="value">An out parameter to contain the result</param>
4650
/// <returns>True if successful</returns>
4751
/// <remarks>
52+
/// 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.
53+
/// <para>
4854
/// Subsystem authors must use this to access annotation values, as it respects the subsystem's <see cref="IAnnotationProvider"/> if it has one.
4955
/// This value is protected because it is intended for use only by subsystem authors. It calls <see cref="AnnotationStorageExtensions"/>
56+
/// </para>
5057
/// </remarks>
51-
protected internal bool TryGetAnnotation<TValue>(CliSymbol symbol, AnnotationId<TValue> annotationId, [NotNullWhen(true)] out TValue? value)
58+
protected internal bool TryGetAnnotation<TValue>(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out TValue? value)
59+
{
60+
if (_annotationProvider is not null && _annotationProvider.TryGet(symbol, annotationId, out object? rawValue))
61+
{
62+
if (rawValue is TValue expectedTypeValue)
63+
{
64+
value = expectedTypeValue;
65+
return true;
66+
}
67+
throw new AnnotationTypeException(annotationId, typeof(TValue), rawValue?.GetType(), _annotationProvider);
68+
}
69+
70+
return symbol.TryGetAnnotation(annotationId, out value);
71+
}
72+
73+
/// <summary>
74+
/// Attempt to retrieve the <paramref name="symbol"/>'s value for the annotation <paramref name="id"/>. This will check the
75+
/// annotation provider that was passed to the subsystem constructor, and the internal annotation storage.
76+
/// </summary>
77+
/// <param name="symbol">The symbol the value is attached to</param>
78+
/// <param name="id">
79+
/// The identifier for the annotation value to be retrieved.
80+
/// For example, the annotation identifier for the help description is <see cref="HelpAnnotations.Description">.
81+
/// </param>
82+
/// <param name="value">An out parameter to contain the result</param>
83+
/// <returns>True if successful</returns>
84+
/// <remarks>
85+
/// If the expected type of the annotation value is known, use the <see cref="TryGetAnnotation{TValue}(CliSymbol, AnnotationId, out TValue?)"/> overload instead.
86+
/// <para>
87+
/// Subsystem authors must use this to access annotation values, as it respects the subsystem's <see cref="IAnnotationProvider"/> if it has one.
88+
/// This value is protected because it is intended for use only by subsystem authors. It calls <see cref="AnnotationStorageExtensions"/>
89+
/// </para>
90+
/// </remarks>
91+
protected internal bool TryGetAnnotation(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value)
5292
{
5393
if (_annotationProvider is not null && _annotationProvider.TryGet(symbol, annotationId, out value))
5494
{

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@ namespace System.CommandLine.Subsystems;
1111
/// </summary>
1212
public interface IAnnotationProvider
1313
{
14-
bool TryGet<TValue>(CliSymbol symbol, AnnotationId<TValue> id, [NotNullWhen(true)] out TValue? value);
14+
bool TryGet(CliSymbol symbol, AnnotationId id, [NotNullWhen(true)] out object? value);
1515
}

0 commit comments

Comments
 (0)