From f3ebc4531257ab7800006ebc5a775105da17765b Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Fri, 1 May 2026 07:49:00 -0400 Subject: [PATCH] Fix typed global options DI binding --- src/Repl.Core/CoreReplApp.Execution.cs | 1 + src/Repl.Core/CoreReplApp.cs | 10 +- .../Documentation/DocumentationEngine.cs | 15 +- .../Internal/Options/OptionSchemaBuilder.cs | 43 +++--- .../Parsing/GlobalOptionDefinition.cs | 3 +- .../Parsing/HandlerArgumentBinder.cs | 13 ++ .../ImplicitServiceParameterRegistry.cs | 67 +++++++++ .../Parsing/InvocationBindingContext.cs | 4 + src/Repl.Core/ParsingOptions.cs | 25 +++- src/Repl.Core/Routing/RoutingEngine.cs | 2 + src/Repl.Core/Session/InteractiveSession.cs | 1 + src/Repl.Defaults/GlobalOptionsExtensions.cs | 25 ++-- .../Given_GlobalOptionsAccessor.cs | 136 ++++++++++++++++-- .../Given_HelpDiscovery.cs | 14 ++ .../Given_CommandBuilderEnrichment.cs | 14 ++ 15 files changed, 315 insertions(+), 58 deletions(-) create mode 100644 src/Repl.Core/Parsing/ImplicitServiceParameterRegistry.cs diff --git a/src/Repl.Core/CoreReplApp.Execution.cs b/src/Repl.Core/CoreReplApp.Execution.cs index f2b0147..4717f7d 100644 --- a/src/Repl.Core/CoreReplApp.Execution.cs +++ b/src/Repl.Core/CoreReplApp.Execution.cs @@ -844,6 +844,7 @@ private InvocationBindingContext CreateInvocationBindingContext( _options.Parsing.NumericFormatProvider, serviceProvider, _options.Interaction, + _implicitServiceParameters, cancellationToken); } diff --git a/src/Repl.Core/CoreReplApp.cs b/src/Repl.Core/CoreReplApp.cs index ba52d49..8542474 100644 --- a/src/Repl.Core/CoreReplApp.cs +++ b/src/Repl.Core/CoreReplApp.cs @@ -19,6 +19,7 @@ public sealed partial class CoreReplApp : ICoreReplApp private readonly List _routes = []; private readonly List> _middleware = []; private readonly ReplOptions _options = new(); + private readonly ImplicitServiceParameterRegistry _implicitServiceParameters = new(); private readonly List _moduleRegistrations = []; private readonly Stack _moduleMappingScope = new(); private int _nextModuleId = 1; @@ -37,6 +38,7 @@ public sealed partial class CoreReplApp : ICoreReplApp internal string? Description => _description; internal IGlobalOptionsAccessor GlobalOptionsAccessor => _globalOptionsSnapshot; internal GlobalOptionsSnapshot GlobalOptionsSnapshotInstance => _globalOptionsSnapshot; + internal ImplicitServiceParameterRegistry ImplicitServiceParameters => _implicitServiceParameters; internal ShellCompletionRuntime ShellCompletionRuntimeInstance => _shellCompletionRuntime; internal IReplExecutionObserver? ExecutionObserver { get; set; } internal List Contexts => _contexts; @@ -61,6 +63,12 @@ private CoreReplApp() static context => context.Channel is ReplRuntimeChannel.Cli or ReplRuntimeChannel.Interactive); } + internal void RegisterGlobalOptionsType(Type optionsType) + { + ArgumentNullException.ThrowIfNull(optionsType); + _implicitServiceParameters.AddGlobalOptionsType(optionsType); + } + /// /// Creates a dependency-free REPL application instance. /// @@ -172,7 +180,7 @@ public CommandBuilder Map(string route, Delegate handler) .Select(existingRoute => existingRoute.Template)); _commands.Add(command); - var optionSchema = OptionSchemaBuilder.Build(template, command, _options.Parsing); + var optionSchema = OptionSchemaBuilder.Build(template, command, _options.Parsing, _implicitServiceParameters); var routeDefinition = new RouteDefinition(template, command, moduleId, optionSchema); _routes.Add(routeDefinition); InvalidateRouting(); diff --git a/src/Repl.Core/Documentation/DocumentationEngine.cs b/src/Repl.Core/Documentation/DocumentationEngine.cs index 6247590..932d6e6 100644 --- a/src/Repl.Core/Documentation/DocumentationEngine.cs +++ b/src/Repl.Core/Documentation/DocumentationEngine.cs @@ -238,7 +238,7 @@ private ReplDocCommand BuildDocumentationCommand(RouteDefinition route) !string.IsNullOrWhiteSpace(parameter.Name) && parameter.ParameterType != typeof(CancellationToken) && !routeParameterNames.Contains(parameter.Name!) - && !IsFrameworkInjectedParameter(parameter.ParameterType) + && !app.ImplicitServiceParameters.IsImplicitServiceParameter(parameter.ParameterType) && parameter.GetCustomAttribute() is null && parameter.GetCustomAttribute() is null && !Attribute.IsDefined(parameter.ParameterType, typeof(ReplOptionsGroupAttribute), inherit: true)) @@ -298,19 +298,6 @@ internal ReplDocApp BuildDocumentationApp() return new ReplDocApp(name, version, description); } - private static bool IsFrameworkInjectedParameter(Type parameterType) => - parameterType == typeof(IServiceProvider) - || parameterType == typeof(ICoreReplApp) - || parameterType == typeof(CoreReplApp) - || parameterType == typeof(IReplSessionState) - || parameterType == typeof(IReplInteractionChannel) - || parameterType == typeof(IReplIoContext) - || parameterType == typeof(IReplKeyReader) - || string.Equals(parameterType.FullName, "Repl.Mcp.IMcpClientRoots", StringComparison.Ordinal) - || string.Equals(parameterType.FullName, "Repl.Mcp.IMcpSampling", StringComparison.Ordinal) - || string.Equals(parameterType.FullName, "Repl.Mcp.IMcpElicitation", StringComparison.Ordinal) - || string.Equals(parameterType.FullName, "Repl.Mcp.IMcpFeedback", StringComparison.Ordinal); - private static bool IsRequiredParameter(ParameterInfo parameter) { if (parameter.HasDefaultValue) diff --git a/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs b/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs index 9f45d22..bea2252 100644 --- a/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs +++ b/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs @@ -9,11 +9,13 @@ internal static class OptionSchemaBuilder public static OptionSchema Build( RouteTemplate template, CommandBuilder command, - ParsingOptions parsingOptions) + ParsingOptions parsingOptions, + ImplicitServiceParameterRegistry implicitServiceParameters) { ArgumentNullException.ThrowIfNull(template); ArgumentNullException.ThrowIfNull(command); ArgumentNullException.ThrowIfNull(parsingOptions); + ArgumentNullException.ThrowIfNull(implicitServiceParameters); var routeParameterNames = template.Segments .OfType() @@ -25,7 +27,7 @@ public static OptionSchema Build( var groupPositionalPropertyNames = new List(); foreach (var parameter in command.Handler.Method.GetParameters()) { - if (ShouldSkipSchemaParameter(parameter, routeParameterNames)) + if (ShouldSkipSchemaParameter(parameter, routeParameterNames, implicitServiceParameters)) { continue; } @@ -57,11 +59,29 @@ public static OptionSchema Build( private static bool ShouldSkipSchemaParameter( ParameterInfo parameter, - HashSet routeParameterNames) + HashSet routeParameterNames, + ImplicitServiceParameterRegistry implicitServiceParameters) { + var optionAttribute = parameter.GetCustomAttribute(inherit: true); + var argumentAttribute = parameter.GetCustomAttribute(inherit: true); + if (implicitServiceParameters.TryGetGlobalOptionsServiceType(parameter.ParameterType, out var globalOptionsType)) + { + if (optionAttribute is not null || argumentAttribute is not null) + { + var attributeName = optionAttribute is not null + ? nameof(ReplOptionAttribute).Replace("Attribute", string.Empty, StringComparison.Ordinal) + : nameof(ReplArgumentAttribute).Replace("Attribute", string.Empty, StringComparison.Ordinal); + throw new InvalidOperationException( + $"Parameter '{parameter.Name}' uses typed global options '{globalOptionsType.Name}' registered through " + + $"UseGlobalOptions() and cannot declare [{attributeName}]. Remove the attribute or use a separate command options type."); + } + + return true; + } + if (string.IsNullOrWhiteSpace(parameter.Name) || parameter.ParameterType == typeof(CancellationToken) - || IsFrameworkInjectedParameter(parameter) + || implicitServiceParameters.IsImplicitServiceParameter(parameter.ParameterType) || parameter.GetCustomAttribute() is not null || parameter.GetCustomAttribute() is not null) { @@ -73,8 +93,6 @@ private static bool ShouldSkipSchemaParameter( return false; } - var optionAttribute = parameter.GetCustomAttribute(inherit: true); - var argumentAttribute = parameter.GetCustomAttribute(inherit: true); if (optionAttribute is null && argumentAttribute is null) { return true; @@ -190,19 +208,6 @@ private static void AppendValueAliases( } } - private static bool IsFrameworkInjectedParameter(ParameterInfo parameter) => - parameter.ParameterType == typeof(IServiceProvider) - || parameter.ParameterType == typeof(ICoreReplApp) - || parameter.ParameterType == typeof(CoreReplApp) - || parameter.ParameterType == typeof(IReplSessionState) - || parameter.ParameterType == typeof(IReplInteractionChannel) - || parameter.ParameterType == typeof(IReplIoContext) - || parameter.ParameterType == typeof(IReplKeyReader) - || string.Equals(parameter.ParameterType.FullName, "Repl.Mcp.IMcpClientRoots", StringComparison.Ordinal) - || string.Equals(parameter.ParameterType.FullName, "Repl.Mcp.IMcpSampling", StringComparison.Ordinal) - || string.Equals(parameter.ParameterType.FullName, "Repl.Mcp.IMcpElicitation", StringComparison.Ordinal) - || string.Equals(parameter.ParameterType.FullName, "Repl.Mcp.IMcpFeedback", StringComparison.Ordinal); - private static ReplArity ResolveArity(ParameterInfo parameter, ReplOptionAttribute? optionAttribute) { if (optionAttribute?.Arity is { } explicitArity) diff --git a/src/Repl.Core/Parsing/GlobalOptionDefinition.cs b/src/Repl.Core/Parsing/GlobalOptionDefinition.cs index dbcca96..fd79451 100644 --- a/src/Repl.Core/Parsing/GlobalOptionDefinition.cs +++ b/src/Repl.Core/Parsing/GlobalOptionDefinition.cs @@ -5,4 +5,5 @@ internal sealed record GlobalOptionDefinition( string CanonicalToken, IReadOnlyList Aliases, string? DefaultValue, - Type ValueType); + Type ValueType, + Type? OwnerType); diff --git a/src/Repl.Core/Parsing/HandlerArgumentBinder.cs b/src/Repl.Core/Parsing/HandlerArgumentBinder.cs index 1da7f20..68ad55d 100644 --- a/src/Repl.Core/Parsing/HandlerArgumentBinder.cs +++ b/src/Repl.Core/Parsing/HandlerArgumentBinder.cs @@ -53,6 +53,19 @@ internal static class HandlerArgumentBinder $"Unable to bind parameter '{parameter.Name}' ({parameter.ParameterType.Name})."); } + if (context.ImplicitServiceParameters.TryGetGlobalOptionsServiceType(parameter.ParameterType, out var globalOptionsServiceType)) + { + var globalOptions = context.ServiceProvider.GetService(globalOptionsServiceType); + if (globalOptions is not null) + { + return globalOptions; + } + + throw new InvalidOperationException( + $"Unable to resolve typed global options parameter '{parameter.Name}' ({globalOptionsServiceType.Name}) from services. " + + $"Ensure the type is registered through UseGlobalOptions<{globalOptionsServiceType.Name}>() before mapping commands."); + } + #pragma warning disable IL2072 if (IsOptionsGroupParameter(parameter)) { diff --git a/src/Repl.Core/Parsing/ImplicitServiceParameterRegistry.cs b/src/Repl.Core/Parsing/ImplicitServiceParameterRegistry.cs new file mode 100644 index 0000000..368704b --- /dev/null +++ b/src/Repl.Core/Parsing/ImplicitServiceParameterRegistry.cs @@ -0,0 +1,67 @@ +namespace Repl; + +internal sealed class ImplicitServiceParameterRegistry +{ + private readonly HashSet _globalOptionsTypes = []; + + public void AddGlobalOptionsType(Type optionsType) + { + ArgumentNullException.ThrowIfNull(optionsType); + _globalOptionsTypes.Add(optionsType); + } + + public bool IsImplicitServiceParameter(Type parameterType) => + IsFrameworkInjectedParameter(parameterType) + || TryGetGlobalOptionsServiceType(parameterType, out _); + + public bool TryGetGlobalOptionsServiceType(Type parameterType, out Type serviceType) + { + ArgumentNullException.ThrowIfNull(parameterType); + + serviceType = typeof(void); + if (_globalOptionsTypes.Contains(parameterType)) + { + serviceType = parameterType; + return true; + } + + if (parameterType == typeof(object)) + { + return false; + } + + var matches = _globalOptionsTypes + .Where(parameterType.IsAssignableFrom) + .OrderBy(static type => type.FullName, StringComparer.Ordinal) + .ToArray(); + if (matches.Length == 0) + { + return false; + } + + if (matches.Length > 1) + { + throw new InvalidOperationException( + $"Ambiguous typed global options binding for parameter type '{parameterType.Name}'. " + + $"Registered matching types: {string.Join(", ", matches.Select(static type => type.Name))}. " + + "Use the concrete registered options type or an explicit [FromServices] parameter."); + } + + serviceType = matches[0]; + return true; + } + + private static bool IsFrameworkInjectedParameter(Type parameterType) => + parameterType == typeof(IServiceProvider) + || parameterType == typeof(ICoreReplApp) + || parameterType == typeof(CoreReplApp) + || parameterType == typeof(IGlobalOptionsAccessor) + || parameterType == typeof(IReplSessionState) + || parameterType == typeof(IReplInteractionChannel) + || parameterType == typeof(IReplIoContext) + || parameterType == typeof(IReplKeyReader) + || string.Equals(parameterType.FullName, "Repl.Mcp.IMcpClientRoots", StringComparison.Ordinal) + || string.Equals(parameterType.FullName, "Repl.Mcp.IMcpSampling", StringComparison.Ordinal) + || string.Equals(parameterType.FullName, "Repl.Mcp.IMcpElicitation", StringComparison.Ordinal) + || string.Equals(parameterType.FullName, "Repl.Mcp.IMcpFeedback", StringComparison.Ordinal); +} diff --git a/src/Repl.Core/Parsing/InvocationBindingContext.cs b/src/Repl.Core/Parsing/InvocationBindingContext.cs index 64f311c..a709594 100644 --- a/src/Repl.Core/Parsing/InvocationBindingContext.cs +++ b/src/Repl.Core/Parsing/InvocationBindingContext.cs @@ -12,6 +12,7 @@ internal sealed class InvocationBindingContext( IFormatProvider numericFormatProvider, IServiceProvider serviceProvider, InteractionOptions interactionOptions, + ImplicitServiceParameterRegistry implicitServiceParameters, CancellationToken cancellationToken) { public IReadOnlyDictionary RouteValues { get; } = routeValues; @@ -33,4 +34,7 @@ internal sealed class InvocationBindingContext( public InteractionOptions InteractionOptions { get; } = interactionOptions; public CancellationToken CancellationToken { get; } = cancellationToken; + + public ImplicitServiceParameterRegistry ImplicitServiceParameters { get; } = + implicitServiceParameters ?? throw new ArgumentNullException(nameof(implicitServiceParameters)); } diff --git a/src/Repl.Core/ParsingOptions.cs b/src/Repl.Core/ParsingOptions.cs index 6e182bb..b0b25c5 100644 --- a/src/Repl.Core/ParsingOptions.cs +++ b/src/Repl.Core/ParsingOptions.cs @@ -117,16 +117,16 @@ public void AddGlobalOption(string name, string[]? aliases = null, T? default public void AddGlobalOption(string name, string constraintOrTypeName, string[]? aliases = null, string? defaultValue = null) => AddGlobalOptionCore(name, ResolveConstraintOrTypeName(constraintOrTypeName, _customRouteConstraints), aliases, defaultValue); - internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases, string? defaultValue) + internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases, string? defaultValue, Type? ownerType = null) { name = string.IsNullOrWhiteSpace(name) ? throw new ArgumentException("Global option name cannot be empty.", nameof(name)) : name.Trim(); var normalizedCanonical = NormalizeLongToken(name); - if (_globalOptions.ContainsKey(name)) + if (_globalOptions.TryGetValue(name, out var existing)) { - throw new InvalidOperationException($"A global option named '{name}' is already registered."); + throw new InvalidOperationException(BuildDuplicateGlobalOptionMessage(name, existing.OwnerType, ownerType)); } var normalizedAliases = (aliases ?? []) @@ -141,7 +141,24 @@ internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases CanonicalToken: normalizedCanonical, Aliases: normalizedAliases, DefaultValue: defaultValue, - ValueType: valueType); + ValueType: valueType, + OwnerType: ownerType); + } + + private static string BuildDuplicateGlobalOptionMessage(string name, Type? existingOwner, Type? newOwner) + { + if (existingOwner is null && newOwner is null) + { + return $"A global option named '{name}' is already registered."; + } + + var existingSource = existingOwner is null + ? "another registration" + : $"typed global options '{existingOwner.Name}'"; + var newSource = newOwner is null + ? "this registration" + : $"typed global options '{newOwner.Name}'"; + return $"A global option named '{name}' is already registered by {existingSource} and cannot also be registered by {newSource}."; } private static Type ResolveConstraintOrTypeName( diff --git a/src/Repl.Core/Routing/RoutingEngine.cs b/src/Repl.Core/Routing/RoutingEngine.cs index f5c830f..ddd11c7 100644 --- a/src/Repl.Core/Routing/RoutingEngine.cs +++ b/src/Repl.Core/Routing/RoutingEngine.cs @@ -189,6 +189,7 @@ internal async ValueTask ValidateContextAsync( app.OptionsSnapshot.Parsing.NumericFormatProvider, serviceProvider, app.OptionsSnapshot.Interaction, + app.ImplicitServiceParameters, cancellationToken); var arguments = HandlerArgumentBinder.Bind(contextMatch.Context.Validation, bindingContext); var validationResult = await CommandInvoker @@ -340,6 +341,7 @@ internal async ValueTask InvokeBannerAsync( numericFormatProvider: app.OptionsSnapshot.Parsing.NumericFormatProvider, serviceProvider: serviceProvider, interactionOptions: app.OptionsSnapshot.Interaction, + implicitServiceParameters: app.ImplicitServiceParameters, cancellationToken: cancellationToken); var arguments = HandlerArgumentBinder.Bind(banner, bindingContext); var result = await CommandInvoker.InvokeAsync(banner, arguments).ConfigureAwait(false); diff --git a/src/Repl.Core/Session/InteractiveSession.cs b/src/Repl.Core/Session/InteractiveSession.cs index 147502b..d822b13 100644 --- a/src/Repl.Core/Session/InteractiveSession.cs +++ b/src/Repl.Core/Session/InteractiveSession.cs @@ -545,6 +545,7 @@ private async ValueTask ExecuteCustomAmbientCommandAsync( numericFormatProvider: app.OptionsSnapshot.Parsing.NumericFormatProvider ?? CultureInfo.InvariantCulture, serviceProvider: serviceProvider, interactionOptions: app.OptionsSnapshot.Interaction, + implicitServiceParameters: app.ImplicitServiceParameters, cancellationToken: cancellationToken); var arguments = HandlerArgumentBinder.Bind(command.Handler, bindingContext); await CommandInvoker.InvokeAsync(command.Handler, arguments).ConfigureAwait(false); diff --git a/src/Repl.Defaults/GlobalOptionsExtensions.cs b/src/Repl.Defaults/GlobalOptionsExtensions.cs index 4a0185b..af3c1f6 100644 --- a/src/Repl.Defaults/GlobalOptionsExtensions.cs +++ b/src/Repl.Defaults/GlobalOptionsExtensions.cs @@ -24,26 +24,27 @@ public static class GlobalOptionsExtensions { ArgumentNullException.ThrowIfNull(app); - ParsingOptions? capturedParsing = null; + var parsing = app.Core.OptionsSnapshot.Parsing; + var properties = GetOptionProperties(); app.Options(options => { - capturedParsing = options.Parsing; var prototype = new T(); - foreach (var property in GetOptionProperties()) + foreach (var property in properties) { var optionAttr = property.GetCustomAttribute(); var name = optionAttr?.Name ?? ToKebabCase(property.Name); var aliases = optionAttr?.Aliases; var defaultValue = property.GetValue(prototype)?.ToString(); - options.Parsing.AddGlobalOptionCore(name, property.PropertyType, aliases, defaultValue); + options.Parsing.AddGlobalOptionCore(name, property.PropertyType, aliases, defaultValue, typeof(T)); } }); + app.Core.RegisterGlobalOptionsType(typeof(T)); app.ServiceDescriptors.TryAddTransient(sp => { var accessor = sp.GetRequiredService(); - return PopulateInstance(accessor, capturedParsing!.NumericFormatProvider); + return PopulateInstance(accessor, parsing.NumericFormatProvider); }); return app; @@ -74,10 +75,16 @@ public static class GlobalOptionsExtensions return instance; } - private static PropertyInfo[] GetOptionProperties<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>() => - typeof(T).GetProperties(BindingFlags.Public | BindingFlags.Instance) - .Where(p => p.CanWrite) - .ToArray(); + private static IReadOnlyList GetOptionProperties<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>() => + GlobalOptionsMetadata.Properties; + + private static class GlobalOptionsMetadata<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T> + { + internal static readonly IReadOnlyList Properties = + typeof(T).GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.CanWrite) + .ToArray(); + } private static string ToKebabCase(string pascalCase) { diff --git a/src/Repl.IntegrationTests/Given_GlobalOptionsAccessor.cs b/src/Repl.IntegrationTests/Given_GlobalOptionsAccessor.cs index a82312c..4cd8946 100644 --- a/src/Repl.IntegrationTests/Given_GlobalOptionsAccessor.cs +++ b/src/Repl.IntegrationTests/Given_GlobalOptionsAccessor.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using Repl.Parameters; namespace Repl.IntegrationTests; @@ -95,6 +96,101 @@ public void When_UsingTypedGlobalOptions_Then_ClassIsPopulatedFromParsedValues() output.Text.Should().Contain("acme:9090"); } + [TestMethod] + [Description("UseGlobalOptions handler parameters are injected from DI even when the parameter name matches a global option.")] + public void When_TypedGlobalOptionsParameterNameMatchesGlobalOption_Then_HandlerReceivesDiInstance() + { + var sut = ReplApp.Create(); + sut.UseGlobalOptions(); + sut.Map("show", (TestGlobalOptions tenant) => tenant.Tenant ?? "none"); + + var output = ConsoleCaptureHelper.Capture( + () => sut.Run(["show", "--tenant", "acme", "--no-logo"])); + + output.ExitCode.Should().Be(0, output.Text); + output.Text.Should().Contain("acme"); + output.Text.Should().NotContain("Ambiguous option"); + } + + [TestMethod] + [Description("UseGlobalOptions supports handler parameters typed as an implemented options interface.")] + public void When_TypedGlobalOptionsParameterUsesImplementedInterface_Then_HandlerReceivesDiInstance() + { + var sut = ReplApp.Create(); + sut.UseGlobalOptions(); + sut.Map("show", (IInterfaceGlobalOptions tenant) => tenant.Tenant ?? "none"); + + var output = ConsoleCaptureHelper.Capture( + () => sut.Run(["show", "--tenant", "acme", "--no-logo"])); + + output.ExitCode.Should().Be(0, output.Text); + output.Text.Should().Contain("acme"); + output.Text.Should().NotContain("Ambiguous option"); + } + + [TestMethod] + [Description("UseGlobalOptions rejects command binding attributes on typed global-options parameters.")] + public void When_TypedGlobalOptionsParameterDeclaresReplOption_Then_MappingFailsClearly() + { + var sut = ReplApp.Create(); + sut.UseGlobalOptions(); + + var act = () => sut.Map( + "show", + ([ReplOption(Name = "tenant")] TestGlobalOptions options) => options.Tenant ?? "none"); + + var exception = act.Should().Throw().Which; + exception.Message.Should().Contain("UseGlobalOptions"); + exception.Message.Should().Contain(nameof(TestGlobalOptions)); + exception.Message.Should().Contain("ReplOption"); + } + + [TestMethod] + [Description("UseGlobalOptions rejects positional binding attributes on typed global-options parameters.")] + public void When_TypedGlobalOptionsParameterDeclaresReplArgument_Then_MappingFailsClearly() + { + var sut = ReplApp.Create(); + sut.UseGlobalOptions(); + + var act = () => sut.Map( + "show", + ([ReplArgument] TestGlobalOptions options) => options.Tenant ?? "none"); + + var exception = act.Should().Throw().Which; + exception.Message.Should().Contain("UseGlobalOptions"); + exception.Message.Should().Contain(nameof(TestGlobalOptions)); + exception.Message.Should().Contain("ReplArgument"); + } + + [TestMethod] + [Description("UseGlobalOptions reports typed-options DI registration issues with actionable diagnostics.")] + public void When_TypedGlobalOptionsServiceIsMissing_Then_RuntimeErrorMentionsUseGlobalOptions() + { + var sut = CoreReplApp.Create(); + sut.RegisterGlobalOptionsType(typeof(MissingServiceGlobalOptions)); + sut.Map("show", (MissingServiceGlobalOptions options) => options.Tenant ?? "none"); + + var output = ConsoleCaptureHelper.Capture( + () => sut.Run(["show", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("UseGlobalOptions"); + output.Text.Should().Contain(nameof(MissingServiceGlobalOptions)); + } + + [TestMethod] + [Description("UseGlobalOptions duplicate property names report the typed options classes involved.")] + public void When_TypedGlobalOptionsPropertyNamesCollide_Then_ErrorMentionsBothTypes() + { + var sut = ReplApp.Create(); + sut.UseGlobalOptions(); + + var act = () => sut.UseGlobalOptions(); + + act.Should().Throw() + .WithMessage("*tenant*TestGlobalOptions*DuplicateTenantGlobalOptions*"); + } + [TestMethod] [Description("UseGlobalOptions properties without values keep defaults.")] public void When_UsingTypedGlobalOptionsWithoutValues_Then_DefaultsAreKept() @@ -157,7 +253,7 @@ public void When_UsingCoreReplApp_Then_AccessorIsAvailableInHandler() [TestMethod] [Description("UseGlobalOptions returns fresh values on each resolution (not stale singleton).")] - public async Task When_TypedOptionsResolvedMultipleTimes_Then_ReflectsLatestValues() + public void When_TypedOptionsResolvedMultipleTimes_Then_ReflectsLatestValues() { var sut = ReplApp.Create(); sut.UseGlobalOptions(); @@ -210,15 +306,6 @@ public void When_NumericCultureIsCurrent_Then_TypedOptionsUsesConfiguredCulture( } } - private sealed record TenantConfig(string Name); - - private sealed class TestGlobalOptions - { - public string? Tenant { get; set; } - - public int Port { get; set; } = 8080; - } - [TestMethod] [Description("UseGlobalOptions converts consecutive uppercase property names to kebab-case correctly.")] public void When_PropertyHasConsecutiveUppercase_Then_KebabCaseIsCorrect() @@ -295,4 +382,33 @@ private sealed class AcronymGlobalOptions { public int XMLPort { get; set; } } + + private sealed record TenantConfig(string Name); + + private sealed class TestGlobalOptions + { + public string? Tenant { get; set; } + + public int Port { get; set; } = 8080; + } + + private interface IInterfaceGlobalOptions + { + string? Tenant { get; } + } + + private sealed class InterfaceGlobalOptions : IInterfaceGlobalOptions + { + public string? Tenant { get; set; } + } + + private sealed class MissingServiceGlobalOptions + { + public string? Tenant { get; set; } + } + + private sealed class DuplicateTenantGlobalOptions + { + public string? Tenant { get; set; } + } } diff --git a/src/Repl.IntegrationTests/Given_HelpDiscovery.cs b/src/Repl.IntegrationTests/Given_HelpDiscovery.cs index 53c3382..c521d47 100644 --- a/src/Repl.IntegrationTests/Given_HelpDiscovery.cs +++ b/src/Repl.IntegrationTests/Given_HelpDiscovery.cs @@ -417,6 +417,20 @@ public void When_RequestingCommandHelpWithDeclaredOptions_Then_OptionsSectionInc output.Text.Should().Contain("--verbose, --no-verbose"); } + [TestMethod] + [Description("Regression guard: verifies injected global-options accessor parameters are omitted from command help.")] + public void When_RequestingCommandHelpWithGlobalOptionsAccessor_Then_AccessorIsNotListedAsCommandOption() + { + var sut = ReplApp.Create(); + sut.Options(options => options.Parsing.AddGlobalOption("tenant")); + sut.Map("show", (IGlobalOptionsAccessor globals) => globals.GetValue("tenant") ?? "none"); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["show", "--help", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().NotContain("--globals"); + } + private static string SendHandler([ComponentDescriptionAttribute("Message to send to all watching sessions")] string message) => message; private enum HelpMode diff --git a/src/Repl.Tests/Given_CommandBuilderEnrichment.cs b/src/Repl.Tests/Given_CommandBuilderEnrichment.cs index f1ffcf3..5d03633 100644 --- a/src/Repl.Tests/Given_CommandBuilderEnrichment.cs +++ b/src/Repl.Tests/Given_CommandBuilderEnrichment.cs @@ -223,6 +223,20 @@ public void When_HandlerHasDescriptionAttribute_Then_ArgumentDescriptionIsPopula arg.Description.Should().Be("Contact numeric id"); } + [TestMethod] + [Description("Verifies injected IGlobalOptionsAccessor parameters are omitted from documentation options.")] + public void When_HandlerUsesGlobalOptionsAccessor_Then_DocumentationOmitsAccessorOption() + { + var sut = CoreReplApp.Create(); + sut.Options(options => options.Parsing.AddGlobalOption("tenant")); + sut.Map("show", (IGlobalOptionsAccessor globals) => globals.GetValue("tenant") ?? "none"); + + var model = sut.CreateDocumentationModel("show"); + + model.Commands.Should().ContainSingle(c => c.Path == "show").Which + .Options.Should().NotContain(option => option.Name == "globals"); + } + // ── ReplRuntimeChannel.Programmatic ──────────────────────────────── [TestMethod]