Skip to content

Commit d25d2da

Browse files
mhutchmhutch
authored
Merge pull request #2489 from mhutch/annotation-resolver
Centralize annotation resolution and allow multivalued annotations
2 parents 1f0c95b + ae85349 commit d25d2da

32 files changed

+616
-255
lines changed

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public VersionThatUsesHelpData(CliSymbol symbol)
3030

3131
public override void Execute(PipelineResult pipelineResult)
3232
{
33-
TryGetAnnotation(Symbol, HelpAnnotations.Description, out string? description);
33+
pipelineResult.Annotations.TryGet(Symbol, HelpAnnotations.Description, out string? description);
3434
pipelineResult.ConsoleHack.WriteLine(description);
3535
pipelineResult.AlreadyHandled = true;
3636
pipelineResult.SetSuccess();
@@ -63,12 +63,12 @@ protected internal override void TearDown(PipelineResult pipelineResult)
6363
}
6464
}
6565

66-
internal class StringDirectiveSubsystem(IAnnotationProvider? annotationProvider = null)
67-
: DirectiveSubsystem("other", SubsystemKind.Diagram, annotationProvider)
66+
internal class StringDirectiveSubsystem()
67+
: DirectiveSubsystem("other", SubsystemKind.Diagram)
6868
{ }
6969

70-
internal class BooleanDirectiveSubsystem(IAnnotationProvider? annotationProvider = null)
71-
: DirectiveSubsystem("diagram", SubsystemKind.Diagram, annotationProvider)
70+
internal class BooleanDirectiveSubsystem()
71+
: DirectiveSubsystem("diagram", SubsystemKind.Diagram)
7272
{ }
7373

7474
}

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

