Centralize annotation resolution and allow multivalued annotations

src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs

@@ -30,7 +30,7 @@ public VersionThatUsesHelpData(CliSymbol symbol)
 
             public override void Execute(PipelineResult pipelineResult)
             {
-                TryGetAnnotation(Symbol, HelpAnnotations.Description, out string? description);
+                pipelineResult.Annotations.TryGet(Symbol, HelpAnnotations.Description, out string? description);
                 pipelineResult.ConsoleHack.WriteLine(description);
                 pipelineResult.AlreadyHandled = true;
                 pipelineResult.SetSuccess();
@@ -63,12 +63,12 @@ protected internal override void TearDown(PipelineResult pipelineResult)
             }
         }
 
-        internal class StringDirectiveSubsystem(IAnnotationProvider? annotationProvider = null)
-           : DirectiveSubsystem("other", SubsystemKind.Diagram, annotationProvider)
+        internal class StringDirectiveSubsystem()
+           : DirectiveSubsystem("other", SubsystemKind.Diagram)
         { }
 
-        internal class BooleanDirectiveSubsystem(IAnnotationProvider? annotationProvider = null)
-           : DirectiveSubsystem("diagram", SubsystemKind.Diagram, annotationProvider)
+        internal class BooleanDirectiveSubsystem()
+           : DirectiveSubsystem("diagram", SubsystemKind.Diagram)
         { }
 
     }

src/System.CommandLine.Subsystems.Tests/ValidationSubsystemTests.cs

