diff --git a/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs b/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs index 5677470b03..c29ee0026f 100644 --- a/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs +++ b/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) { } } diff --git a/src/System.CommandLine.Subsystems.Tests/ValidationSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/ValidationSubsystemTests.cs index 3ca4e362c4..e9d2e30833 100644 --- a/src/System.CommandLine.Subsystems.Tests/ValidationSubsystemTests.cs +++ b/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 lowerBound, T upperBound) + private CliOption GetOptionWithSimpleRange(string name, T lowerBound, T upperBound) where T : IComparable { - var option = new CliOption("--intOpt"); + var option = new CliOption(name); option.SetRange(lowerBound, upperBound); return option; } - private CliOption GetOptionWithRangeBounds(ValueSource lowerBound, ValueSource upperBound) + private CliOption GetOptionWithRangeBounds(string name, ValueSource lowerBound, ValueSource upperBound) where T : IComparable { - var option = new CliOption("--intOpt"); + var option = new CliOption(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(ValueSource.Create(() => (true, 1)), 50); + var option = GetOptionWithRangeBounds("--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.Create(() => (true, 1)), ValueSource.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.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("-a"); - var option = GetOptionWithRangeBounds(ValueSource.Create(otherOption, o => (true, (int)o + 1)), 50); + var option = GetOptionWithRangeBounds("--intOpt", ValueSource.Create(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("-a"); - var option = GetOptionWithRangeBounds(ValueSource.Create(otherOption, o => (true, (int)o + 1)), ValueSource.Create(otherOption, o => (true, (int)o + 10))); + var option = GetOptionWithRangeBounds("--intOpt", + ValueSource.Create(otherOption, o => (true, o + 1)), + ValueSource.Create(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("-a"); - var option = GetOptionWithRangeBounds(0, ValueSource.Create(otherOption, o => (true, (int)o + 10))); + var option = GetOptionWithRangeBounds("--intOpt", 0, ValueSource.Create(otherOption, o => (true, o + 10))); var command = new CliCommand("cmd") { option, otherOption }; var pipelineResult = ExecutedPipelineResultForCommand(command, "--intOpt 9 -a -2"); diff --git a/src/System.CommandLine.Subsystems/CompletionSubsystem.cs b/src/System.CommandLine.Subsystems/CompletionSubsystem.cs index 8fd0156ba0..67039d2f08 100644 --- a/src/System.CommandLine.Subsystems/CompletionSubsystem.cs +++ b/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 diff --git a/src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs b/src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs index 791290a097..421b10851f 100644 --- a/src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs +++ b/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); diff --git a/src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs b/src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs index efb51bceee..3fb43e18ea 100644 --- a/src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs +++ b/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; } diff --git a/src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs b/src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs index c626348e28..421c89256a 100644 --- a/src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs +++ b/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; } diff --git a/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs b/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs index 7c7432616b..27f43ab9f2 100644 --- a/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs +++ b/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs @@ -15,8 +15,8 @@ namespace System.CommandLine; /// 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) diff --git a/src/System.CommandLine.Subsystems/HelpAnnotationExtensions.cs b/src/System.CommandLine.Subsystems/HelpAnnotationExtensions.cs index 8cb06c594d..82b37c3dee 100644 --- a/src/System.CommandLine.Subsystems/HelpAnnotationExtensions.cs +++ b/src/System.CommandLine.Subsystems/HelpAnnotationExtensions.cs @@ -40,11 +40,28 @@ public static void SetDescription(this TSymbol symbol, string descripti /// The symbol /// The symbol description if any, otherwise /// - /// This is intended to be called by CLI authors. Subsystems should instead call , - /// values from the subsystem's . + /// This is intended to be called by CLI authors. Subsystem authors should instead call + /// to get values from + /// the pipeline's , which takes dynamic providers into account. /// public static string? GetDescription(this TSymbol symbol) where TSymbol : CliSymbol { return symbol.GetAnnotationOrDefault(HelpAnnotations.Description); } + + /// + /// Get the help description for the from the , + /// which takes dynamic providers into account. + /// + /// The type of the symbol + /// The symbol + /// The symbol description if any, otherwise + /// + /// This is intended to be called by subsystem authors. CLI authors should instead call + /// to get the value associated directly with the symbol. + /// + public static string? GetDescription(this AnnotationResolver resolver, TSymbol symbol) where TSymbol : CliSymbol + { + return resolver.GetAnnotationOrDefault(symbol, HelpAnnotations.Description); + } } diff --git a/src/System.CommandLine.Subsystems/HelpSubsystem.cs b/src/System.CommandLine.Subsystems/HelpSubsystem.cs index ca4d99c2ae..29dd06aff4 100644 --- a/src/System.CommandLine.Subsystems/HelpSubsystem.cs +++ b/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) { /// /// 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); } diff --git a/src/System.CommandLine.Subsystems/InvocationSubsystem.cs b/src/System.CommandLine.Subsystems/InvocationSubsystem.cs index a451065831..4dec80e8b1 100644 --- a/src/System.CommandLine.Subsystems/InvocationSubsystem.cs +++ b/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) {} diff --git a/src/System.CommandLine.Subsystems/Pipeline.cs b/src/System.CommandLine.Subsystems/Pipeline.cs index 837fdea398..0f8711efd6 100644 --- a/src/System.CommandLine.Subsystems/Pipeline.cs +++ b/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 invocationPhase = new(SubsystemKind.Invocation); private readonly PipelinePhase errorReportingPhase = new(SubsystemKind.ErrorReporting); private readonly IEnumerable phases; + private readonly List annotationProviders; /// /// 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? 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() }; /// @@ -54,7 +57,7 @@ public static Pipeline Create(HelpSubsystem? help = null, public static Pipeline CreateEmpty() => new(); - private Pipeline() + private Pipeline(IEnumerable? 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] + : []; } /// @@ -186,6 +193,16 @@ public ErrorReportingSubsystem? ErrorReporting /// public ResponseSubsystem Response { get; } + /// + /// Gets the list of annotation providers registered with the pipeline + /// + public IReadOnlyList AnnotationProviders => annotationProviders; + + /// + /// Adds an annotation provider to the pipeline + /// + public void AddAnnotationProvider(IAnnotationProvider annotationProvider) => annotationProviders.Add(annotationProvider); + public ParseResult Parse(CliConfiguration configuration, string rawInput) => Parse(configuration, CliParser.SplitCommandLine(rawInput).ToArray()); diff --git a/src/System.CommandLine.Subsystems/PipelineResult.cs b/src/System.CommandLine.Subsystems/PipelineResult.cs index 55b5f4963e..cb1763c3d1 100644 --- a/src/System.CommandLine.Subsystems/PipelineResult.cs +++ b/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; } diff --git a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationCollectionTypeException.cs b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationCollectionTypeException.cs new file mode 100644 index 0000000000..97ba7ea921 --- /dev/null +++ b/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; + +/// +/// Thrown when an annotation collection value does not match the expected type for that . +/// +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."; + } + } +} diff --git a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationResolveContext.cs b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationResolveContext.cs new file mode 100644 index 0000000000..a13fffea1c --- /dev/null +++ b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationResolveContext.cs @@ -0,0 +1,36 @@ +// 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. + +using System.CommandLine.Parsing; + +namespace System.CommandLine.Subsystems.Annotations; + +/// +/// Additional context that is passed to . +/// +/// +/// This class exists so that additional context properties can be added without +/// breaking existing implementations. +/// +/// This is intended to be usable independently of the pipeline. For example, a method could be +/// implemented that takes a and prints help output based on the help +/// annotations in the tree, which would then be usable by developers who +/// are using the API directly. +/// +/// +public class AnnotationResolveContext(ParseResult parseResult) +{ + public AnnotationResolveContext(PipelineResult pipelineResult) + : this(pipelineResult.ParseResult) + { + } + + /// + /// The for which annotations are being resolved. + /// + /// + /// This may be used to resolve different values for an annotation based on the parents of the symbol, + /// or based on values of other symbols in the parse result. + /// + public ParseResult ParseResult { get; } = parseResult; +} diff --git a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationResolver.cs b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationResolver.cs new file mode 100644 index 0000000000..0a16cd40a1 --- /dev/null +++ b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationResolver.cs @@ -0,0 +1,194 @@ +// 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. + +using System.Diagnostics.CodeAnalysis; + +namespace System.CommandLine.Subsystems.Annotations; + +/// +/// Provides a resolved annotation value for a given , +/// taking into account both the values stored directly on the symbol via extension methods +/// and any values from providers. +/// +/// The providers from which annotation values will be resolved +/// The context for resolving annotation values +/// +/// The will be enumerated each time an annotation value is requested. +/// It may be modified after the resolver is created. +/// +public class AnnotationResolver(IEnumerable providers, AnnotationResolveContext context) +{ + public AnnotationResolver(PipelineResult pipelineResult) + : this(pipelineResult.Pipeline.AnnotationProviders, new AnnotationResolveContext(pipelineResult)) + { + } + + /// + /// Attempt to retrieve the 's value for the annotation . This will check any + /// annotation providers that were passed to the resolver, and the internal per-symbol annotation storage. + /// + /// + /// The expected type of the annotation value. If the type does not match, a will be thrown. + /// If the annotation allows multiple types for its values, and a type parameter cannot be determined statically, + /// use to access the annotation value without checking its type. + /// + /// The symbol that is annotated + /// + /// The identifier for the annotation value to be retrieved. + /// For example, the annotation identifier for the help description is . + /// + /// An out parameter to contain the result + /// True if successful + /// + /// This is intended for use by developers defining custom annotation IDs. Anyone defining an annotation + /// ID should also define an accessor extension method on + /// that subsystem authors can use to access the annotation value, such as + /// . + /// + /// If the annotation value does not have a single expected type for this symbol, use the + /// overload instead. + /// + /// + public bool TryGet(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out TValue? value) + { + foreach (var provider in providers) + { + if (provider.TryGet(symbol, annotationId, context, out object? rawValue)) + { + if (rawValue is TValue expectedTypeValue) + { + value = expectedTypeValue; + return true; + } + throw new AnnotationTypeException(annotationId, typeof(TValue), rawValue?.GetType(), provider); + } + } + + return symbol.TryGetAnnotation(annotationId, out value); + } + + /// + /// Attempt to retrieve the 's value for the annotation . This will check any + /// annotation providers that were passed to the resolver, and the internal per-symbol annotation storage. + /// + /// The symbol that is annotated + /// + /// The identifier for the annotation value to be retrieved. + /// For example, the annotation identifier for the help description is . + /// + /// An out parameter to contain the result + /// True if successful + /// + /// This is intended for use by developers defining custom annotation IDs. Anyone defining an annotation + /// ID should also define an accessor extension method on extension method + /// on that subsystem authors can use to access the annotation value, such as + /// . + /// + /// If the expected type of the annotation value is known, use the + /// overload instead. + /// + /// + public bool TryGet(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value) + { + // a value set directly on the symbol takes precedence over values returned by providers. + if (symbol.TryGetAnnotation(annotationId, out value)) + { + return true; + } + + // Providers are given precedence in the order they were provided to the resolver. + foreach (var provider in providers) + { + if (provider.TryGet(symbol, annotationId, context, out value)) + { + return true; + } + } + + return false; + } + + /// + /// Attempt to retrieve the 's value for the annotation . This will check any + /// annotation providers that were passed to the constructor, and the internal per-symbol annotation storage. If the + /// annotation value is not found, the default value for will be returned. + /// + /// The type of the annotation value + /// The symbol that is annotated + /// + /// The identifier for the annotation. For example, the annotation identifier for the help description + /// is . + /// + /// The annotation value, if successful, otherwise default + /// + /// This is intended for use by developers defining custom annotation IDs. Anyone defining an annotation + /// ID should also define an accessor extension method on extension method + /// on that subsystem authors can use to access the annotation value, such as + /// . + /// + public TValue? GetAnnotationOrDefault(CliSymbol symbol, AnnotationId annotationId) + { + if (TryGet(symbol, annotationId, out TValue? value)) + { + return value; + } + + return default; + } + + /// + /// For an annotation that permits multiple values, enumerate the values associated with + /// the . If the annotation is not set, an empty enumerable will be returned. This will + /// check any annotation providers that were passed to the resolver, and the internal per-symbol annotation storage. + /// + /// + /// The expected type of the annotation value. If the type does not match, a + /// will be thrown. + /// + /// The symbol that is annotated + /// + /// The identifier for the annotation value to be retrieved. + /// For example, the annotation identifier for the help description is . + /// + /// An out parameter to contain the result + /// True if successful + /// + /// This is intended for use by developers defining custom annotation IDs. Anyone defining an annotation + /// ID should also define an accessor extension method on extension method + /// on that subsystem authors can use to access the annotation value, such as + /// . + /// + public IEnumerable Enumerate(CliSymbol symbol, AnnotationId annotationId) + { + // Values added directly on the symbol take precedence over values returned by providers, + // so they are returned first. + // NOTE: EnumerateAnnotations returns these in the reverse order they were added, which means that + // callers that take the first value of a given subtype will get the most recently added value of + // that subtype that the CLI author added to the symbol. + foreach (var value in symbol.EnumerateAnnotations(annotationId)) + { + yield return value; + } + + // Providers are given precedence in the order they were provided to the resolver. + foreach (var provider in providers) + { + if (!provider.TryGet(symbol, annotationId, context, out object? rawValue)) + { + continue; + } + + if (rawValue is IEnumerable expectedTypeValue) + { + foreach (var value in expectedTypeValue) + { + yield return value; + } + } + else + { + throw new AnnotationTypeException(annotationId, typeof(IEnumerable), rawValue?.GetType(), provider); + } + } + } +} \ No newline at end of file diff --git a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.AnnotationStorage.cs b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.AnnotationStorage.cs index 58341d3bf1..c9cc4a7031 100644 --- a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.AnnotationStorage.cs +++ b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.AnnotationStorage.cs @@ -7,7 +7,7 @@ namespace System.CommandLine.Subsystems.Annotations; partial class AnnotationStorageExtensions { - class AnnotationStorage : IAnnotationProvider + class AnnotationStorage { record struct AnnotationKey(CliSymbol symbol, string prefix, string id) { diff --git a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.cs b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.cs index 5a8a4be569..cddca63bdd 100644 --- a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.cs +++ b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationStorageExtensions.cs @@ -92,8 +92,6 @@ public static TSymbol WithAnnotation(this TSymbol symbol, AnnotationId /// /// /// The expected type of the annotation value. If the type does not match, a will be thrown. - /// If the annotation allows multiple types for its values, and a type parameter cannot be determined statically, - /// use to access the annotation value without checking its type. /// /// The symbol that is annotated /// @@ -102,19 +100,20 @@ public static TSymbol WithAnnotation(this TSymbol symbol, AnnotationId /// The annotation value, if successful, otherwise default /// True if successful /// - /// If the annotation value does not have a single expected type for this symbol, use the overload instead. - /// /// This is intended to be called by the implementation of specialized ID-specific accessors for CLI authors such as . - /// /// - /// Subsystems should not call it directly, as it does not account for values from the subsystem's . They should instead call - /// or an ID-specific accessor on the subsystem such - /// . + /// Subsystems should not call this directly, as it does not account for values from the pipeline's . + /// They should instead access annotations from the property using + /// or an ID-specific + /// extension method such as . /// /// + /// + /// Thrown when the type of the annotation value does not match the expected type. + /// public static bool TryGetAnnotation(this CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out TValue? value) { - if (TryGetAnnotation(symbol, annotationId, out object? rawValue)) + if (symbolToAnnotationStorage.TryGetValue(symbol, out var storage) && storage.TryGet(symbol, annotationId, out var rawValue)) { if (rawValue is TValue expectedTypeValue) { @@ -129,60 +128,160 @@ public static bool TryGetAnnotation(this CliSymbol symbol, AnnotationId } /// - /// Attempts to get the value for the annotation associated with the in the internal annotation - /// storage used to store values set via . + /// Attempts to get the value for the annotation associated with the + /// in the internal annotation storage used to store values set via + /// . /// + /// The type of the annotation value /// The symbol that is annotated /// - /// The identifier for the annotation. For example, the annotation identifier for the help description is . + /// The identifier for the annotation. For example, the annotation identifier for the help description + /// is . /// - /// The annotation value, if successful, otherwise default - /// True if successful + /// The annotation value, if successful, otherwise default /// - /// If the expected type of the annotation value is known, use the overload instead. - /// - /// This is intended to be called by the implementation of specialized ID-specific accessors for CLI authors such as . - /// - /// - /// Subsystems should not call it directly, as it does not account for values from the subsystem's . They should instead call - /// or an ID-specific accessor on the subsystem such - /// . - /// + /// Subsystems should not call this directly, as it does not account for values from the pipeline's + /// . + /// They should instead access annotations from the property using + /// or an ID-specific + /// extension method such as . /// - public static bool TryGetAnnotation(this CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value) + /// + /// Thrown when the type of the annotation value does not match the expected type. + /// + public static TValue? GetAnnotationOrDefault(this CliSymbol symbol, AnnotationId annotationId) { - if (symbolToAnnotationStorage.TryGetValue(symbol, out var storage) && storage.TryGet(symbol, annotationId, out value)) + if (symbol.TryGetAnnotation(annotationId, out TValue? value)) { - return true; + return value; } - value = default; - return false; + return default; } /// - /// Attempts to get the value for the annotation associated with the in the internal annotation - /// storage used to store values set via . + /// For an annotation that permits multiple values, add this value to the collection + /// associated with in the internal annotation storage. /// - /// The type of the annotation value /// The symbol that is annotated - /// + /// /// The identifier for the annotation. For example, the annotation identifier for the help description is . /// - /// The annotation value, if successful, otherwise default + /// The annotation value + public static void AddAnnotation(this CliSymbol symbol, AnnotationId annotationId, object value) + { + var storage = symbolToAnnotationStorage.GetValue(symbol, static (CliSymbol _) => new AnnotationStorage()); + if (!storage.TryGet(symbol, annotationId, out var existingValue)) + { + // avoid creation of the list until we have a second value + storage.Set(symbol, annotationId, value); + return; + } + + if (existingValue is AnnotationList existingList) + { + existingList.Add(value); + return; + } + + storage.Set(symbol, annotationId, new AnnotationList { existingValue, value }); + } + + /// + /// For an annotation that permits multiple values, attempt to remove this value from the collection + /// associated with in the internal annotation storage. + /// + /// The symbol that is annotated + /// + /// The identifier for the annotation. For example, the annotation identifier for the help description is + /// . + /// + /// The annotation value + /// True if the value was removed, false if the value was not found + public static bool RemoveAnnotation(this CliSymbol symbol, AnnotationId annotationId, object value) + { + var storage = symbolToAnnotationStorage.GetValue(symbol, static (CliSymbol _) => new AnnotationStorage()); + if (!storage.TryGet(symbol, annotationId, out var existingValue)) + { + return false; + } + + if (existingValue is not AnnotationList existingList) + { + if (Equals(existingValue, value)) + { + storage.Set(symbol, annotationId, null); + return true; + } + return false; + } + + return existingList.Remove(value); + } + + /// + /// For an annotation that permits multiple values, enumerate the values associated with + /// the . If the annotation is not set, an empty enumerable will be returned. + /// + /// The expected types of the annotation value. + /// If a value type does not match, a will be thrown. + /// The symbol that is annotated + /// + /// The identifier for the annotation. For example, the annotation identifier for the help description is + /// . + /// + /// The annotation values /// - /// This is intended to be called by specialized ID-specific accessors for CLI authors such as . - /// Subsystems should not call it, as it does not account for values from the subsystem's . They should instead call - /// or an ID-specific accessor on the subsystem such - /// . + /// The values are returned in the reverse order they were added, so that the first value enumerated is the + /// last value added. This means that if callers take the first value of a given subtype, this will give the + /// most recent value of the expected type. + /// + /// This is intended to be called by the implementation of specialized ID-specific accessors for + /// CLI authors such as . + /// + /// + /// Subsystems should not call this directly, as it does not account for values from the pipeline's + /// . + /// They should instead access annotations from the property using + /// or an ID-specific + /// extension method such as . + /// /// - public static TValue? GetAnnotationOrDefault(this CliSymbol symbol, AnnotationId annotationId) + /// + /// Thrown when the type of the annotation value does not match the expected type. + /// + public static IEnumerable EnumerateAnnotations(this CliSymbol symbol, AnnotationId annotationId) { - if (symbol.TryGetAnnotation(annotationId, out TValue? value)) + if (!symbolToAnnotationStorage.TryGetValue(symbol, out var storage) || !storage.TryGet(symbol, annotationId, out var rawValue)) { + yield break; + } + + if (rawValue is AnnotationList list) { - return value; + // NOTE: These are returned in the reverse order they were added, which means that callers that + // take the first value of a given subtype will get the most recently added value of that subtype + // that the CLI author added to the symbol. + for(int i = list.Count - 1; i >= 0; i--) + { + if (list[i] is TValue expectedTypeValue) { + yield return expectedTypeValue; + } else { + throw new AnnotationCollectionTypeException(annotationId, typeof(TValue), rawValue?.GetType()); + } + } } + else if (rawValue is TValue singleValue) + { + yield return singleValue; + } + else + { + throw new AnnotationCollectionTypeException(annotationId, typeof(TValue), rawValue?.GetType()); + } + } - return default; + // this private subclass ensures we don't cause issues if some annotation has a expected value of type List + class AnnotationList : List + { } } diff --git a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationTypeException.cs b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationTypeException.cs index 4d1a19aded..1df81b3033 100644 --- a/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationTypeException.cs +++ b/src/System.CommandLine.Subsystems/Subsystems/Annotations/AnnotationTypeException.cs @@ -20,14 +20,18 @@ public override string Message 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."; + $"Typed accessor for annotation '${AnnotationId}' expected value of type '{ExpectedType}' but the " + + $"annotation provider returned an 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 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."; + $"Typed accessor for annotation '${AnnotationId}' expected value 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 be stored directly " + + $"with the incorrect type, bypassing the typed accessors."; } } -} \ No newline at end of file +} diff --git a/src/System.CommandLine.Subsystems/Subsystems/Annotations/IAnnotationProvider.cs b/src/System.CommandLine.Subsystems/Subsystems/Annotations/IAnnotationProvider.cs new file mode 100644 index 0000000000..131769e9c0 --- /dev/null +++ b/src/System.CommandLine.Subsystems/Subsystems/Annotations/IAnnotationProvider.cs @@ -0,0 +1,22 @@ +// 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. + +using System.Diagnostics.CodeAnalysis; + +namespace System.CommandLine.Subsystems.Annotations; + +/// +/// Alternative storage of annotations, enabling lazy loading and dynamic annotations. +/// +public interface IAnnotationProvider +{ + /// + /// Try to get the value of the annotation with the given for the . + /// + /// Additional context that may be used when resolving the annotation value. + /// The symbol + /// The annotation identifier + /// The annotation value + /// if the symbol was resolved, otherwise + bool TryGet(CliSymbol symbol, AnnotationId annotationId, AnnotationResolveContext context, [NotNullWhen(true)] out object? value); +} diff --git a/src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs b/src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs index 49e6f98af7..640a0cebf2 100644 --- a/src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs +++ b/src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs @@ -12,10 +12,9 @@ namespace System.CommandLine.Subsystems; /// public abstract class CliSubsystem { - protected CliSubsystem(string name, SubsystemKind subsystemKind, IAnnotationProvider? annotationProvider) + protected CliSubsystem(string name, SubsystemKind subsystemKind) { Name = name; - _annotationProvider = annotationProvider; Kind = subsystemKind; } @@ -30,74 +29,6 @@ protected CliSubsystem(string name, SubsystemKind subsystemKind, IAnnotationProv public SubsystemKind Kind { get; } public AddToPhaseBehavior RecommendedAddToPhaseBehavior { get; } - private readonly IAnnotationProvider? _annotationProvider; - - /// - /// Attempt to retrieve the 's value for the annotation . This will check the - /// annotation provider that was passed to the subsystem constructor, and the internal annotation storage. - /// - /// - /// The expected type of the annotation value. If the type does not match, a will be thrown. - /// If the annotation allows multiple types for its values, and a type parameter cannot be determined statically, - /// use to access the annotation value without checking its type. - /// - /// The symbol the value is attached to - /// - /// The identifier for the annotation value to be retrieved. - /// For example, the annotation identifier for the help description is . - /// - /// An out parameter to contain the result - /// True if successful - /// - /// If the annotation value does not have a single expected type for this symbol, use the overload instead. - /// - /// Subsystem authors must use this to access annotation values, as it respects the subsystem's if it has one. - /// This value is protected because it is intended for use only by subsystem authors. It calls - /// - /// - protected internal bool TryGetAnnotation(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); - } - - /// - /// Attempt to retrieve the 's value for the annotation . This will check the - /// annotation provider that was passed to the subsystem constructor, and the internal annotation storage. - /// - /// The symbol the value is attached to - /// - /// The identifier for the annotation value to be retrieved. - /// For example, the annotation identifier for the help description is . - /// - /// An out parameter to contain the result - /// True if successful - /// - /// If the expected type of the annotation value is known, use the overload instead. - /// - /// Subsystem authors must use this to access annotation values, as it respects the subsystem's if it has one. - /// This value is protected because it is intended for use only by subsystem authors. It calls - /// - /// - protected internal bool TryGetAnnotation(CliSymbol symbol, AnnotationId annotationId, [NotNullWhen(true)] out object? value) - { - if (_annotationProvider is not null && _annotationProvider.TryGet(symbol, annotationId, out value)) - { - return true; - } - - return symbol.TryGetAnnotation(annotationId, out value); - } - /// /// The subystem executes, even if another subsystem has handled the operation. This is expected to be used in things like error reporting. /// diff --git a/src/System.CommandLine.Subsystems/Subsystems/IAnnotationProvider.cs b/src/System.CommandLine.Subsystems/Subsystems/IAnnotationProvider.cs deleted file mode 100644 index dd4d9e4fd5..0000000000 --- a/src/System.CommandLine.Subsystems/Subsystems/IAnnotationProvider.cs +++ /dev/null @@ -1,15 +0,0 @@ -// 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. - -using System.CommandLine.Subsystems.Annotations; -using System.Diagnostics.CodeAnalysis; - -namespace System.CommandLine.Subsystems; - -/// -/// Alternative storage of annotations, enabling lazy loading and dynamic annotations. -/// -public interface IAnnotationProvider -{ - bool TryGet(CliSymbol symbol, AnnotationId id, [NotNullWhen(true)] out object? value); -} diff --git a/src/System.CommandLine.Subsystems/ValidationSubsystem.cs b/src/System.CommandLine.Subsystems/ValidationSubsystem.cs index e3dd610b38..00e44e9971 100644 --- a/src/System.CommandLine.Subsystems/ValidationSubsystem.cs +++ b/src/System.CommandLine.Subsystems/ValidationSubsystem.cs @@ -14,8 +14,8 @@ public sealed class ValidationSubsystem : CliSubsystem private Dictionary valueValidators = []; private Dictionary commandValidators = []; - private ValidationSubsystem(IAnnotationProvider? annotationProvider = null) - : base("", SubsystemKind.Validation, annotationProvider) + private ValidationSubsystem() + : base("", SubsystemKind.Validation) { } public static ValidationSubsystem Create() @@ -68,28 +68,31 @@ public override void Execute(PipelineResult pipelineResult) private void ValidateValue(CliValueSymbol valueSymbol, ValidationContext validationContext) { - var valueConditions = valueSymbol.GetValueConditions(); - if (valueConditions is null) + var valueConditions = valueSymbol.EnumerateValueConditions(); + + // manually implement the foreach so we can efficiently skip getting the + // value if there are no conditions + var enumerator = valueConditions.GetEnumerator(); + if (!enumerator.MoveNext()) { - return; // nothing to do + return; } var value = validationContext.GetValue(valueSymbol); var valueResult = validationContext.GetValueResult(valueSymbol); - foreach (var condition in valueConditions) - { - ValidateValueCondition(value, valueSymbol, valueResult, condition, validationContext); - } + + do { + ValidateValueCondition(value, valueSymbol, valueResult, enumerator.Current, validationContext); + } while (enumerator.MoveNext()); } private void ValidateCommand(CliCommandResult commandResult, ValidationContext validationContext) { - var valueConditions = commandResult.Command.GetCommandConditions(); - if (valueConditions is null) - { - return; // nothing to do - } + var valueConditions = commandResult.Command.EnumerateCommandConditions(); + // unlike ValidateValue we do not need to manually implement the foreach + // to skip unnecessary work, as there is no additional work to be before + // calling command validators foreach (var condition in valueConditions) { ValidateCommandCondition(commandResult, condition, validationContext); diff --git a/src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs b/src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs index 676bcc9b7d..68d3d6324e 100644 --- a/src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs +++ b/src/System.CommandLine.Subsystems/ValueAnnotationExtensions.cs @@ -17,9 +17,12 @@ public static class ValueAnnotationExtensions /// The option /// The option's default value annotation if any, otherwise /// - /// This is intended to be called by CLI authors. Subsystems should instead call , - /// which calculates the actual default value, based on the default value annotation and default value calculation, - /// whether directly stored on the symbol or from the subsystem's . + /// This is intended to be called by CLI authors inspecting the default value source they have + /// associated directly with the . + /// + /// Subsystems should instead use , which caches calculated + /// values and respects dynamic/lazy annotations from the . + /// /// public static bool TryGetDefaultValueSource(this CliValueSymbol valueSymbol, [NotNullWhen(true)] out ValueSource? defaultValueSource) => valueSymbol.TryGetAnnotation(ValueAnnotations.DefaultValueSource, out defaultValueSource); diff --git a/src/System.CommandLine.Subsystems/ValueConditionAnnotationExtensions.cs b/src/System.CommandLine.Subsystems/ValueConditionAnnotationExtensions.cs index 1ca563b7e4..77a9e65b8c 100644 --- a/src/System.CommandLine.Subsystems/ValueConditionAnnotationExtensions.cs +++ b/src/System.CommandLine.Subsystems/ValueConditionAnnotationExtensions.cs @@ -1,4 +1,7 @@ -using System.CommandLine.Subsystems.Annotations; +// 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. + +using System.CommandLine.Subsystems.Annotations; using System.CommandLine.ValueConditions; using System.CommandLine.ValueSources; @@ -10,41 +13,48 @@ namespace System.CommandLine; public static class ValueConditionAnnotationExtensions { /// - /// Set the upper and/or lower bound values of the range. + /// Set upper and/or lower bounds on the range of values that the symbol value may have. /// - /// The type of the bounds. + /// The type of the symbol whose value is bounded by the range. + /// The type of the value that is bounded by the range. /// The option or argument the range applies to. /// The lower bound of the range. /// The upper bound of the range. - // TODO: Add RangeBounds - // TODO: You should not have to set both...why not nullable? - public static void SetRange(this CliValueSymbol symbol, T lowerBound, T upperBound) - where T : IComparable + // TODO: can we eliminate this overload and just reply on the implicit cast to ValueSource? + public static void SetRange(this TValueSymbol symbol, TValue lowerBound, TValue upperBound) + where TValueSymbol : CliValueSymbol, ICliValueSymbol + where TValue : IComparable { - var range = new Range(lowerBound, upperBound); + var range = new Range(lowerBound, upperBound); symbol.SetValueCondition(range); } /// - /// Set the upper and/or lower bound via ValueSource. Implicit conversions means this - /// generally just works with any . + /// Set upper and/or lower bounds on the range of values that the symbol value may have. + /// Implicit conversions means this generally just works with any . /// - /// The type of the bounds. + /// The type of the symbol whose value is bounded by the range. + /// The type of the value that is bounded by the range. /// The option or argument the range applies to. /// The that is the lower bound of the range. /// The that is the upper bound of the range. - // TODO: Add RangeBounds - // TODO: You should not have to set both...why not nullable? - public static void SetRange(this CliValueSymbol symbol, ValueSource lowerBound, ValueSource upperBound) - where T : IComparable - // TODO: You should not have to set both...why not nullable? + public static void SetRange(this TValueSymbol symbol, ValueSource? lowerBound, ValueSource? upperBound) + where TValueSymbol : CliValueSymbol, ICliValueSymbol + where TValue : IComparable { - var range = new Range(lowerBound, upperBound); + var range = new Range(lowerBound, upperBound); symbol.SetValueCondition(range); } + /// + /// Get the upper and/or lower bound of the symbol's value. + /// + /// The option or argument the range applies to. + public static ValueConditions.Range? GetRange(this CliValueSymbol symbol) + => symbol.GetValueCondition(); + /// /// Indicates that there is an inclusive group of options and arguments for the command. All /// members of an inclusive must be present, or none can be present. @@ -54,54 +64,48 @@ public static void SetRange(this CliValueSymbol symbol, ValueSource lowerB public static void SetInclusiveGroup(this CliCommand command, IEnumerable group) => command.SetValueCondition(new InclusiveGroup(group)); - // TODO: This should not be public if ValueConditions are not public public static void SetValueCondition(this TValueSymbol symbol, TValueCondition valueCondition) where TValueSymbol : CliValueSymbol where TValueCondition : ValueCondition - { - if (!symbol.TryGetAnnotation>(ValueConditionAnnotations.ValueConditions, out var valueConditions)) - { - valueConditions = []; - symbol.SetAnnotation(ValueConditionAnnotations.ValueConditions, valueConditions); - } - valueConditions.Add(valueCondition); - } + => symbol.AddAnnotation(ValueConditionAnnotations.ValueConditions, valueCondition); - // TODO: This should not be public if ValueConditions are not public public static void SetValueCondition(this CliCommand symbol, TCommandCondition commandCondition) where TCommandCondition : CommandCondition - { - if (!symbol.TryGetAnnotation>(ValueConditionAnnotations.ValueConditions, out var valueConditions)) - { - valueConditions = []; - symbol.SetAnnotation(ValueConditionAnnotations.ValueConditions, valueConditions); - } - valueConditions.Add(commandCondition); - } + => symbol.AddAnnotation(ValueConditionAnnotations.ValueConditions, commandCondition); /// /// Gets a list of conditions on an option or argument. /// /// The option or argument to get the conditions for. /// The conditions that have been applied to the option or argument. - /// - // TODO: This is public because it will be used by other subsystems we might not own. It could be an extension method the subsystem namespace - public static List? GetValueConditions(this CliValueSymbol symbol) - => symbol.TryGetAnnotation>(ValueConditionAnnotations.ValueConditions, out var valueConditions) - ? valueConditions - : null; + public static IEnumerable EnumerateValueConditions(this CliValueSymbol symbol) + => symbol.EnumerateAnnotations(ValueConditionAnnotations.ValueConditions); + + /// + /// Gets a list of conditions on an option or argument. + /// + /// The option or argument to get the conditions for. + /// The conditions that have been applied to the option or argument. + public static IEnumerable EnumerateValueConditions(this AnnotationResolver resolver, CliValueSymbol symbol) + => resolver.Enumerate(symbol, ValueConditionAnnotations.ValueConditions); + + /// + /// Gets a list of conditions on a command. + /// + /// The command to get the conditions for. + /// The conditions that have been applied to the command. + public static IEnumerable EnumerateCommandConditions(this CliCommand command) + => command.EnumerateAnnotations(ValueConditionAnnotations.ValueConditions); /// /// Gets a list of conditions on a command. /// /// The command to get the conditions for. /// The conditions that have been applied to the command. - /// + /// // TODO: This is public because it will be used by other subsystems we might not own. It could be an extension method the subsystem namespace - public static List? GetCommandConditions(this CliCommand command) - => command.TryGetAnnotation>(ValueConditionAnnotations.ValueConditions, out var valueConditions) - ? valueConditions - : null; + public static IEnumerable EnumerateCommandConditions(this AnnotationResolver resolver, CliCommand command) + => resolver.Enumerate(command, ValueConditionAnnotations.ValueConditions); /// /// Gets the condition that matches the type, if it exists on this option or argument. @@ -109,13 +113,9 @@ public static void SetValueCondition(this CliCommand symbol, /// The type of condition to return. /// The option or argument that may contain the condition. /// The condition if it exists on the option or argument, otherwise null. - // This method feels useful because it clarifies that last should win and returns one, when only one should be applied - // TODO: Consider removing user facing naming, other than the base type, that is Value or CommandCondition and just use Condition public static TCondition? GetValueCondition(this CliValueSymbol symbol) where TCondition : ValueCondition - => !symbol.TryGetAnnotation(ValueConditionAnnotations.ValueConditions, out List? valueConditions) - ? null - : valueConditions.OfType().LastOrDefault(); + => symbol.EnumerateValueConditions().OfType().FirstOrDefault(); /// /// Gets the condition that matches the type, if it exists on this command. @@ -123,12 +123,9 @@ public static void SetValueCondition(this CliCommand symbol, /// The type of condition to return. /// The command that may contain the condition. /// The condition if it exists on the command, otherwise null. - // This method feels useful because it clarifies that last should win and returns one, when only one should be applied public static TCondition? GetCommandCondition(this CliCommand symbol) where TCondition : CommandCondition - => !symbol.TryGetAnnotation(ValueConditionAnnotations.ValueConditions, out List? valueConditions) - ? null - : valueConditions.OfType().LastOrDefault(); + => symbol.EnumerateCommandConditions().OfType().FirstOrDefault(); } diff --git a/src/System.CommandLine.Subsystems/ValueProvider.cs b/src/System.CommandLine.Subsystems/ValueProvider.cs index f51a1edb63..adb6b9c90e 100644 --- a/src/System.CommandLine.Subsystems/ValueProvider.cs +++ b/src/System.CommandLine.Subsystems/ValueProvider.cs @@ -1,6 +1,7 @@ // 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. +using System.CommandLine.Subsystems.Annotations; using System.CommandLine.ValueSources; namespace System.CommandLine; @@ -48,7 +49,7 @@ private bool TryGetValue(CliSymbol symbol, out T? value) { return UseValue(valueSymbol, valueResult.GetValue()); } - if (valueSymbol.TryGetDefaultValueSource(out ValueSource? defaultValueSource)) + if (pipelineResult.Annotations.TryGet(valueSymbol, ValueAnnotations.DefaultValueSource, out ValueSource? defaultValueSource)) { if (defaultValueSource is not ValueSource typedDefaultValueSource) { diff --git a/src/System.CommandLine.Subsystems/ValueSources/RelativeToSymbolValueSource.cs b/src/System.CommandLine.Subsystems/ValueSources/RelativeToSymbolValueSource.cs index a53e13c573..ba48870130 100644 --- a/src/System.CommandLine.Subsystems/ValueSources/RelativeToSymbolValueSource.cs +++ b/src/System.CommandLine.Subsystems/ValueSources/RelativeToSymbolValueSource.cs @@ -17,7 +17,7 @@ public sealed class RelativeToSymbolValueSource internal RelativeToSymbolValueSource( CliValueSymbol otherSymbol, bool onlyUserEnteredValues = false, - Func? calculation = null, + Func? calculation = null, string? description = null) { OtherSymbol = otherSymbol; @@ -29,7 +29,7 @@ internal RelativeToSymbolValueSource( public override string? Description { get; } public CliValueSymbol OtherSymbol { get; } public bool OnlyUserEnteredValues { get; } - public Func? Calculation { get; } + public Func? Calculation { get; } /// public override bool TryGetTypedValue(PipelineResult pipelineResult, out T? value) diff --git a/src/System.CommandLine.Subsystems/ValueSources/ValueSource.cs b/src/System.CommandLine.Subsystems/ValueSources/ValueSource.cs index 43098896ed..18ef7a97b6 100644 --- a/src/System.CommandLine.Subsystems/ValueSources/ValueSource.cs +++ b/src/System.CommandLine.Subsystems/ValueSources/ValueSource.cs @@ -31,7 +31,7 @@ public static ValueSource Create(Func<(bool success, T? value)> calculatio => new CalculatedValueSource(calculation, description); public static ValueSource Create(CliValueSymbol otherSymbol, - Func? calculation = null, + Func? calculation = null, bool userEnteredValueOnly = false, string? description = null) => new RelativeToSymbolValueSource(otherSymbol, userEnteredValueOnly, calculation, description); diff --git a/src/System.CommandLine.Subsystems/VersionSubsystem.cs b/src/System.CommandLine.Subsystems/VersionSubsystem.cs index 1aac8b75a1..c8b8c784dc 100644 --- a/src/System.CommandLine.Subsystems/VersionSubsystem.cs +++ b/src/System.CommandLine.Subsystems/VersionSubsystem.cs @@ -11,8 +11,8 @@ public class VersionSubsystem : CliSubsystem { private string? specificVersion = null; - public VersionSubsystem(IAnnotationProvider? annotationProvider = null) - : base(VersionAnnotations.Prefix, SubsystemKind.Version, annotationProvider) + public VersionSubsystem() + : base(VersionAnnotations.Prefix, SubsystemKind.Version) { } diff --git a/src/System.CommandLine/CliArgument{T}.cs b/src/System.CommandLine/CliArgument{T}.cs index 14015b7ac5..f21c2db0d5 100644 --- a/src/System.CommandLine/CliArgument{T}.cs +++ b/src/System.CommandLine/CliArgument{T}.cs @@ -4,12 +4,12 @@ using System.Collections.Generic; using System.CommandLine.Parsing; using System.Diagnostics.CodeAnalysis; -using System.IO; namespace System.CommandLine { + /// - public class CliArgument : CliArgument + public class CliArgument : CliArgument, ICliValueSymbol { // TODO: custom parser /* diff --git a/src/System.CommandLine/CliOption{T}.cs b/src/System.CommandLine/CliOption{T}.cs index f2ca874d1a..21db11cb97 100644 --- a/src/System.CommandLine/CliOption{T}.cs +++ b/src/System.CommandLine/CliOption{T}.cs @@ -7,7 +7,7 @@ namespace System.CommandLine { /// /// The that the option's arguments are expected to be parsed as. - public class CliOption : CliOption + public class CliOption : CliOption, ICliValueSymbol { // TODO: do not expose private fields internal readonly CliArgument _argument; diff --git a/src/System.CommandLine/ICliValueSymbol.cs b/src/System.CommandLine/ICliValueSymbol.cs new file mode 100644 index 0000000000..d4d7dbdddf --- /dev/null +++ b/src/System.CommandLine/ICliValueSymbol.cs @@ -0,0 +1,14 @@ +// 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; + +/// +/// This is implemented only by and , and allows +/// allows methods with a argument to apply constraints based on the +/// value type. +/// +/// The value type +public interface ICliValueSymbol +{ +} diff --git a/src/System.CommandLine/System.CommandLine.csproj b/src/System.CommandLine/System.CommandLine.csproj index cfb211a276..bee122292b 100644 --- a/src/System.CommandLine/System.CommandLine.csproj +++ b/src/System.CommandLine/System.CommandLine.csproj @@ -27,6 +27,7 @@ +