+17-15
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,18 @@ namespace System.CommandLine.Subsystems.Tests;
1111
public class ValidationSubsystemTests
1212
{
1313
// Running exactly the same code is important here because missing a step will result in a false positive. Ask me how I know
14-
private CliOption GetOptionWithSimpleRange<T>(T lowerBound, T upperBound)
14+
private CliOption<T> GetOptionWithSimpleRange<T>(string name, T lowerBound, T upperBound)
1515
where T : IComparable<T>
1616
{
17-
var option = new CliOption<int>("--intOpt");
17+
var option = new CliOption<T>(name);
1818
option.SetRange(lowerBound, upperBound);
1919
return option;
2020
}
2121

22-
private CliOption GetOptionWithRangeBounds<T>(ValueSource<T> lowerBound, ValueSource<T> upperBound)
22+
private CliOption<T> GetOptionWithRangeBounds<T>(string name, ValueSource<T> lowerBound, ValueSource<T> upperBound)
2323
where T : IComparable<T>
2424
{
25-
var option = new CliOption<int>("--intOpt");
25+
var option = new CliOption<T>(name);
2626
option.SetRange(lowerBound, upperBound);
2727
return option;
2828
}
@@ -45,7 +45,7 @@ private PipelineResult ExecutedPipelineResultForCommand(CliCommand command, stri
4545
[Fact]
4646
public void Int_values_in_specified_range_do_not_have_errors()
4747
{
48-
var option = GetOptionWithSimpleRange(0, 50);
48+
var option = GetOptionWithSimpleRange("--intOpt", 0, 50);
4949

5050
var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
5151

@@ -56,7 +56,7 @@ public void Int_values_in_specified_range_do_not_have_errors()
5656
[Fact]
5757
public void Int_values_above_upper_bound_report_error()
5858
{
59-
var option = GetOptionWithSimpleRange(0, 5);
59+
var option = GetOptionWithSimpleRange("--intOpt", 0, 5);
6060

6161
var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
6262

@@ -69,7 +69,7 @@ public void Int_values_above_upper_bound_report_error()
6969
[Fact]
7070
public void Int_below_lower_bound_report_error()
7171
{
72-
var option = GetOptionWithSimpleRange(0, 5);
72+
var option = GetOptionWithSimpleRange("--intOpt", 0, 5);
7373

7474
var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt -42");
7575

@@ -82,7 +82,7 @@ public void Int_below_lower_bound_report_error()
8282
[Fact]
8383
public void Int_values_on_lower_range_bound_do_not_report_error()
8484
{
85-
var option = GetOptionWithSimpleRange(42, 50);
85+
var option = GetOptionWithSimpleRange("--intOpt", 42, 50);
8686

8787
var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
8888

@@ -93,7 +93,7 @@ public void Int_values_on_lower_range_bound_do_not_report_error()
9393
[Fact]
9494
public void Int_values_on_upper_range_bound_do_not_report_error()
9595
{
96-
var option = GetOptionWithSimpleRange(0, 42);
96+
var option = GetOptionWithSimpleRange("--intOpt", 0, 42);
9797

9898
var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
9999

@@ -104,7 +104,7 @@ public void Int_values_on_upper_range_bound_do_not_report_error()
104104
[Fact]
105105
public void Values_below_calculated_lower_bound_report_error()
106106
{
107-
var option = GetOptionWithRangeBounds<int>(ValueSource.Create(() => (true, 1)), 50);
107+
var option = GetOptionWithRangeBounds<int>("--intOpt", ValueSource.Create(() => (true, 1)), 50);
108108

109109
var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 0");
110110

@@ -118,7 +118,7 @@ public void Values_below_calculated_lower_bound_report_error()
118118
[Fact]
119119
public void Values_within_calculated_range_do_not_report_error()
120120
{
121-
var option = GetOptionWithRangeBounds(ValueSource<int>.Create(() => (true, 1)), ValueSource<int>.Create(() => (true, 50)));
121+
var option = GetOptionWithRangeBounds("--intOpt", ValueSource.Create(() => (true, 1)), ValueSource.Create(() => (true, 50)));
122122

123123
var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
124124

@@ -129,7 +129,7 @@ public void Values_within_calculated_range_do_not_report_error()
129129
[Fact]
130130
public void Values_above_calculated_upper_bound_report_error()
131131
{
132-
var option = GetOptionWithRangeBounds(0, ValueSource<int>.Create(() => (true, 40)));
132+
var option = GetOptionWithRangeBounds("--intOpt", 0, ValueSource.Create(() => (true, 40)));
133133

134134
var pipelineResult = ExecutedPipelineResultForRangeOption(option, "--intOpt 42");
135135

@@ -143,7 +143,7 @@ public void Values_above_calculated_upper_bound_report_error()
143143
public void Values_below_relative_lower_bound_report_error()
144144
{
145145
var otherOption = new CliOption<int>("-a");
146-
var option = GetOptionWithRangeBounds(ValueSource.Create(otherOption, o => (true, (int)o + 1)), 50);
146+
var option = GetOptionWithRangeBounds("--intOpt", ValueSource.Create<int>(otherOption, o => (true, o + 1)), 50);
147147
var command = new CliCommand("cmd") { option, otherOption };
148148

149149
var pipelineResult = ExecutedPipelineResultForCommand(command, "--intOpt 0 -a 0");
@@ -159,7 +159,9 @@ public void Values_below_relative_lower_bound_report_error()
159159
public void Values_within_relative_range_do_not_report_error()
160160
{
161161
var otherOption = new CliOption<int>("-a");
162-
var option = GetOptionWithRangeBounds(ValueSource<int>.Create(otherOption, o => (true, (int)o + 1)), ValueSource<int>.Create(otherOption, o => (true, (int)o + 10)));
162+
var option = GetOptionWithRangeBounds("--intOpt",
163+
ValueSource.Create<int>(otherOption, o => (true, o + 1)),
164+
ValueSource.Create<int>(otherOption, o => (true, o + 10)));
163165
var command = new CliCommand("cmd") { option, otherOption };
164166

165167
var pipelineResult = ExecutedPipelineResultForCommand(command, "--intOpt 11 -a 3");
@@ -172,7 +174,7 @@ public void Values_within_relative_range_do_not_report_error()
172174
public void Values_above_relative_upper_bound_report_error()
173175
{
174176
var otherOption = new CliOption<int>("-a");
175-
var option = GetOptionWithRangeBounds(0, ValueSource<int>.Create(otherOption, o => (true, (int)o + 10)));
177+
var option = GetOptionWithRangeBounds("--intOpt", 0, ValueSource.Create<int>(otherOption, o => (true, o + 10)));
176178
var command = new CliCommand("cmd") { option, otherOption };
177179

178180
var pipelineResult = ExecutedPipelineResultForCommand(command, "--intOpt 9 -a -2");

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ namespace System.CommandLine;
1717

1818
public class CompletionSubsystem : CliSubsystem
1919
{
20-
public CompletionSubsystem(IAnnotationProvider? annotationProvider = null)
21-
: base(CompletionAnnotations.Prefix, SubsystemKind.Completion, annotationProvider)
20+
public CompletionSubsystem()
21+
: base(CompletionAnnotations.Prefix, SubsystemKind.Completion)
2222
{ }
2323

2424
// TODO: Figure out trigger for completions

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66

77
namespace System.CommandLine.Directives;
88

9-
public class DiagramSubsystem(IAnnotationProvider? annotationProvider = null)
10-
: DirectiveSubsystem("diagram", SubsystemKind.Diagram, annotationProvider)
9+
public class DiagramSubsystem()
10+
: DirectiveSubsystem("diagram", SubsystemKind.Diagram)
1111
{
1212
//protected internal override bool GetIsActivated(ParseResult? parseResult)
1313
// => parseResult is not null && option is not null && parseResult.GetValue(option);

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ public abstract class DirectiveSubsystem : CliSubsystem
1313
public string Id { get; }
1414
public Location? Location { get; private set; }
1515

16-
public DirectiveSubsystem(string name, SubsystemKind kind, IAnnotationProvider? annotationProvider = null, string? id = null)
17-
: base(name, kind, annotationProvider: annotationProvider)
16+
public DirectiveSubsystem(string name, SubsystemKind kind, string? id = null)
17+
: base(name, kind)
1818
{
1919
Id = id ?? name;
2020
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
namespace System.CommandLine.Directives;
88

99
public class ResponseSubsystem()
10-
: CliSubsystem("Response", SubsystemKind.Response, null)
10+
: CliSubsystem("Response", SubsystemKind.Response)
1111
{
1212
public bool Enabled { get; set; }
1313

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ namespace System.CommandLine;
1515
/// </remarks>
1616
public class ErrorReportingSubsystem : CliSubsystem
1717
{
18-
public ErrorReportingSubsystem(IAnnotationProvider? annotationProvider = null)
19-
: base(ErrorReportingAnnotations.Prefix, SubsystemKind.ErrorReporting, annotationProvider)
18+
public ErrorReportingSubsystem()
19+
: base(ErrorReportingAnnotations.Prefix, SubsystemKind.ErrorReporting)
2020
{ }
2121

2222
protected internal override bool GetIsActivated(ParseResult? parseResult)

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

+19-2
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,28 @@ public static void SetDescription<TSymbol>(this TSymbol symbol, string descripti
4040
/// <param name="symbol">The symbol</param>
4141
/// <returns>The symbol description if any, otherwise <see langword="null"/></returns>
4242
/// <remarks>
43-
/// This is intended to be called by CLI authors. Subsystems should instead call <see cref="HelpSubsystem.TryGetDescription(CliSymbol, out string?)"/>,
44-
/// values from the subsystem's <see cref="IAnnotationProvider"/>.
43+
/// This is intended to be called by CLI authors. Subsystem authors should instead call
44+
/// <see cref="GetDescription{TSymbol}(AnnotationResolver, TSymbol)"/> to get values from
45+
/// the pipeline's <see cref="Pipeline.Annotations"/>, which takes dynamic providers into account.
4546
/// </remarks>
4647
public static string? GetDescription<TSymbol>(this TSymbol symbol) where TSymbol : CliSymbol
4748
{
4849
return symbol.GetAnnotationOrDefault<string>(HelpAnnotations.Description);
4950
}
51+
52+
/// <summary>
53+
/// Get the help description for the <paramref name="symbol"/> from the <paramref name="resolver"/>,
54+
/// which takes dynamic providers into account.
55+
/// </summary>
56+
/// <typeparam name="TSymbol">The type of the symbol</typeparam>
57+
/// <param name="symbol">The symbol</param>
58+
/// <returns>The symbol description if any, otherwise <see langword="null"/></returns>
59+
/// <remarks>
60+
/// This is intended to be called by subsystem authors. CLI authors should instead call
61+
/// <see cref="GetDescription{TSymbol}(TSymbol)"/> to get the value associated directly with the symbol.
62+
/// </remarks>
63+
public static string? GetDescription<TSymbol>(this AnnotationResolver resolver, TSymbol symbol) where TSymbol : CliSymbol
64+
{
65+
return resolver.GetAnnotationOrDefault<string>(symbol, HelpAnnotations.Description);
66+
}
5067
}

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

+2-5
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ namespace System.CommandLine;
1515
// var command = new CliCommand("greet")
1616
// .With(help.Description, "Greet the user");
1717
//
18-
public class HelpSubsystem(IAnnotationProvider? annotationProvider = null)
19-
: CliSubsystem(HelpAnnotations.Prefix, SubsystemKind.Help, annotationProvider)
18+
public class HelpSubsystem()
19+
: CliSubsystem(HelpAnnotations.Prefix, SubsystemKind.Help)
2020
{
2121
/// <summary>
2222
/// Gets the help option, which allows the user to customize
@@ -44,7 +44,4 @@ public override void Execute(PipelineResult pipelineResult)
4444
pipelineResult.ConsoleHack.WriteLine("Help me!");
4545
pipelineResult.SetSuccess();
4646
}
47-
48-
public bool TryGetDescription(CliSymbol symbol, out string? description)
49-
=> TryGetAnnotation(symbol, HelpAnnotations.Description, out description);
5047
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@
66

77
namespace System.CommandLine;
88

9-
public class InvocationSubsystem(IAnnotationProvider? annotationProvider = null)
10-
: CliSubsystem(InvocationAnnotations.Prefix, SubsystemKind.Invocation, annotationProvider)
9+
public class InvocationSubsystem()
10+
: CliSubsystem(InvocationAnnotations.Prefix, SubsystemKind.Invocation)
1111
{}

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

+21-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.CommandLine.Directives;
55
using System.CommandLine.Parsing;
66
using System.CommandLine.Subsystems;
7+
using System.CommandLine.Subsystems.Annotations;
78

89
namespace System.CommandLine;
910

@@ -17,6 +18,7 @@ public partial class Pipeline
1718
private readonly PipelinePhase<InvocationSubsystem> invocationPhase = new(SubsystemKind.Invocation);
1819
private readonly PipelinePhase<ErrorReportingSubsystem> errorReportingPhase = new(SubsystemKind.ErrorReporting);
1920
private readonly IEnumerable<PipelinePhase> phases;
21+
private readonly List<IAnnotationProvider> annotationProviders;
2022

2123
/// <summary>
2224
/// Creates an instance of the pipeline using standard features.
@@ -34,14 +36,15 @@ public static Pipeline Create(HelpSubsystem? help = null,
3436
VersionSubsystem? version = null,
3537
CompletionSubsystem? completion = null,
3638
DiagramSubsystem? diagram = null,
37-
ErrorReportingSubsystem? errorReporting = null)
38-
=> new()
39+
ErrorReportingSubsystem? errorReporting = null,
40+
IEnumerable<IAnnotationProvider>? annotationProviders = null)
41+
=> new(annotationProviders)
3942
{
4043
Help = help ?? new HelpSubsystem(),
4144
Version = version ?? new VersionSubsystem(),
4245
Completion = completion ?? new CompletionSubsystem(),
4346
Diagram = diagram ?? new DiagramSubsystem(),
44-
ErrorReporting = errorReporting ?? new ErrorReportingSubsystem(),
47+
ErrorReporting = errorReporting ?? new ErrorReportingSubsystem()
4548
};
4649

4750
/// <summary>
@@ -54,7 +57,7 @@ public static Pipeline Create(HelpSubsystem? help = null,
5457
public static Pipeline CreateEmpty()
5558
=> new();
5659

57-
private Pipeline()
60+
private Pipeline(IEnumerable<IAnnotationProvider>? annotationProviders = null)
5861
{
5962
Response = new ResponseSubsystem();
6063
Invocation = new InvocationSubsystem();
@@ -68,6 +71,10 @@ private Pipeline()
6871
diagramPhase, completionPhase, helpPhase, versionPhase,
6972
validationPhase, invocationPhase, errorReportingPhase
7073
];
74+
75+
this.annotationProviders = annotationProviders is not null
76+
? [..annotationProviders]
77+
: [];
7178
}
7279

7380
/// <summary>
@@ -186,6 +193,16 @@ public ErrorReportingSubsystem? ErrorReporting
186193
/// </summary>
187194
public ResponseSubsystem Response { get; }
188195

196+
/// <summary>
197+
/// Gets the list of annotation providers registered with the pipeline
198+
/// </summary>
199+
public IReadOnlyList<IAnnotationProvider> AnnotationProviders => annotationProviders;
200+
201+
/// <summary>
202+
/// Adds an annotation provider to the pipeline
203+
/// </summary>
204+
public void AddAnnotationProvider(IAnnotationProvider annotationProvider) => annotationProviders.Add(annotationProvider);
205+
189206
public ParseResult Parse(CliConfiguration configuration, string rawInput)
190207
=> Parse(configuration, CliParser.SplitCommandLine(rawInput).ToArray());
191208

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

+4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.CommandLine.Parsing;
5+
using System.CommandLine.Subsystems.Annotations;
56

67
namespace System.CommandLine;
78

@@ -18,6 +19,7 @@ public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeli
1819
Pipeline = pipeline ?? Pipeline.CreateEmpty();
1920
ConsoleHack = consoleHack ?? new ConsoleHack();
2021
valueProvider = new ValueProvider(this);
22+
Annotations = new AnnotationResolver(this);
2123
}
2224

2325
public ParseResult ParseResult { get; }
@@ -27,6 +29,8 @@ public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeli
2729
public Pipeline Pipeline { get; }
2830
public ConsoleHack ConsoleHack { get; }
2931

32+
public AnnotationResolver Annotations { get; }
33+
3034
public bool AlreadyHandled { get; set; }
3135
public int ExitCode { get; set; }
3236

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

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
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 collection value does not match the expected type for that <see cref="AnnotationId"/>.
8+
/// </summary>
9+
public class AnnotationCollectionTypeException(AnnotationId annotationId, Type expectedType, Type? actualType, IAnnotationProvider? provider = null)
10+
: AnnotationTypeException(annotationId, expectedType, actualType, provider)
11+
{
12+
public override string Message
13+
{
14+
get
15+
{
16+
if (Provider is not null)
17+
{
18+
return
19+
$"Typed accessor for annotation '${AnnotationId}' expected collection of values of type " +
20+
$"'{ExpectedType}' but the annotation provider returned an annotation value of type " +
21+
$"'{ActualType?.ToString() ?? "[null]"}'. " +
22+
$"This may be an authoring error in in the annotation provider '{Provider.GetType()}' or in a " +
23+
"typed annotation accessor.";
24+
25+
}
26+
27+
return
28+
$"Typed accessor for annotation '${AnnotationId}' expected collection of values of type '{ExpectedType}' " +
29+
$"but the stored annotation value is of type '{ActualType?.ToString() ?? "[null]"}'. " +
30+
$"This may be an authoring error in a typed annotation accessor, or the annotation may have been stored " +
31+
$"directly with the incorrect type, bypassing the typed accessors.";
32+
}
33+
}
34+
}

0 commit comments

Comments
 (0)