@@ -11,18 +11,18 @@ namespace System.CommandLine.Subsystems.Tests;
 public class ValidationSubsystemTests
 {
     // Running exactly the same code is important here because missing a step will result in a false positive. Ask me how I know
-    private CliOption GetOptionWithSimpleRange<T>(T lowerBound, T upperBound)
+    private CliOption<T> GetOptionWithSimpleRange<T>(string name, T lowerBound, T upperBound)
         where T : IComparable<T>
     {
-        var option = new CliOption<int>("--intOpt");
+        var option = new CliOption<T>(name);
         option.SetRange(lowerBound, upperBound);
         return option;
     }
 
-    private CliOption GetOptionWithRangeBounds<T>(ValueSource<T> lowerBound, ValueSource<T> upperBound)
+    private CliOption<T> GetOptionWithRangeBounds<T>(string name, ValueSource<T> lowerBound, ValueSource<T> upperBound)
         where T : IComparable<T>
     {
-        var option = new CliOption<int>("--intOpt");
+        var option = new CliOption<T>(name);
         option.SetRange(lowerBound, upperBound);
         return option;
     }
@@ -45,7 +45,7 @@ private PipelineResult ExecutedPipelineResultForCommand(CliCommand command, stri
     [Fact]
     public void Int_values_in_specified_range_do_not_have_errors()
     {
-        var option = GetOptionWithSimpleRange(0, 50);
+        var option = GetOptionWithSimpleRange("--intOpt", 0, 50);
 
         var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
 
@@ -56,7 +56,7 @@ public void Int_values_in_specified_range_do_not_have_errors()
     [Fact]
     public void Int_values_above_upper_bound_report_error()
     {
-        var option = GetOptionWithSimpleRange(0, 5);
+        var option = GetOptionWithSimpleRange("--intOpt", 0, 5);
 
         var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
 
@@ -69,7 +69,7 @@ public void Int_values_above_upper_bound_report_error()
     [Fact]
     public void Int_below_lower_bound_report_error()
     {
-        var option = GetOptionWithSimpleRange(0, 5);
+        var option = GetOptionWithSimpleRange("--intOpt", 0, 5);
 
         var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt -42");
 
@@ -82,7 +82,7 @@ public void Int_below_lower_bound_report_error()
     [Fact]
     public void Int_values_on_lower_range_bound_do_not_report_error()
     {
-        var option = GetOptionWithSimpleRange(42, 50);
+        var option = GetOptionWithSimpleRange("--intOpt", 42, 50);
 
         var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
 
@@ -93,7 +93,7 @@ public void Int_values_on_lower_range_bound_do_not_report_error()
     [Fact]
     public void Int_values_on_upper_range_bound_do_not_report_error()
     {
-        var option = GetOptionWithSimpleRange(0, 42);
+        var option = GetOptionWithSimpleRange("--intOpt", 0, 42);
 
         var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
 
@@ -104,7 +104,7 @@ public void Int_values_on_upper_range_bound_do_not_report_error()
     [Fact]
     public void Values_below_calculated_lower_bound_report_error()
     {
-        var option = GetOptionWithRangeBounds<int>(ValueSource.Create(() => (true, 1)), 50);
+        var option = GetOptionWithRangeBounds<int>("--intOpt", ValueSource.Create(() => (true, 1)), 50);
 
         var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 0");
 
@@ -118,7 +118,7 @@ public void Values_below_calculated_lower_bound_report_error()
     [Fact]
     public void Values_within_calculated_range_do_not_report_error()
     {
-        var option = GetOptionWithRangeBounds(ValueSource<int>.Create(() => (true, 1)), ValueSource<int>.Create(() => (true, 50)));
+        var option = GetOptionWithRangeBounds("--intOpt", ValueSource.Create(() => (true, 1)), ValueSource.Create(() => (true, 50)));
 
         var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
 
@@ -129,7 +129,7 @@ public void Values_within_calculated_range_do_not_report_error()
     [Fact]
     public void Values_above_calculated_upper_bound_report_error()
     {
-        var option = GetOptionWithRangeBounds(0, ValueSource<int>.Create(() => (true, 40)));
+        var option = GetOptionWithRangeBounds("--intOpt", 0, ValueSource.Create(() => (true, 40)));
 
         var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
 
@@ -143,7 +143,7 @@ public void Values_above_calculated_upper_bound_report_error()
     public void Values_below_relative_lower_bound_report_error()
     {
         var otherOption = new CliOption<int>("-a");
-        var option = GetOptionWithRangeBounds(ValueSource.Create(otherOption, o => (true, (int)o + 1)), 50);
+        var option = GetOptionWithRangeBounds("--intOpt", ValueSource.Create<int>(otherOption, o => (true, o + 1)), 50);
         var command = new CliCommand("cmd") { option, otherOption };
 
         var pipelineResult = ExecutedPipelineResultForCommand(command, "--intOpt 0 -a 0");
@@ -159,7 +159,9 @@ public void Values_below_relative_lower_bound_report_error()
     public void Values_within_relative_range_do_not_report_error()
     {
         var otherOption = new CliOption<int>("-a");
-        var option = GetOptionWithRangeBounds(ValueSource<int>.Create(otherOption, o => (true, (int)o + 1)), ValueSource<int>.Create(otherOption, o => (true, (int)o + 10)));
+        var option = GetOptionWithRangeBounds("--intOpt",
+            ValueSource.Create<int>(otherOption, o => (true, o + 1)),
+            ValueSource.Create<int>(otherOption, o => (true, o + 10)));
         var command = new CliCommand("cmd") { option, otherOption };
 
         var pipelineResult = ExecutedPipelineResultForCommand(command, "--intOpt 11 -a 3");
@@ -172,7 +174,7 @@ public void Values_within_relative_range_do_not_report_error()
     public void Values_above_relative_upper_bound_report_error()
     {
         var otherOption = new CliOption<int>("-a");
-        var option = GetOptionWithRangeBounds(0, ValueSource<int>.Create(otherOption, o => (true, (int)o + 10)));
+        var option = GetOptionWithRangeBounds("--intOpt", 0, ValueSource.Create<int>(otherOption, o => (true, o + 10)));
         var command = new CliCommand("cmd") { option, otherOption };
 
         var pipelineResult = ExecutedPipelineResultForCommand(command, "--intOpt 9 -a -2");

src/System.CommandLine.Subsystems/CompletionSubsystem.cs

@@ -17,8 +17,8 @@ namespace System.CommandLine;
 
 public class CompletionSubsystem : CliSubsystem
 {
-    public CompletionSubsystem(IAnnotationProvider? annotationProvider = null)
-        : base(CompletionAnnotations.Prefix, SubsystemKind.Completion, annotationProvider)
+    public CompletionSubsystem()
+        : base(CompletionAnnotations.Prefix, SubsystemKind.Completion)
     { }
 
     // TODO: Figure out trigger for completions

src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs

@@ -6,8 +6,8 @@
 
 namespace System.CommandLine.Directives;
 
-public class DiagramSubsystem(IAnnotationProvider? annotationProvider = null)
-    : DirectiveSubsystem("diagram", SubsystemKind.Diagram, annotationProvider)
+public class DiagramSubsystem()
+    : DirectiveSubsystem("diagram", SubsystemKind.Diagram)
 {
     //protected internal override bool GetIsActivated(ParseResult? parseResult)
     //   => parseResult is not null && option is not null && parseResult.GetValue(option);

src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs

@@ -13,8 +13,8 @@ public abstract class DirectiveSubsystem : CliSubsystem
     public string Id { get; }
     public Location? Location { get; private set; }
 
-    public DirectiveSubsystem(string name, SubsystemKind kind, IAnnotationProvider? annotationProvider = null, string? id = null)
-        : base(name, kind, annotationProvider: annotationProvider)
+    public DirectiveSubsystem(string name, SubsystemKind kind, string? id = null)
+        : base(name, kind)
     {
         Id = id ?? name;
     }

src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs

@@ -7,7 +7,7 @@
 namespace System.CommandLine.Directives;
 
 public class ResponseSubsystem()
-    : CliSubsystem("Response", SubsystemKind.Response, null)
+    : CliSubsystem("Response", SubsystemKind.Response)
 {
     public bool Enabled { get; set; }
 

src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs

@@ -15,8 +15,8 @@ namespace System.CommandLine;
 /// </remarks>
 public class ErrorReportingSubsystem : CliSubsystem
 {
-    public ErrorReportingSubsystem(IAnnotationProvider? annotationProvider = null)
-        : base(ErrorReportingAnnotations.Prefix, SubsystemKind.ErrorReporting, annotationProvider)
+    public ErrorReportingSubsystem()
+        : base(ErrorReportingAnnotations.Prefix, SubsystemKind.ErrorReporting)
     { }
 
     protected internal override bool GetIsActivated(ParseResult? parseResult)

src/System.CommandLine.Subsystems/HelpAnnotationExtensions.cs

@@ -40,11 +40,28 @@ public static void SetDescription<TSymbol>(this TSymbol symbol, string descripti
     /// <param name="symbol">The symbol</param>
     /// <returns>The symbol description if any, otherwise <see langword="null"/></returns>
     /// <remarks>
-    /// This is intended to be called by CLI authors. Subsystems should instead call <see cref="HelpSubsystem.TryGetDescription(CliSymbol, out string?)"/>,
-    /// values from the subsystem's <see cref="IAnnotationProvider"/>.
+    /// This is intended to be called by CLI authors. Subsystem authors should instead call
+    /// <see cref="GetDescription{TSymbol}(AnnotationResolver, TSymbol)"/> to get values from
+    /// the pipeline's <see cref="Pipeline.Annotations"/>, which takes dynamic providers into account.
     /// </remarks>
     public static string? GetDescription<TSymbol>(this TSymbol symbol) where TSymbol : CliSymbol
     {
         return symbol.GetAnnotationOrDefault<string>(HelpAnnotations.Description);
     }
+
+    /// <summary>
+    /// Get the help description for the <paramref name="symbol"/> from the <paramref name="resolver"/>,
+    /// which takes dynamic providers into account.
+    /// </summary>
+    /// <typeparam name="TSymbol">The type of the symbol</typeparam>
+    /// <param name="symbol">The symbol</param>
+    /// <returns>The symbol description if any, otherwise <see langword="null"/></returns>
+    /// <remarks>
+    /// This is intended to be called by subsystem authors. CLI authors should instead call
+    /// <see cref="GetDescription{TSymbol}(TSymbol)"/> to get the value associated directly with the symbol.
+    /// </remarks>
+    public static string? GetDescription<TSymbol>(this AnnotationResolver resolver, TSymbol symbol) where TSymbol : CliSymbol
+    {
+        return resolver.GetAnnotationOrDefault<string>(symbol, HelpAnnotations.Description);
+    }
 }

src/System.CommandLine.Subsystems/HelpSubsystem.cs

@@ -15,8 +15,8 @@ namespace System.CommandLine;
 //        var command = new CliCommand("greet")
 //          .With(help.Description, "Greet the user");
 //
-public class HelpSubsystem(IAnnotationProvider? annotationProvider = null)
-    : CliSubsystem(HelpAnnotations.Prefix, SubsystemKind.Help, annotationProvider)
+public class HelpSubsystem()
+    : CliSubsystem(HelpAnnotations.Prefix, SubsystemKind.Help)
 {
     /// <summary>
     /// Gets the help option, which allows the user to customize
@@ -44,7 +44,4 @@ public override void Execute(PipelineResult pipelineResult)
         pipelineResult.ConsoleHack.WriteLine("Help me!");
         pipelineResult.SetSuccess();
     }
-
-    public bool TryGetDescription(CliSymbol symbol, out string? description)
-        => TryGetAnnotation(symbol, HelpAnnotations.Description, out description);
 }

src/System.CommandLine.Subsystems/InvocationSubsystem.cs

@@ -6,6 +6,6 @@
 
 namespace System.CommandLine;
 
-public class InvocationSubsystem(IAnnotationProvider? annotationProvider = null)
-    : CliSubsystem(InvocationAnnotations.Prefix, SubsystemKind.Invocation, annotationProvider)
+public class InvocationSubsystem()
+    : CliSubsystem(InvocationAnnotations.Prefix, SubsystemKind.Invocation)
 {}

src/System.CommandLine.Subsystems/Pipeline.cs

@@ -4,6 +4,7 @@
 using System.CommandLine.Directives;
 using System.CommandLine.Parsing;
 using System.CommandLine.Subsystems;
+using System.CommandLine.Subsystems.Annotations;
 
 namespace System.CommandLine;
 
@@ -17,6 +18,7 @@ public partial class Pipeline
     private readonly PipelinePhase<InvocationSubsystem> invocationPhase = new(SubsystemKind.Invocation);
     private readonly PipelinePhase<ErrorReportingSubsystem> errorReportingPhase = new(SubsystemKind.ErrorReporting);
     private readonly IEnumerable<PipelinePhase> phases;
+    private readonly List<IAnnotationProvider> annotationProviders;
 
     /// <summary>
     /// Creates an instance of the pipeline using standard features.
@@ -34,14 +36,15 @@ public static Pipeline Create(HelpSubsystem? help = null,
                                   VersionSubsystem? version = null,
                                   CompletionSubsystem? completion = null,
                                   DiagramSubsystem? diagram = null,
-                                  ErrorReportingSubsystem? errorReporting = null)
-        => new()
+                                  ErrorReportingSubsystem? errorReporting = null,
+                                  IEnumerable<IAnnotationProvider>? annotationProviders = null)
+        => new(annotationProviders)
         {
             Help = help ?? new HelpSubsystem(),
             Version = version ?? new VersionSubsystem(),
             Completion = completion ?? new CompletionSubsystem(),
             Diagram = diagram ?? new DiagramSubsystem(),
-            ErrorReporting = errorReporting ?? new ErrorReportingSubsystem(),
+            ErrorReporting = errorReporting ?? new ErrorReportingSubsystem()
         };
 
     /// <summary>
@@ -54,7 +57,7 @@ public static Pipeline Create(HelpSubsystem? help = null,
     public static Pipeline CreateEmpty()
         => new();
 
-    private Pipeline()
+    private Pipeline(IEnumerable<IAnnotationProvider>? annotationProviders = null)
     {
         Response = new ResponseSubsystem();
         Invocation = new InvocationSubsystem();
@@ -68,6 +71,10 @@ private Pipeline()
             diagramPhase, completionPhase, helpPhase, versionPhase,
             validationPhase, invocationPhase, errorReportingPhase
         ];
+
+        this.annotationProviders = annotationProviders is not null
+            ? [..annotationProviders]
+            : [];
     }
 
     /// <summary>
@@ -186,6 +193,16 @@ public ErrorReportingSubsystem? ErrorReporting
     /// </summary>
     public ResponseSubsystem Response { get; }
 
+    /// <summary>
+    /// Gets the list of annotation providers registered with the pipeline
+    /// </summary>
+    public IReadOnlyList<IAnnotationProvider> AnnotationProviders => annotationProviders;
+
+    /// <summary>
+    /// Adds an annotation provider to the pipeline
+    /// </summary>
+    public void AddAnnotationProvider(IAnnotationProvider annotationProvider) => annotationProviders.Add(annotationProvider);
+
     public ParseResult Parse(CliConfiguration configuration, string rawInput)
         => Parse(configuration, CliParser.SplitCommandLine(rawInput).ToArray());
 

src/System.CommandLine.Subsystems/PipelineResult.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.CommandLine.Parsing;
+using System.CommandLine.Subsystems.Annotations;
 
 namespace System.CommandLine;
 
@@ -18,6 +19,7 @@ public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeli
         Pipeline = pipeline ?? Pipeline.CreateEmpty();
         ConsoleHack = consoleHack ?? new ConsoleHack();
         valueProvider = new ValueProvider(this);
+        Annotations = new AnnotationResolver(this);
     }
 
     public ParseResult ParseResult { get; }
@@ -27,6 +29,8 @@ public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeli
     public Pipeline Pipeline { get; }
     public ConsoleHack ConsoleHack { get; }
 
+    public AnnotationResolver Annotations { get; }
+
     public bool AlreadyHandled { get; set; }
     public int ExitCode { get; set; }
 

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

@@ -0,0 +1,34 @@
+// 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 collection value does not match the expected type for that <see cref="AnnotationId"/>.
+/// </summary>
+public class AnnotationCollectionTypeException(AnnotationId annotationId, Type expectedType, Type? actualType, IAnnotationProvider? provider = null)
+    : AnnotationTypeException(annotationId, expectedType, actualType, provider)
+{
+    public override string Message
+    {
+        get
+        {
+            if (Provider is not null)
+            {
+                return
+                    $"Typed accessor for annotation '${AnnotationId}' expected collection of values of type " +
+                    $"'{ExpectedType}' but the annotation provider returned an annotation value 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 collection of values of type '{ExpectedType}' " +
+                $"but the stored annotation value is of type '{ActualType?.ToString() ?? "[null]"}'. " +
+                $"This may be an authoring error in a typed annotation accessor, or the annotation may have been stored " +
+                $"directly  with the incorrect type, bypassing the typed accessors.";
+        }
+    }
+}