diff --git a/docs/parameter-system.md b/docs/parameter-system.md index 8c2771ea..472d90b5 100644 --- a/docs/parameter-system.md +++ b/docs/parameter-system.md @@ -29,7 +29,6 @@ Application-facing parameter DSL: - optional per-parameter `CaseSensitivity` - optional `Arity` - `ReplArgumentAttribute` - - optional positional `Position` - `Mode` - `ReplValueAliasAttribute` - maps a token to an injected parameter value (for example `--json` -> `output=json`) diff --git a/src/Repl.Core/Documentation/DocumentationEngine.cs b/src/Repl.Core/Documentation/DocumentationEngine.cs index 324b7956..8c77d2e2 100644 --- a/src/Repl.Core/Documentation/DocumentationEngine.cs +++ b/src/Repl.Core/Documentation/DocumentationEngine.cs @@ -294,6 +294,13 @@ internal ReplDocApp BuildDocumentationApp() return new ReplDocApp(name, version, description); } + // An explicit OneOrMore/ExactlyOne arity now fails binding when the option is absent + // (HandlerArgumentBinder), so exported docs must report it as required regardless of the + // CLR parameter shape. + private static bool HasExplicitRequiredArity(OptionSchema schema, string parameterName) => + schema.TryGetParameter(parameterName, out var schemaParameter) + && schemaParameter.ExplicitArity is ReplArity.OneOrMore or ReplArity.ExactlyOne; + private static bool IsRequiredParameter(ParameterInfo parameter) { if (parameter.HasDefaultValue) @@ -406,7 +413,7 @@ private static ReplDocOption BuildDocumentationOptionFromProperty( return new ReplDocOption( Name: aliases.Length > 0 ? aliases[0].TrimStart('-') : property.Name, Type: GetFriendlyTypeName(property.PropertyType), - Required: false, + Required: HasExplicitRequiredArity(schema, property.Name), Description: property.GetCustomAttribute()?.Description, Aliases: aliases, ReverseAliases: reverseAliases, @@ -446,7 +453,7 @@ private static ReplDocOption BuildDocumentationOption(OptionSchema schema, Param return new ReplDocOption( Name: aliases.Length > 0 ? aliases[0].TrimStart('-') : parameter.Name!, Type: GetFriendlyTypeName(parameter.ParameterType), - Required: IsRequiredParameter(parameter), + Required: IsRequiredParameter(parameter) || HasExplicitRequiredArity(schema, parameter.Name!), Description: parameter.GetCustomAttribute()?.Description, Aliases: aliases, ReverseAliases: reverseAliases, diff --git a/src/Repl.Core/Internal/Options/OptionSchema.cs b/src/Repl.Core/Internal/Options/OptionSchema.cs index 060410c9..5f88fdce 100644 --- a/src/Repl.Core/Internal/Options/OptionSchema.cs +++ b/src/Repl.Core/Internal/Options/OptionSchema.cs @@ -22,8 +22,11 @@ public OptionSchema( // Benign race: concurrent first reads compute the same array. private string[]? _knownTokens; + // Ordinal dedup: case-differing tokens can belong to DIFFERENT case-sensitive options + // (per-entry overrides), so collapsing them ignoring case would drop a real token and + // make "Did you mean" suggest the wrong casing. public IReadOnlyCollection KnownTokens => - _knownTokens ??= [.. Entries.Select(entry => entry.Token).Distinct(StringComparer.OrdinalIgnoreCase)]; + _knownTokens ??= [.. Entries.Select(entry => entry.Token).Distinct(StringComparer.Ordinal)]; public IReadOnlyList ResolveToken(string token, ReplCaseSensitivity globalCaseSensitivity) { @@ -50,4 +53,32 @@ public IReadOnlyList ResolveToken(string token, ReplCaseSensi public bool TryGetParameter(string parameterName, out OptionSchemaParameter parameter) => Parameters.TryGetValue(parameterName, out parameter!); + + /// + /// Effective arity of a parameter, resolved from its named-option/flag entry; + /// parameters without such an entry default to the permissive . + /// + public ReplArity ResolveParameterArity(string parameterName) => + FindNamedEntry(parameterName)?.Arity ?? ReplArity.ZeroOrMore; + + /// + /// Canonical display token of a parameter's named-option/flag entry (with prefix), + /// or null for parameters without one (e.g. ArgumentOnly). + /// + public string? ResolveDisplayToken(string parameterName) => + FindNamedEntry(parameterName)?.Token; + + private OptionSchemaEntry? FindNamedEntry(string parameterName) + { + foreach (var entry in Entries) + { + if (string.Equals(entry.ParameterName, parameterName, StringComparison.OrdinalIgnoreCase) + && entry.TokenKind is OptionSchemaTokenKind.NamedOption or OptionSchemaTokenKind.BoolFlag) + { + return entry; + } + } + + return null; + } } diff --git a/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs b/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs index bea2252b..d1e69bde 100644 --- a/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs +++ b/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs @@ -53,7 +53,7 @@ public static OptionSchema Build( } ValidatePositionalBindingCompatibility(regularPositionalParameterNames, groupPositionalPropertyNames); - ValidateTokenCollisions(entries, parsingOptions); + ValidateTokenCollisions(entries, parsingOptions, template); return new OptionSchema(entries, parameters); } @@ -112,14 +112,15 @@ private static void AppendParameterSchemaEntries( if (parameters.ContainsKey(parameter.Name!)) { throw new InvalidOperationException( - $"Option token collision detected for parameter name '{parameter.Name}'."); + $"Duplicate parameter name '{parameter.Name}' in the command signature."); } parameters[parameter.Name!] = new OptionSchemaParameter( parameter.Name!, parameter.ParameterType, mode, - CaseSensitivity: optionAttribute?.CaseSensitivity); + CaseSensitivity: optionAttribute?.CaseSensitivityOverride, + ExplicitArity: optionAttribute?.ArityOverride); if (mode == ReplParameterMode.ArgumentOnly) { return; @@ -133,7 +134,7 @@ private static void AppendParameterSchemaEntries( parameter.Name!, tokenKind, arity, - CaseSensitivity: optionAttribute?.CaseSensitivity)); + CaseSensitivity: optionAttribute?.CaseSensitivityOverride)); AppendOptionAliases(parameter, tokenKind, arity, optionAttribute, entries); AppendReverseAliases(parameter, optionAttribute, entries); AppendValueAliases(parameter, optionAttribute, entries); @@ -168,7 +169,7 @@ private static void AppendOptionAliases( parameter.Name!, tokenKind, arity, - CaseSensitivity: optionAttribute?.CaseSensitivity)); + CaseSensitivity: optionAttribute?.CaseSensitivityOverride)); } } @@ -185,7 +186,7 @@ private static void AppendReverseAliases( parameter.Name!, OptionSchemaTokenKind.ReverseFlag, ReplArity.ZeroOrOne, - CaseSensitivity: optionAttribute?.CaseSensitivity, + CaseSensitivity: optionAttribute?.CaseSensitivityOverride, InjectedValue: "false")); } } @@ -203,14 +204,14 @@ private static void AppendValueAliases( parameter.Name!, OptionSchemaTokenKind.ValueAlias, ReplArity.ZeroOrOne, - CaseSensitivity: valueAlias.CaseSensitivity ?? optionAttribute?.CaseSensitivity, + CaseSensitivity: valueAlias.CaseSensitivityOverride ?? optionAttribute?.CaseSensitivityOverride, InjectedValue: valueAlias.Value)); } } private static ReplArity ResolveArity(ParameterInfo parameter, ReplOptionAttribute? optionAttribute) { - if (optionAttribute?.Arity is { } explicitArity) + if (optionAttribute?.ArityOverride is { } explicitArity) { return explicitArity; } @@ -293,7 +294,7 @@ private static void AppendEnumAliases( parameter.Name!, OptionSchemaTokenKind.EnumAlias, ReplArity.ZeroOrOne, - CaseSensitivity: enumFlag.CaseSensitivity ?? optionAttribute?.CaseSensitivity, + CaseSensitivity: enumFlag.CaseSensitivityOverride ?? optionAttribute?.CaseSensitivityOverride, InjectedValue: field.Name)); } } @@ -405,14 +406,15 @@ private static void AppendPropertySchemaEntries( if (parameters.ContainsKey(property.Name)) { throw new InvalidOperationException( - $"Option token collision detected for parameter name '{property.Name}'."); + $"Duplicate parameter name '{property.Name}' between options group '{property.DeclaringType?.Name}' and the command signature."); } parameters[property.Name] = new OptionSchemaParameter( property.Name, property.PropertyType, mode, - CaseSensitivity: optionAttribute?.CaseSensitivity); + CaseSensitivity: optionAttribute?.CaseSensitivityOverride, + ExplicitArity: optionAttribute?.ArityOverride); if (mode != ReplParameterMode.OptionOnly) { positionalPropertyNames.Add(property.Name); @@ -430,7 +432,7 @@ private static void AppendPropertySchemaEntries( property.Name, tokenKind, arity, - CaseSensitivity: optionAttribute?.CaseSensitivity)); + CaseSensitivity: optionAttribute?.CaseSensitivityOverride)); AppendPropertyOptionAliases(property.Name, tokenKind, arity, optionAttribute, entries); AppendPropertyReverseAliases(property.Name, optionAttribute, entries); AppendPropertyValueAliases(property, optionAttribute, entries); @@ -439,7 +441,7 @@ private static void AppendPropertySchemaEntries( private static ReplArity ResolvePropertyArity(Type propertyType, ReplOptionAttribute? optionAttribute) { - if (optionAttribute?.Arity is { } explicitArity) + if (optionAttribute?.ArityOverride is { } explicitArity) { return explicitArity; } @@ -472,7 +474,7 @@ private static void AppendPropertyOptionAliases( propertyName, tokenKind, arity, - CaseSensitivity: optionAttribute?.CaseSensitivity)); + CaseSensitivity: optionAttribute?.CaseSensitivityOverride)); } } @@ -489,7 +491,7 @@ private static void AppendPropertyReverseAliases( propertyName, OptionSchemaTokenKind.ReverseFlag, ReplArity.ZeroOrOne, - CaseSensitivity: optionAttribute?.CaseSensitivity, + CaseSensitivity: optionAttribute?.CaseSensitivityOverride, InjectedValue: "false")); } } @@ -507,7 +509,7 @@ private static void AppendPropertyValueAliases( property.Name, OptionSchemaTokenKind.ValueAlias, ReplArity.ZeroOrOne, - CaseSensitivity: valueAlias.CaseSensitivity ?? optionAttribute?.CaseSensitivity, + CaseSensitivity: valueAlias.CaseSensitivityOverride ?? optionAttribute?.CaseSensitivityOverride, InjectedValue: valueAlias.Value)); } } @@ -541,7 +543,7 @@ private static void AppendPropertyEnumAliases( property.Name, OptionSchemaTokenKind.EnumAlias, ReplArity.ZeroOrOne, - CaseSensitivity: enumFlag.CaseSensitivity ?? optionAttribute?.CaseSensitivity, + CaseSensitivity: enumFlag.CaseSensitivityOverride ?? optionAttribute?.CaseSensitivityOverride, InjectedValue: field.Name)); } } @@ -549,29 +551,49 @@ private static void AppendPropertyEnumAliases( private static void ValidateTokenCollisions( IReadOnlyList entries, - ParsingOptions parsingOptions) + ParsingOptions parsingOptions, + RouteTemplate template) { - var comparer = parsingOptions.OptionCaseSensitivity == ReplCaseSensitivity.CaseInsensitive - ? StringComparer.OrdinalIgnoreCase - : StringComparer.Ordinal; - var map = new Dictionary(comparer); + // Collisions are decided per PAIR under each entry's effective case sensitivity, not + // under the global comparer alone: ordinal-equal tokens always collide, while tokens + // differing only by casing collide only when BOTH entries are effectively + // case-insensitive (full mutual shadowing — no typed token can distinguish them). + // Two explicitly case-sensitive entries stay distinguishable ordinally even under a + // global case-insensitive default, and a mixed pair keeps per-token runtime + // resolution (the parser's "Ambiguous option" diagnostic) instead of failing + // registration. Buckets are keyed ignoring case to surface every candidate pair. + var map = new Dictionary>(StringComparer.OrdinalIgnoreCase); foreach (var entry in entries) { - if (!map.TryGetValue(entry.Token, out var existing)) + if (!map.TryGetValue(entry.Token, out var bucket)) { - map[entry.Token] = entry; + map[entry.Token] = [entry]; continue; } - if (string.Equals(existing.ParameterName, entry.ParameterName, StringComparison.OrdinalIgnoreCase) - && existing.TokenKind == entry.TokenKind - && string.Equals(existing.InjectedValue, entry.InjectedValue, StringComparison.Ordinal)) + foreach (var existing in bucket) { - continue; + var ordinalEqual = string.Equals(existing.Token, entry.Token, StringComparison.Ordinal); + var bothInsensitive = + (existing.CaseSensitivity ?? parsingOptions.OptionCaseSensitivity) == ReplCaseSensitivity.CaseInsensitive + && (entry.CaseSensitivity ?? parsingOptions.OptionCaseSensitivity) == ReplCaseSensitivity.CaseInsensitive; + if (!ordinalEqual && !bothInsensitive) + { + continue; + } + + if (string.Equals(existing.ParameterName, entry.ParameterName, StringComparison.OrdinalIgnoreCase) + && existing.TokenKind == entry.TokenKind + && string.Equals(existing.InjectedValue, entry.InjectedValue, StringComparison.Ordinal)) + { + continue; + } + + throw new InvalidOperationException( + $"Option token collision detected for '{entry.Token}' between '{existing.ParameterName}' and '{entry.ParameterName}' while registering command '{template.Template}'."); } - throw new InvalidOperationException( - $"Option token collision detected for '{entry.Token}' between '{existing.ParameterName}' and '{entry.ParameterName}'."); + bucket.Add(entry); } } } diff --git a/src/Repl.Core/Internal/Options/OptionSchemaParameter.cs b/src/Repl.Core/Internal/Options/OptionSchemaParameter.cs index 3bf67141..e506c84d 100644 --- a/src/Repl.Core/Internal/Options/OptionSchemaParameter.cs +++ b/src/Repl.Core/Internal/Options/OptionSchemaParameter.cs @@ -6,4 +6,5 @@ internal sealed record OptionSchemaParameter( string Name, Type ParameterType, ReplParameterMode Mode, - ReplCaseSensitivity? CaseSensitivity = null); + ReplCaseSensitivity? CaseSensitivity = null, + ReplArity? ExplicitArity = null); diff --git a/src/Repl.Core/Parameters/Attributes/ReplArgumentAttribute.cs b/src/Repl.Core/Parameters/Attributes/ReplArgumentAttribute.cs index 78f1eb45..2769e016 100644 --- a/src/Repl.Core/Parameters/Attributes/ReplArgumentAttribute.cs +++ b/src/Repl.Core/Parameters/Attributes/ReplArgumentAttribute.cs @@ -6,11 +6,6 @@ namespace Repl.Parameters; [AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] public sealed class ReplArgumentAttribute : Attribute { - /// - /// Optional explicit position for positional binding. - /// - public int? Position { get; set; } - /// /// Binding mode for the parameter. /// diff --git a/src/Repl.Core/Parameters/Attributes/ReplEnumFlagAttribute.cs b/src/Repl.Core/Parameters/Attributes/ReplEnumFlagAttribute.cs index 5c5f7165..dfb73bd9 100644 --- a/src/Repl.Core/Parameters/Attributes/ReplEnumFlagAttribute.cs +++ b/src/Repl.Core/Parameters/Attributes/ReplEnumFlagAttribute.cs @@ -20,8 +20,25 @@ public ReplEnumFlagAttribute(params string[] aliases) /// public string[] Aliases { get; } + // Nullable enums are not legal attribute named arguments (CS0655), so the optional + // override exposes a non-nullable property and tracks the unset state in a nullable + // backing field surfaced through the read-only CaseSensitivityOverride property. + private ReplCaseSensitivity? _caseSensitivity; + /// /// Optional case-sensitivity override for these aliases. + /// Only an explicit assignment overrides the global parsing default; when unset, the getter + /// returns the enum default and does not reflect the effective behavior — read + /// to distinguish unset from an explicit value. + /// + public ReplCaseSensitivity CaseSensitivity + { + get => _caseSensitivity ?? default; + set => _caseSensitivity = value; + } + + /// + /// Explicit case-sensitivity override, or null to inherit the global default. /// - public ReplCaseSensitivity? CaseSensitivity { get; set; } + public ReplCaseSensitivity? CaseSensitivityOverride => _caseSensitivity; } diff --git a/src/Repl.Core/Parameters/Attributes/ReplOptionAttribute.cs b/src/Repl.Core/Parameters/Attributes/ReplOptionAttribute.cs index 3480496c..96858fab 100644 --- a/src/Repl.Core/Parameters/Attributes/ReplOptionAttribute.cs +++ b/src/Repl.Core/Parameters/Attributes/ReplOptionAttribute.cs @@ -26,13 +26,43 @@ public sealed class ReplOptionAttribute : Attribute /// public ReplParameterMode Mode { get; set; } = ReplParameterMode.OptionAndPositional; + // Nullable enums are not legal attribute named arguments (CS0655), so the optional + // overrides expose a non-nullable property and track the unset state in a nullable + // backing field surfaced through the read-only *Override properties. + private ReplCaseSensitivity? _caseSensitivity; + private ReplArity? _arity; + /// /// Optional case-sensitivity override for this option. + /// Only an explicit assignment overrides the global parsing default; when unset, the getter + /// returns the enum default and does not reflect the effective behavior — read + /// to distinguish unset from an explicit value. + /// + public ReplCaseSensitivity CaseSensitivity + { + get => _caseSensitivity ?? default; + set => _caseSensitivity = value; + } + + /// + /// Explicit case-sensitivity override, or null to inherit the global default. /// - public ReplCaseSensitivity? CaseSensitivity { get; set; } + public ReplCaseSensitivity? CaseSensitivityOverride => _caseSensitivity; /// /// Optional arity override. + /// Only an explicit assignment overrides the arity inferred from the parameter shape; when + /// unset, the getter returns the enum default and does not reflect the effective arity — read + /// to distinguish unset from an explicit value. + /// + public ReplArity Arity + { + get => _arity ?? default; + set => _arity = value; + } + + /// + /// Explicit arity override, or null to use the arity inferred from the parameter shape. /// - public ReplArity? Arity { get; set; } + public ReplArity? ArityOverride => _arity; } diff --git a/src/Repl.Core/Parameters/Attributes/ReplValueAliasAttribute.cs b/src/Repl.Core/Parameters/Attributes/ReplValueAliasAttribute.cs index 0737bb72..fe41ea02 100644 --- a/src/Repl.Core/Parameters/Attributes/ReplValueAliasAttribute.cs +++ b/src/Repl.Core/Parameters/Attributes/ReplValueAliasAttribute.cs @@ -29,8 +29,25 @@ public ReplValueAliasAttribute(string token, string value) /// public string Value { get; } + // Nullable enums are not legal attribute named arguments (CS0655), so the optional + // override exposes a non-nullable property and tracks the unset state in a nullable + // backing field surfaced through the read-only CaseSensitivityOverride property. + private ReplCaseSensitivity? _caseSensitivity; + /// /// Optional case-sensitivity override for this alias. + /// Only an explicit assignment overrides the global parsing default; when unset, the getter + /// returns the enum default and does not reflect the effective behavior — read + /// to distinguish unset from an explicit value. + /// + public ReplCaseSensitivity CaseSensitivity + { + get => _caseSensitivity ?? default; + set => _caseSensitivity = value; + } + + /// + /// Explicit case-sensitivity override, or null to inherit the global default. /// - public ReplCaseSensitivity? CaseSensitivity { get; set; } + public ReplCaseSensitivity? CaseSensitivityOverride => _caseSensitivity; } diff --git a/src/Repl.Core/Parsing/HandlerArgumentBinder.cs b/src/Repl.Core/Parsing/HandlerArgumentBinder.cs index fbff1705..91d6e7c5 100644 --- a/src/Repl.Core/Parsing/HandlerArgumentBinder.cs +++ b/src/Repl.Core/Parsing/HandlerArgumentBinder.cs @@ -106,18 +106,30 @@ internal static class HandlerArgumentBinder return resolved; } - if (bindingMode != ReplParameterMode.OptionOnly - && TryConsumePositional( - parameter, - context.PositionalArguments, - context.NumericFormatProvider, - enumIgnoreCase, - ref positionalIndex, - out var positionalValue)) + if (bindingMode != ReplParameterMode.OptionOnly) { - return positionalValue; + var positionalStart = positionalIndex; + if (TryConsumePositional( + parameter, + context.PositionalArguments, + context.NumericFormatProvider, + enumIgnoreCase, + ref positionalIndex, + out var positionalValue)) + { + // Positional parity for explicit upper bounds: a collection consumes every + // remaining positional, so the named-path arity diagnostics must apply here too. + ThrowIfExplicitUpperBoundExceeded( + context.OptionSchema, parameterName, positionalIndex - positionalStart); + return positionalValue; + } } + // Explicit lower bounds can only be enforced here, after BOTH the named and the + // positional binding attempts came up empty — option parsing alone cannot see values + // the parameter would have consumed positionally. + ThrowIfExplicitLowerBoundUnsatisfied(context.OptionSchema, parameterName); + if (parameter.HasDefaultValue) { return parameter.DefaultValue; @@ -133,6 +145,45 @@ internal static class HandlerArgumentBinder $"Unable to bind parameter '{parameter.Name}' ({parameter.ParameterType.Name})."); } + // Only EXPLICIT arity overrides reject absence: inferred ExactlyOne (plain required + // scalars) keeps its historical binding behavior, and inferred arities never produce + // OneOrMore. ArgumentOnly parameters have no named entry, so the message falls back to + // the parameter name instead of a token the parser would not accept. + private static void ThrowIfExplicitLowerBoundUnsatisfied(OptionSchema schema, string parameterName) + { + if (!schema.TryGetParameter(parameterName, out var schemaParameter) + || schemaParameter.ExplicitArity is not (ReplArity.OneOrMore or ReplArity.ExactlyOne)) + { + return; + } + + var token = schema.ResolveDisplayToken(parameterName); + var subject = token is null ? $"Parameter '{parameterName}'" : $"Option '{token}'"; + var requirement = schemaParameter.ExplicitArity == ReplArity.OneOrMore + ? "at least one value" + : "exactly one value"; + throw new InvalidOperationException($"{subject} requires {requirement}."); + } + + // Positional twin of the parse-time ValidateTooManyValues: only explicit upper bounds are + // enforced (inferred collection arity is ZeroOrMore), and only collections can consume + // more than one positional. + private static void ThrowIfExplicitUpperBoundExceeded(OptionSchema schema, string parameterName, int consumedCount) + { + if (consumedCount <= 1 + || !schema.TryGetParameter(parameterName, out var schemaParameter) + || schemaParameter.ExplicitArity is not (ReplArity.ZeroOrOne or ReplArity.ExactlyOne)) + { + return; + } + + var token = schema.ResolveDisplayToken(parameterName); + var subject = token is null ? $"Parameter '{parameterName}'" : $"Option '{token}'"; + throw new InvalidOperationException(schemaParameter.ExplicitArity == ReplArity.ZeroOrOne + ? $"{subject} accepts at most one value." + : $"{subject} requires exactly one value."); + } + private static bool HasExplicitBindingDirection(System.Reflection.ParameterInfo parameter) => parameter.GetCustomAttributes(typeof(FromContextAttribute), inherit: true).Length > 0 || parameter.GetCustomAttributes(typeof(FromServicesAttribute), inherit: true).Length > 0; @@ -584,17 +635,29 @@ private static object BindOptionsGroup( continue; } - if (bindingMode != ReplParameterMode.OptionOnly - && TryConsumePositionalForProperty( + if (bindingMode != ReplParameterMode.OptionOnly) + { + var positionalStart = positionalIndex; + if (TryConsumePositionalForProperty( property, context.PositionalArguments, context.NumericFormatProvider, enumIgnoreCase, ref positionalIndex, out var positionalValue)) - { - property.SetValue(instance, positionalValue); + { + // Same positional upper-bound parity as the handler-parameter path. + ThrowIfExplicitUpperBoundExceeded( + context.OptionSchema, propertyName, positionalIndex - positionalStart); + property.SetValue(instance, positionalValue); + continue; + } } + + // Mirrors the handler-parameter path: an explicit lower bound rejects absence + // after both named and positional attempts, instead of silently keeping the + // property default. + ThrowIfExplicitLowerBoundUnsatisfied(context.OptionSchema, propertyName); } return instance; diff --git a/src/Repl.Core/Parsing/InvocationOptionParser.cs b/src/Repl.Core/Parsing/InvocationOptionParser.cs index 151e1590..e8eed28a 100644 --- a/src/Repl.Core/Parsing/InvocationOptionParser.cs +++ b/src/Repl.Core/Parsing/InvocationOptionParser.cs @@ -202,7 +202,7 @@ public static OptionParsingResult Parse( diagnostics); } - ValidateArityAndConflicts(schema, namedOptions, diagnostics); + ValidateArityAndConflicts(schema, namedOptions, options.OptionCaseSensitivity, diagnostics); var readonlyNamedOptions = namedOptions.ToDictionary( pair => pair.Key, pair => (IReadOnlyList)pair.Value, @@ -300,6 +300,26 @@ private static void HandleUnknownOption( value = effectiveTokens[index]; } + // A token naming a KNOWN parameter must stay an inert unknown when its schema entries + // rejected it for case-sensitivity reasons: storing it would let the namedOptions + // comparer (global) rebind it to that parameter, bypassing the declared casing. + // Two shapes reach here: an EXPLICIT CaseSensitive override always wins (even when the + // rejected token ordinally equals the CLR parameter name — the renamed-token edge), + // and an effectively sensitive parameter drops casing VARIANTS (they were inert under + // a global-case-sensitive dictionary anyway). Unconstrained parameters keep the + // documented permissive bind-by-name behavior. The value token stays consumed. + if (schema.TryGetParameter(optionName, out var knownParameter)) + { + var explicitlySensitive = knownParameter.CaseSensitivity == ReplCaseSensitivity.CaseSensitive; + var effectivelySensitive = + (knownParameter.CaseSensitivity ?? options.OptionCaseSensitivity) == ReplCaseSensitivity.CaseSensitive; + var matchesClrName = string.Equals(knownParameter.Name, optionName, StringComparison.Ordinal); + if (explicitlySensitive || (effectivelySensitive && !matchesClrName)) + { + return; + } + } + AddNamedValue(namedOptions, optionName, value ?? "true"); } @@ -440,6 +460,7 @@ private static void AddNamedValue( private static void ValidateArityAndConflicts( OptionSchema schema, Dictionary> namedOptions, + ReplCaseSensitivity globalCaseSensitivity, List diagnostics) { foreach (var parameter in schema.Parameters.Values) @@ -451,7 +472,7 @@ private static void ValidateArityAndConflicts( ValidateTooManyValues(schema, parameter, values, diagnostics); ValidateBooleanConflicts(parameter, values, diagnostics); - ValidateEnumConflicts(parameter, values, diagnostics); + ValidateEnumConflicts(parameter, values, globalCaseSensitivity, diagnostics); } } @@ -504,6 +525,7 @@ private static void ValidateBooleanConflicts( private static void ValidateEnumConflicts( OptionSchemaParameter parameter, List values, + ReplCaseSensitivity globalCaseSensitivity, List diagnostics) { var effectiveType = Nullable.GetUnderlyingType(parameter.ParameterType) ?? parameter.ParameterType; @@ -512,7 +534,9 @@ private static void ValidateEnumConflicts( return; } - var comparer = parameter.CaseSensitivity == ReplCaseSensitivity.CaseSensitive + // Unset per-option override falls back to the global default, matching every other + // effective-case resolution site (OptionSchema.ResolveToken, HandlerArgumentBinder, ...). + var comparer = (parameter.CaseSensitivity ?? globalCaseSensitivity) == ReplCaseSensitivity.CaseSensitive ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase; if (!values.Distinct(comparer).Skip(1).Any()) @@ -525,13 +549,8 @@ private static void ValidateEnumConflicts( $"Option '--{parameter.Name}' received multiple enum values in a single invocation.")); } - private static ReplArity ResolveParameterArity(OptionSchema schema, string parameterName) - { - var entry = schema.Entries.FirstOrDefault(candidate => - string.Equals(candidate.ParameterName, parameterName, StringComparison.OrdinalIgnoreCase) - && candidate.TokenKind is OptionSchemaTokenKind.NamedOption or OptionSchemaTokenKind.BoolFlag); - return entry?.Arity ?? ReplArity.ZeroOrMore; - } + private static ReplArity ResolveParameterArity(OptionSchema schema, string parameterName) => + schema.ResolveParameterArity(parameterName); private static List ExpandResponseFiles( IReadOnlyList tokens, diff --git a/src/Repl.Defaults/GlobalOptionsExtensions.cs b/src/Repl.Defaults/GlobalOptionsExtensions.cs index d19d8c6e..34765b7d 100644 --- a/src/Repl.Defaults/GlobalOptionsExtensions.cs +++ b/src/Repl.Defaults/GlobalOptionsExtensions.cs @@ -26,6 +26,9 @@ public static class GlobalOptionsExtensions var parsing = app.Core.OptionsSnapshot.Parsing; var properties = GetOptionProperties(); + + ThrowIfUnsupportedOverrides(typeof(T), properties); + app.Options(options => { var prototype = new T(); @@ -55,6 +58,39 @@ public static class GlobalOptionsExtensions return app; } + // Typed global options do not flow per-option CaseSensitivity/Arity overrides into + // AddGlobalOptionCore (the global-option pipeline has no per-option override concept + // yet). Fail fast instead of silently discarding a now-settable override. Enum-flag + // overrides are declared on the enum TYPE's fields — which may be legitimately shared + // with route commands where they do work — so they are deliberately not rejected here. + private static void ThrowIfUnsupportedOverrides(Type optionsType, IReadOnlyList properties) + { + foreach (var property in properties) + { + var optionAttr = property.GetCustomAttribute(); + if (optionAttr?.CaseSensitivityOverride is not null) + { + throw new NotSupportedException( + $"Global option property '{optionsType.Name}.{property.Name}' declares a CaseSensitivity override, which is not supported for typed global options. Remove the override or expose the option through a per-command options type."); + } + + if (optionAttr?.ArityOverride is not null) + { + throw new NotSupportedException( + $"Global option property '{optionsType.Name}.{property.Name}' declares an Arity override, which is not supported for typed global options. Remove the override or expose the option through a per-command options type."); + } + + foreach (var valueAlias in property.GetCustomAttributes()) + { + if (valueAlias.CaseSensitivityOverride is not null) + { + throw new NotSupportedException( + $"Global option property '{optionsType.Name}.{property.Name}' declares a CaseSensitivity override on value alias '{valueAlias.Token}', which is not supported for typed global options. Remove the override or expose the option through a per-command options type."); + } + } + } + } + internal static T PopulateInstance<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T>(IGlobalOptionsAccessor accessor, IFormatProvider numericFormatProvider) where T : class, new() { diff --git a/src/Repl.IntegrationTests/Given_OptionAttributeOverrides.cs b/src/Repl.IntegrationTests/Given_OptionAttributeOverrides.cs new file mode 100644 index 00000000..24187f78 --- /dev/null +++ b/src/Repl.IntegrationTests/Given_OptionAttributeOverrides.cs @@ -0,0 +1,531 @@ +namespace Repl.IntegrationTests; + +[TestClass] +[DoNotParallelize] +public sealed class Given_OptionAttributeOverrides +{ + private enum ShadockSyllable + { + None = 0, + + [ReplEnumFlag("--ga", CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] + Ga = 1, + + [ReplEnumFlag("--bu")] + Bu = 2, + } + + [ReplOptionsGroup] + public sealed class DenimOutfitOptions + { + [ReplOption(CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] + public string Fabric { get; set; } = "denim"; + + [ReplOption(Arity = ReplArity.ZeroOrOne)] + public string[] Patches { get; set; } = []; + } + + public sealed class OverrideGlobals + { + [ReplOption(Name = "tenant", CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] + public string Tenant { get; set; } = "ga"; + } + + public sealed class OverrideArityGlobals + { + [ReplOption(Name = "retries", Arity = ReplArity.ExactlyOne)] + public int Retries { get; set; } = 42; + } + + public sealed class ValueAliasOverrideGlobals + { + [ReplValueAlias("--prod", "production", CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] + public string Environment { get; set; } = "dev"; + } + + [ReplOptionsGroup] + public sealed class RequiredPatchesOptions + { + [ReplOption(Arity = ReplArity.OneOrMore)] + public string[] Patches { get; set; } = []; + } + + [ReplOptionsGroup] + public sealed class SinglePatchOptions + { + [ReplOption(Mode = ReplParameterMode.OptionAndPositional, Arity = ReplArity.ZeroOrOne)] + public string[] Patches { get; set; } = []; + } + + [TestMethod] + [Description("Regression guard for issue #57: [ReplOption(CaseSensitivity = ...)] must be a legal attribute argument (a nullable enum triggers CS0655, making the override dead code) and the per-option override must accept casing variants while the global default stays case-sensitive.")] + public void When_OptionCaseSensitivityOverriddenViaAttribute_Then_CasingVariantIsAccepted() + { + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] string text) => text); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "--Text", "bib overalls", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("bib overalls"); + } + + [TestMethod] + [Description("Regression guard for issue #57 (expanded): [ReplOption(Arity = ...)] must be a legal attribute argument. A collection parameter naturally allows repetition (ZeroOrMore); the explicit ZeroOrOne override must be honored so a repeated option is rejected at parse time.")] + public void When_ArityOverriddenViaAttributeToZeroOrOne_Then_RepeatedOptionIsRejected() + { + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(Arity = ReplArity.ZeroOrOne)] string[] items) => string.Join(',', items)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "--items", "ga", "--items", "bu", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("accepts at most one value"); + } + + [TestMethod] + [Description("Regression guard for issue #57 (expanded): [ReplValueAlias(..., CaseSensitivity = ...)] must be a legal attribute argument so an alias token matched ignoring case still injects its configured value.")] + public void When_ValueAliasCaseSensitivityOverriddenViaAttribute_Then_CasingVariantInjectsValue() + { + var sut = ReplApp.Create(); + sut.Map("wear", ([ReplValueAlias("--denim", "bib overalls", CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] string outfit = "none") => outfit); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["wear", "--DENIM", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("bib overalls"); + } + + [TestMethod] + [Description("Regression guard for issue #57 (expanded): [ReplEnumFlag(..., CaseSensitivity = ...)] must be a legal attribute argument so an enum-flag alias matched ignoring case binds the enum member while other members stay case-sensitive.")] + public void When_EnumFlagCaseSensitivityOverriddenViaAttribute_Then_CasingVariantBindsEnumMember() + { + var sut = ReplApp.Create(); + sut.Map("say", (ShadockSyllable syllable = ShadockSyllable.None) => syllable.ToString()); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["say", "--GA", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("Ga"); + } + + [TestMethod] + [Description("Guards the ambiguity edge unlocked by issue #57: a typed token matching both a case-insensitive alias and another option's case-sensitive canonical token must be rejected as ambiguous at parse time, never silently bound to either parameter.")] + public void When_TokenMatchesCaseInsensitiveAliasAndAnotherOption_Then_AmbiguityIsRejected() + { + var sut = ReplApp.Create(); + sut.Map( + "probe", + ([ReplOption(Aliases = ["--Mode"], CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] string first = "ga", + [ReplOption(Name = "mode")] string second = "bu") => $"first={first};second={second}"); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["probe", "--mode", "zo", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("Ambiguous option '--mode'"); + } + + [TestMethod] + [Description("Guards the override boundary: a sibling enum-flag alias without a case-sensitivity override must keep the global case-sensitive default, so per-member overrides stay scoped to their own aliases.")] + public void When_EnumFlagWithoutOverrideAndCasingDiffers_Then_TokenIsRejected() + { + var sut = ReplApp.Create(); + sut.Map("say", (ShadockSyllable syllable = ShadockSyllable.None) => syllable.ToString()); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["say", "--BU", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("Unknown option '--BU'"); + } + + [TestMethod] + [Description("Guards the unset state of the Arity override: an attributed collection parameter without an explicit Arity must keep the inferred ZeroOrMore, so a builder-site drift back to the public (non-nullable) property — which compiles cleanly under ?. — would wrongly force ZeroOrOne and break repetition.")] + public void When_AttributedCollectionParameterWithoutArityOverride_Then_RepetitionIsAccepted() + { + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(Name = "item")] string[] items) => string.Join(',', items)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "--item", "ga", "--item", "bu", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("ga,bu"); + } + + [TestMethod] + [Description("Guards the unset state of the CaseSensitivity override at execution: an attributed option without an explicit override must inherit the global CaseInsensitive default instead of the enum default (CaseSensitive, value 0).")] + public void When_GlobalCaseInsensitiveAndNoOverride_Then_CasingVariantIsAccepted() + { + var sut = ReplApp.Create() + .Options(options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + sut.Map("echo", ([ReplOption(Name = "channel")] string channel) => channel); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "--Channel", "zo", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("zo"); + } + + [TestMethod] + [Description("Guards the explicit-assignment-of-enum-value-zero edge: [ReplOption(CaseSensitivity = CaseSensitive)] (enum value 0) under a global CaseInsensitive default must register as a real override and reject casing variants — a sentinel-style refactor treating value 0 as unset would silently pass them.")] + public void When_ExplicitCaseSensitiveOverrideUnderGlobalInsensitive_Then_CasingVariantIsRejected() + { + var sut = ReplApp.Create() + .Options(options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + sut.Map("echo", ([ReplOption(CaseSensitivity = ReplCaseSensitivity.CaseSensitive)] string channel) => channel); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "--Channel", "zo", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("Unknown option '--Channel'"); + } + + [TestMethod] + [Description("Guards the options-group property path (a parallel branch in OptionSchemaBuilder): a case-sensitivity override on a group property must be honored the same way as on a handler parameter.")] + public void When_GroupPropertyCaseSensitivityOverridden_Then_CasingVariantIsAccepted() + { + var sut = ReplApp.Create(); + sut.Map("wear", (DenimOutfitOptions options) => options.Fabric); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["wear", "--FABRIC", "bib overalls", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("bib overalls"); + } + + [TestMethod] + [Description("Guards the options-group property path for the arity override: a collection group property naturally infers ZeroOrMore; the explicit ZeroOrOne override must be honored so repetition is rejected.")] + public void When_GroupPropertyArityOverriddenToZeroOrOne_Then_RepeatedOptionIsRejected() + { + var sut = ReplApp.Create(); + sut.Map("wear", (DenimOutfitOptions options) => string.Join(',', options.Patches)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["wear", "--patches", "ga", "--patches", "bu", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("accepts at most one value"); + } + + [TestMethod] + [Description("Guards execution parity for enum duplicate detection: with an explicit ZeroOrMore arity (reachable only through the now-settable override), repeated enum values differing only by casing are distinct under the global CaseSensitive default and must be reported as conflicting — the validator previously hardcoded case-insensitive comparison when no per-option override was set.")] + public void When_RepeatedEnumValuesDifferByCaseUnderGlobalCaseSensitive_Then_ConflictIsReported() + { + var sut = ReplApp.Create(); + sut.Map("say", ([ReplOption(Arity = ReplArity.ZeroOrMore)] ShadockSyllable syllable = ShadockSyllable.None) => syllable.ToString()); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["say", "--syllable", "Ga", "--syllable", "GA", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("received multiple enum values"); + } + + [TestMethod] + [Description("Guards against a silent no-op newly reachable through issue #57: typed global options (UseGlobalOptions) do not support per-option CaseSensitivity/Arity overrides, so declaring one must fail fast at registration instead of being silently discarded.")] + public void When_GlobalOptionsPropertyDeclaresOverride_Then_RegistrationFailsFast() + { + var act = () => ReplApp.Create().UseGlobalOptions(); + + act.Should().Throw().WithMessage("*CaseSensitivity*"); + } + + [TestMethod] + [Description("Guards collision validation against false positives newly reachable through issue #57: under a global CaseInsensitive default, two options explicitly marked CaseSensitive whose tokens differ only by casing are distinguishable ordinally at resolution time, so registration must accept them and route each token to its own parameter.")] + public void When_TwoExplicitlyCaseSensitiveOptionsDifferOnlyByCase_Then_RegistrationAndResolutionSucceed() + { + var sut = ReplApp.Create() + .Options(options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + sut.Map( + "cfg", + ([ReplOption(Name = "mode", CaseSensitivity = ReplCaseSensitivity.CaseSensitive)] string lower = "ga", + [ReplOption(Name = "MODE", CaseSensitivity = ReplCaseSensitivity.CaseSensitive)] string upper = "bu") => $"lower={lower};upper={upper}"); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["cfg", "--MODE", "zo", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("lower=ga;upper=zo"); + } + + [TestMethod] + [Description("Guards the OneOrMore lower bound newly reachable through issue #57: an option with an explicit OneOrMore arity invoked without any value (named or positional) must fail with an arity diagnostic instead of invoking the handler with a missing value.")] + public void When_OneOrMoreArityOptionIsAbsent_Then_BindingFailsWithoutInvokingHandler() + { + var invoked = false; + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(Arity = ReplArity.OneOrMore)] string[] items) => + { + invoked = true; + return string.Join(',', items); + }); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("requires at least one value"); + invoked.Should().BeFalse(); + } + + [TestMethod] + [Description("Guards the boundary of the OneOrMore lower bound: values consumed positionally satisfy the arity, so the absence check must run only after both named and positional binding attempts — not at option-parse time, which cannot see positional consumption.")] + public void When_OneOrMoreArityOptionReceivesPositionalValues_Then_BindingSucceeds() + { + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(Arity = ReplArity.OneOrMore)] string[] items) => string.Join(',', items)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "ga", "bu", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("ga,bu"); + } + + [TestMethod] + [Description("Guards the OneOrMore lower bound on the options-group property path: an absent group property with an explicit OneOrMore arity must fail binding instead of silently keeping its default and invoking the handler.")] + public void When_GroupPropertyOneOrMoreArityIsAbsent_Then_BindingFailsWithoutInvokingHandler() + { + var invoked = false; + var sut = ReplApp.Create(); + sut.Map("wear", (RequiredPatchesOptions options) => + { + invoked = true; + return string.Join(',', options.Patches); + }); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["wear", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("requires at least one value"); + invoked.Should().BeFalse(); + } + + [TestMethod] + [Description("Guards the OneOrMore lower bound for ArgumentOnly parameters: they have no named-option schema entry, so the explicit arity must be carried at the parameter level — otherwise the declared override is silently dropped and the handler receives a missing value.")] + public void When_ArgumentOnlyOneOrMoreArityIsAbsent_Then_BindingFails() + { + var sut = ReplApp.Create(); + sut.Map("copy", ([ReplOption(Mode = ReplParameterMode.ArgumentOnly, Arity = ReplArity.OneOrMore)] string[] files) => string.Join(',', files)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["copy", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("requires at least one value"); + } + + [TestMethod] + [Description("Boundary for the ArgumentOnly OneOrMore bound: positional values satisfy the explicit arity, so the same registration invoked with positionals must bind and execute.")] + public void When_ArgumentOnlyOneOrMoreArityReceivesPositionals_Then_BindingSucceeds() + { + var sut = ReplApp.Create(); + sut.Map("copy", ([ReplOption(Mode = ReplParameterMode.ArgumentOnly, Arity = ReplArity.OneOrMore)] string[] files) => string.Join(',', files)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["copy", "ga", "bu", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("ga,bu"); + } + + [TestMethod] + [Description("Guards the explicit ExactlyOne lower bound: ReplArity.ExactlyOne is documented as 'must appear exactly one time', so an EXPLICIT override on an otherwise-optional parameter must reject absence — while inferred ExactlyOne (plain required scalars) keeps its existing binding behavior.")] + public void When_ExplicitExactlyOneArityOptionIsAbsent_Then_BindingFails() + { + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(Arity = ReplArity.ExactlyOne)] string? channel = null) => channel ?? "none"); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("requires exactly one value"); + } + + [TestMethod] + [Description("Guards diagnostic accuracy: the lower-bound failure message must name the canonical option token ([ReplOption(Name = ...)]), not the CLR parameter name — telling the user to supply a token the parser would reject is actively misleading.")] + public void When_RenamedOneOrMoreOptionIsAbsent_Then_MessageUsesCanonicalToken() + { + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(Name = "item", Arity = ReplArity.OneOrMore)] string[] items) => string.Join(',', items)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("Option '--item' requires"); + } + + [TestMethod] + [Description("Guards the Arity branch of the typed-global-options fail-fast: reverting only that branch would silently discard the override while the CaseSensitivity twin keeps the suite green.")] + public void When_GlobalOptionsPropertyDeclaresArityOverride_Then_RegistrationFailsFast() + { + var act = () => ReplApp.Create().UseGlobalOptions(); + + act.Should().Throw().WithMessage("*Arity*"); + } + + [TestMethod] + [Description("Guards the value-alias branch of the typed-global-options fail-fast: a [ReplValueAlias(..., CaseSensitivity = ...)] override on a global property is equally newly settable and must be rejected instead of silently discarded.")] + public void When_GlobalOptionsValueAliasDeclaresOverride_Then_RegistrationFailsFast() + { + var act = () => ReplApp.Create().UseGlobalOptions(); + + act.Should().Throw().WithMessage("*CaseSensitivity*"); + } + + [TestMethod] + [Description("Guards suggestion fidelity for case-distinct tokens: KnownTokens must not dedupe tokens ignoring case, otherwise one of two case-sensitive twins vanishes and 'Did you mean' proposes the wrong casing.")] + public void When_CaseSensitiveTwinTokensRegistered_Then_SuggestionPreservesEachCasing() + { + var sut = ReplApp.Create(); + sut.Map( + "cfg", + ([ReplOption(Name = "mode")] string lower = "ga", + [ReplOption(Name = "MODE")] string upper = "bu") => $"lower={lower};upper={upper}"); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["cfg", "--MODEs", "zo", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("Did you mean '--MODE'"); + } + + [TestMethod] + [Description("Guards diagnostic context: a token collision thrown at Map() time must name the command being registered, so an app with many registrations does not need a debugger to find the offending one.")] + public void When_TokenCollisionDetected_Then_MessageNamesTheCommand() + { + var sut = ReplApp.Create(); + var act = () => sut.Map( + "cfg", + ([ReplOption(Name = "mode")] string first = "ga", + [ReplOption(Aliases = ["--mode"])] string second = "bu") => $"{first}{second}"); + + act.Should().Throw().WithMessage("*cfg*"); + } + + [TestMethod] + [Description("Guards diagnostic wording: a group property whose name collides with a handler parameter is a duplicate-name error, not a token collision — the old wording sent developers hunting for alias conflicts that do not exist.")] + public void When_GroupPropertyNameCollidesWithParameter_Then_MessageSaysDuplicateName() + { + var sut = ReplApp.Create(); + var act = () => sut.Map( + "wear", + (DenimOutfitOptions options, [ReplOption] string fabric = "denim") => fabric); + + act.Should().Throw().WithMessage("*Duplicate*"); + } + + [TestMethod] + [Description("Guards named/positional parity for explicit upper bounds: a ZeroOrOne collection rejects a repeated named option, so two positional values feeding the same collection must be rejected too — otherwise the override is enforceable on one input path and bypassable on the other.")] + public void When_ZeroOrOneCollectionReceivesTwoPositionals_Then_BindingFails() + { + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(Arity = ReplArity.ZeroOrOne)] string[] items) => string.Join(',', items)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "ga", "bu", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("accepts at most one value"); + } + + [TestMethod] + [Description("Guards named/positional parity for the ExactlyOne upper bound on the positional path: two positional values must be rejected with the exactly-one diagnostic, mirroring the named-option validator.")] + public void When_ExactlyOneCollectionReceivesTwoPositionals_Then_BindingFails() + { + var sut = ReplApp.Create(); + sut.Map("echo", ([ReplOption(Arity = ReplArity.ExactlyOne)] string[] items) => string.Join(',', items)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["echo", "ga", "bu", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("requires exactly one value"); + } + + [TestMethod] + [Description("Guards named/positional parity for explicit upper bounds on the options-group property path: a positional ZeroOrOne collection property must reject two positional values the same way repeated named values are rejected.")] + public void When_GroupZeroOrOneCollectionReceivesTwoPositionals_Then_BindingFails() + { + var sut = ReplApp.Create(); + sut.Map("wear", (SinglePatchOptions options) => string.Join(',', options.Patches)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["wear", "ga", "bu", "--no-logo"])); + + output.ExitCode.Should().Be(1); + output.Text.Should().Contain("accepts at most one value"); + } + + [TestMethod] + [Description("Guards the CaseSensitive override against the permissive unknown-option fallback: with AllowUnknownOptions and a global CaseInsensitive default, a casing-mismatched token must stay an inert unknown instead of rebinding by parameter name to an option that explicitly rejected that casing.")] + public void When_PermissiveUnknownTokenCaseMismatchesCaseSensitiveOption_Then_ValueIsNotBound() + { + var sut = ReplApp.Create() + .Options(options => + { + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive; + options.Parsing.AllowUnknownOptions = true; + }); + sut.Map("run", ([ReplOption(CaseSensitivity = ReplCaseSensitivity.CaseSensitive)] string mode = "ga") => mode); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["run", "--Mode", "zo", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("ga"); + output.Text.Should().NotContain("zo"); + } + + [TestMethod] + [Description("Boundary for the permissive-fallback guard: without a per-option override, permissive mode keeps binding unknown casing variants by parameter name under the global CaseInsensitive comparer — the documented migration behavior must survive the fix.")] + public void When_PermissiveUnknownTokenCaseMismatchesUnconstrainedOption_Then_ValueStillBinds() + { + var sut = ReplApp.Create() + .Options(options => + { + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive; + options.Parsing.AllowUnknownOptions = true; + }); + sut.Map("run", ([ReplOption] string mode = "ga") => mode); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["run", "--MoDe", "zo", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("zo"); + } + + [TestMethod] + [Description("Guards the permissive-fallback guard against the renamed-token edge: when [ReplOption(Name = ...)] differs from the CLR parameter name only by casing, a token rejected by the case-sensitive schema entry must not slip through the name-equality path of the fallback and bind anyway.")] + public void When_PermissiveTokenMatchesRenamedCaseSensitiveOption_Then_ValueIsNotBound() + { + var sut = ReplApp.Create() + .Options(options => + { + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive; + options.Parsing.AllowUnknownOptions = true; + }); + sut.Map("run", ([ReplOption(Name = "Mode", CaseSensitivity = ReplCaseSensitivity.CaseSensitive)] string mode = "ga") => mode); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["run", "--mode", "zo", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("ga"); + output.Text.Should().NotContain("zo"); + } + + [TestMethod] + [Description("Guards documentation fidelity for explicit lower bounds: an option that now fails when omitted (explicit OneOrMore) must be exported as required — otherwise generated docs tell users a runtime-required option is optional.")] + public void When_ExportingDocForExplicitOneOrMoreOption_Then_OptionIsRequired() + { + var sut = ReplApp.Create() + .UseDocumentationExport(); + sut.Map("echo", ([ReplOption(Arity = ReplArity.OneOrMore)] string[] items) => string.Join(',', items)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "--json", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("\"required\": true"); + } + + [TestMethod] + [Description("Guards documentation fidelity on the options-group path: group options were hard-coded as not required, so an explicit OneOrMore group property — which now fails binding when omitted — must be exported as required.")] + public void When_ExportingDocForExplicitOneOrMoreGroupProperty_Then_OptionIsRequired() + { + var sut = ReplApp.Create() + .UseDocumentationExport(); + sut.Map("wear", (RequiredPatchesOptions options) => string.Join(',', options.Patches)); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "--json", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("\"required\": true"); + } +} diff --git a/src/Repl.IntegrationTests/Given_OptionsGroupBinding.cs b/src/Repl.IntegrationTests/Given_OptionsGroupBinding.cs index a564ae47..d0f1ff38 100644 --- a/src/Repl.IntegrationTests/Given_OptionsGroupBinding.cs +++ b/src/Repl.IntegrationTests/Given_OptionsGroupBinding.cs @@ -155,7 +155,7 @@ public void When_NullableGroupPropertyInitializedToUnderlyingClrDefault_Then_Hel } [TestMethod] - [Description("Regression guard: verifies token collision between group and regular parameter fails at registration.")] + [Description("Regression guard: verifies a duplicate name between a group property and a regular parameter fails at registration with a duplicate-name diagnostic (not a token-collision one).")] public void When_GroupPropertyCollidesWithParam_Then_MapFails() { var sut = ReplApp.Create(); @@ -163,7 +163,7 @@ public void When_GroupPropertyCollidesWithParam_Then_MapFails() var act = () => sut.Map("list", (TestOutputOptions output, string format) => format); act.Should().Throw() - .WithMessage("*collision*"); + .WithMessage("*Duplicate parameter name*"); } [TestMethod]