diff --git a/docs/docs/how-to/custom-commands.md b/docs/docs/how-to/custom-commands.md index 7af54e27b3d..e9ea5e47481 100644 --- a/docs/docs/how-to/custom-commands.md +++ b/docs/docs/how-to/custom-commands.md @@ -22,6 +22,35 @@ This is the equivalent to running: `dotnet tool install --global dotnet-coverage` +By default, `Arguments` appears after generated non-terminal options and operands. It +appears before `RunSettings` (and its `--` marker) and before options in the `Terminal` +phase. When `ArgumentsContainToolOptions` is enabled, recognized tool options can be +hoisted ahead of a structured or declared end-of-options marker. + +## Adding Unmodeled Options + +Use `AdditionalArguments` when a strongly typed or generated options record does not yet +model a tool option. Each entry accepts a `CommandLinePhase`; entries with +`IsGlobalOption: true` appear before the command or subcommand parts. + +```csharp +var options = new SomeGeneratedOptions +{ + AdditionalArguments = + [ + new("--global-flag", IsGlobalOption: true), + new("--new-option", CommandLinePhase.Normal), + new("value", CommandLinePhase.Normal), + ], +}; +``` + +Within each non-terminal phase, additional tokens retain their declared order and appear +before generated tokens. The supported phases render as `EarlyOperand`, `Normal`, +`Passthrough`, then `Terminal`. Use `RunSettings` or a declared marker in `Arguments` for +end-of-options pass-through values. Terminal tokens appear after `Arguments` and cannot +be combined with an end-of-options marker or `RunSettings`. + ## Strongly Typed Options Static command identities use one source for each part: diff --git a/src/ModularPipelines/Attributes/CommandLinePhase.cs b/src/ModularPipelines/Attributes/CommandLinePhase.cs index 0974d0e885c..02b7b17f38d 100644 --- a/src/ModularPipelines/Attributes/CommandLinePhase.cs +++ b/src/ModularPipelines/Attributes/CommandLinePhase.cs @@ -33,3 +33,8 @@ public enum CommandLinePhase /// Passthrough = 3, } + +internal static class CommandLinePhaseCompatibility +{ + internal const CommandLinePhase LegacyEndOfOptions = (CommandLinePhase) 2; +} diff --git a/src/ModularPipelines/Context/CommandLineBuilder.cs b/src/ModularPipelines/Context/CommandLineBuilder.cs index abf573ba8df..013e7335212 100644 --- a/src/ModularPipelines/Context/CommandLineBuilder.cs +++ b/src/ModularPipelines/Context/CommandLineBuilder.cs @@ -14,7 +14,7 @@ namespace ModularPipelines.Context; /// 1. Resolve tool name from [CliTool] attribute or constructor parameter /// 2. Get subcommand parts from [CliSubCommand] or a preferred [CliCommandAlias] /// 3. Build arguments from [CliOption], [CliFlag], and [CliArgument] attributes -/// 4. Combine global arguments, command parts, and command-specific arguments. +/// 4. Insert phase-aware AdditionalArguments and combine command parts. /// 5. Add manual Arguments if present. /// 6. Render RunSettings as option-terminated pass-through arguments. /// 7. Validate option terminators against terminal options in one place. @@ -57,6 +57,9 @@ public CommandLine Build(CommandLineToolOptions options) // on a [CliGlobalOptions] base belong before the subcommand; command-specific // properties retain their normal position after it. var commandModel = _commandModelProvider.GetCommandModel(options.GetType()); + var additionalArguments = options.AdditionalArguments?.ToList() ?? []; + ValidateAdditionalArguments(additionalArguments); + var terminalCommandModel = commandModel .Where(part => part.Phase == CommandLinePhase.Terminal) .ToList(); @@ -66,17 +69,21 @@ public CommandLine Build(CommandLineToolOptions options) var globalCommandModel = nonTerminalCommandModel.Where(part => part.IsGlobalOption).ToList(); var commandSpecificModel = nonTerminalCommandModel.Where(part => !part.IsGlobalOption).ToList(); var emittedOptionTerminator = false; - var globalArgs = _commandArgumentBuilder.BuildArguments( + var globalArgs = BuildNonTerminalArguments( globalCommandModel, + additionalArguments, options, + isGlobalOption: true, ref emittedOptionTerminator, - out var globalOptionTerminatorIndex).ToList(); + out var globalOptionTerminatorIndex); var terminatorEmittedBeforeProperties = emittedOptionTerminator; - var propertyArgs = _commandArgumentBuilder.BuildArguments( + var propertyArgs = BuildNonTerminalArguments( commandSpecificModel, + additionalArguments, options, + isGlobalOption: false, ref emittedOptionTerminator, - out var commandOptionTerminatorIndex).ToList(); + out var commandOptionTerminatorIndex); var manualArgs = options.Arguments?.ToList() ?? []; ValidateManualOptionsAfterGlobalTerminator( options, @@ -96,6 +103,10 @@ public CommandLine Build(CommandLineToolOptions options) [.. terminalCommandModel.Where(static part => part is ArgumentPart)], options, ref pendingTerminatorState); + var terminalAdditionalArgs = GetAdditionalArguments( + additionalArguments, + CommandLinePhase.Terminal) + .ToList(); var hasOptionTerminator = pendingTerminatorState; var extractedManualOptions = options.ArgumentsContainToolOptions && hasOptionTerminator @@ -145,7 +156,8 @@ [.. terminalCommandModel.Where(static part => part is FlagPart or OptionPart)], allArgs.AddRange(runSettingsArgs); // 7. A terminal option must not follow any rendered or manually supplied option terminator. - if (terminalOptionArgs.Count > 0 && emittedOptionTerminator) + if ((terminalAdditionalArgs.Count > 0 || terminalOptionArgs.Count > 0) + && emittedOptionTerminator) { throw new InvalidOperationException( "Terminal options cannot be combined with arguments that emit or supply an " @@ -154,11 +166,123 @@ [.. terminalCommandModel.Where(static part => part is FlagPart or OptionPart)], // Terminal options must follow every positional argument source. allArgs.AddRange(terminalArgumentArgs); + allArgs.AddRange(terminalAdditionalArgs); allArgs.AddRange(terminalOptionArgs); return new CommandLine(tool, allArgs); } + private List BuildNonTerminalArguments( + IReadOnlyList commandModel, + IReadOnlyList additionalArguments, + CommandLineToolOptions options, + bool isGlobalOption, + ref bool emittedOptionTerminator, + out int? emittedOptionTerminatorIndex) + { + var result = new List(); + emittedOptionTerminatorIndex = null; + + foreach (var phase in Enum.GetValues() + .Where(static phase => phase != CommandLinePhase.Terminal)) + { + var phaseAdditionalArguments = GetAdditionalArguments( + additionalArguments, + phase, + isGlobalOption) + .ToList(); + if (phase == CommandLinePhaseCompatibility.LegacyEndOfOptions + && phaseAdditionalArguments.Count > 0) + { + if (emittedOptionTerminator) + { + throw new InvalidOperationException( + "An additional end-of-options marker cannot follow one that was already emitted."); + } + + emittedOptionTerminatorIndex = result.Count; + emittedOptionTerminator = true; + } + + result.AddRange(phaseAdditionalArguments); + + var phaseModel = commandModel.Where(part => part.Phase == phase).ToList(); + var phaseArguments = _commandArgumentBuilder.BuildArguments( + phaseModel, + options, + ref emittedOptionTerminator, + out var phaseOptionTerminatorIndex); + if (emittedOptionTerminatorIndex is null + && phaseOptionTerminatorIndex is { } phaseIndex) + { + emittedOptionTerminatorIndex = result.Count + phaseIndex; + } + + result.AddRange(phaseArguments); + } + + return result; + } + + private static IEnumerable GetAdditionalArguments( + IEnumerable additionalArguments, + CommandLinePhase phase, + bool? isGlobalOption = null) + => additionalArguments + .Where(argument => argument.Phase == phase + && (isGlobalOption is null || argument.IsGlobalOption == isGlobalOption)) + .Select(argument => argument.Value); + + private static void ValidateAdditionalArguments( + IReadOnlyCollection additionalArguments) + { + foreach (var argument in additionalArguments) + { + ArgumentNullException.ThrowIfNull(argument); + + if (!Enum.IsDefined(argument.Phase)) + { + throw new ArgumentOutOfRangeException( + nameof(CommandLineToolOptions.AdditionalArguments), + argument.Phase, + "The additional argument phase is not defined."); + } + + ArgumentNullException.ThrowIfNull(argument.Value); + + if (argument is { IsGlobalOption: true, Phase: CommandLinePhase.Terminal }) + { + throw new ArgumentException( + "A terminal additional argument cannot be a global option.", + nameof(CommandLineToolOptions.AdditionalArguments)); + } + + if (argument.Phase == CommandLinePhaseCompatibility.LegacyEndOfOptions + && argument.Value != "--") + { + throw new ArgumentException( + "The legacy end-of-options phase only accepts the '--' marker.", + nameof(CommandLineToolOptions.AdditionalArguments)); + } + + if (argument.Value == "--" + && argument.Phase != CommandLinePhaseCompatibility.LegacyEndOfOptions) + { + throw new ArgumentException( + "The '--' marker must use the legacy end-of-options phase.", + nameof(CommandLineToolOptions.AdditionalArguments)); + } + } + + if (additionalArguments.Count(argument => + argument.Phase == CommandLinePhaseCompatibility.LegacyEndOfOptions) > 1) + { + throw new ArgumentException( + "Additional arguments can contain at most one end-of-options marker.", + nameof(CommandLineToolOptions.AdditionalArguments)); + } + } + private static void ValidateTerminatorState( CommandLineToolOptions options, IReadOnlyCollection commandParts, diff --git a/src/ModularPipelines/Helpers/Internal/CommandArgumentBuilder.cs b/src/ModularPipelines/Helpers/Internal/CommandArgumentBuilder.cs index 021e0e29975..eb3db57397c 100644 --- a/src/ModularPipelines/Helpers/Internal/CommandArgumentBuilder.cs +++ b/src/ModularPipelines/Helpers/Internal/CommandArgumentBuilder.cs @@ -11,8 +11,6 @@ namespace ModularPipelines.Helpers.Internal; /// internal sealed class CommandArgumentBuilder : ICommandArgumentBuilder { - private const CommandLinePhase LegacyEndOfOptionsPhase = (CommandLinePhase) 2; - /// public IReadOnlyList BuildArguments( IReadOnlyList commandModel, @@ -87,7 +85,7 @@ public IReadOnlyList BuildArguments( { CommandLinePhase.EarlyOperand => 0, CommandLinePhase.Normal => 1, - LegacyEndOfOptionsPhase => 2, + CommandLinePhaseCompatibility.LegacyEndOfOptions => 2, CommandLinePhase.Passthrough => 3, CommandLinePhase.Terminal => 4, _ => throw new ArgumentOutOfRangeException(nameof(phase), phase, null), @@ -123,7 +121,7 @@ private static RenderedPhase RenderPhase( else { AddFlagsAndOptions(rendered, phaseOptions, renderedOptionValues); - if (phase == LegacyEndOfOptionsPhase + if (phase == CommandLinePhaseCompatibility.LegacyEndOfOptions && rendered.IndexOf("--") is var terminatorIndex && terminatorIndex >= 0) { @@ -219,11 +217,11 @@ private static void ValidateOptionTerminatorOrdering( } var legacyOptionTerminatorRendered = renderedOptionValues.Any(static pair => - pair.Key.Phase == LegacyEndOfOptionsPhase + pair.Key.Phase == CommandLinePhaseCompatibility.LegacyEndOfOptions && pair.Value.Contains("--", StringComparer.Ordinal)); if (legacyOptionTerminatorRendered && renderedOptions.Any(static option => - GetRenderOrder(option.Phase) > GetRenderOrder(LegacyEndOfOptionsPhase))) + GetRenderOrder(option.Phase) > GetRenderOrder(CommandLinePhaseCompatibility.LegacyEndOfOptions))) { throw new InvalidOperationException( "CLI flags or options cannot be rendered after a legacy end-of-options marker."); diff --git a/src/ModularPipelines/Options/AdditionalCommandLineArgument.cs b/src/ModularPipelines/Options/AdditionalCommandLineArgument.cs new file mode 100644 index 00000000000..7b6d55b6115 --- /dev/null +++ b/src/ModularPipelines/Options/AdditionalCommandLineArgument.cs @@ -0,0 +1,16 @@ +using ModularPipelines.Attributes; + +namespace ModularPipelines.Options; + +/// +/// A manually supplied command-line token with explicit placement metadata. +/// +/// The token to add to the command line. +/// The semantic rendering phase for the token. +/// +/// Whether the token belongs before the command or subcommand parts. +/// +public sealed record AdditionalCommandLineArgument( + string Value, + CommandLinePhase Phase = CommandLinePhase.Normal, + bool IsGlobalOption = false); diff --git a/src/ModularPipelines/Options/CommandLineToolOptions.cs b/src/ModularPipelines/Options/CommandLineToolOptions.cs index 2070814ea9d..e28f33fe7e0 100644 --- a/src/ModularPipelines/Options/CommandLineToolOptions.cs +++ b/src/ModularPipelines/Options/CommandLineToolOptions.cs @@ -20,10 +20,18 @@ public abstract record CommandLineToolOptions public IReadOnlyList? CommandParts { get; init; } /// - /// Gets used for providing switches and arguments to the tool. + /// Gets manual tokens appended after generated non-terminal options and operands, + /// unless recognized tool options are hoisted when + /// is enabled. These tokens precede and terminal options. /// public IEnumerable? Arguments { get; init; } + /// + /// Gets manual tokens whose placement is controlled by their command-line phase. + /// Use this for unmodeled options on strongly typed or generated option records. + /// + public IEnumerable? AdditionalArguments { get; init; } + /// /// Gets whether option-shaped tokens in are options for this tool. /// When enabled, recognized options can be moved before an end-of-options marker emitted by diff --git a/test/ModularPipelines.UnitTests/Context/CommandLineBuilderTests.cs b/test/ModularPipelines.UnitTests/Context/CommandLineBuilderTests.cs index 93b18c1e832..b90fb64511e 100644 --- a/test/ModularPipelines.UnitTests/Context/CommandLineBuilderTests.cs +++ b/test/ModularPipelines.UnitTests/Context/CommandLineBuilderTests.cs @@ -872,6 +872,112 @@ await Assert.That(result.ToString()).IsEqualTo( "liquibase --search-path=changelogs update --changelog-file=main.xml"); } + [Test] + public async Task Build_Places_Additional_Arguments_By_Phase_And_Scope() + { + var builder = await GetService(); + + var result = builder.Build(new TestMultiLevelCommandOptions + { + Context = "remote", + Reference = "build-reference", + Follow = true, + Arguments = ["manual"], + AdditionalArguments = + [ + new("--global-unmodeled", IsGlobalOption: true), + new("early-unmodeled", CommandLinePhase.EarlyOperand), + new("--normal-unmodeled"), + new("pass-through", CommandLinePhase.Passthrough), + ], + }); + + await Assert.That(result.ToString()).IsEqualTo( + "docker --global-unmodeled --context remote buildx history logs " + + "early-unmodeled build-reference --normal-unmodeled --follow pass-through manual"); + } + + [Test] + public async Task Build_Places_Additional_Option_Before_Property_Terminator() + { + var builder = await GetService(); + + var result = builder.Build(new TestTerminalOptions + { + Filter = "-1", + AdditionalArguments = + [ + new("--unmodeled"), + ], + }); + + await Assert.That(result.ToString()).IsEqualTo("jq --unmodeled -- -1"); + } + + [Test] + public async Task Build_Places_Additional_Terminal_Arguments_Last() + { + var builder = await GetService(); + + var result = builder.Build(new TestAttributeOptions + { + Force = true, + Arguments = ["manual"], + AdditionalArguments = + [ + new("terminal", CommandLinePhase.Terminal), + ], + }); + + await Assert.That(result.ToString()).IsEqualTo( + "mytool sub command --force manual terminal"); + } + + [Test] + public async Task Build_Rejects_Additional_Terminal_Argument_With_RunSettings() + { + var builder = await GetService(); + + CommandLine Build() => builder.Build(new TestAttributeOptions + { + RunSettings = ["pass-through"], + AdditionalArguments = + [ + new("terminal", CommandLinePhase.Terminal), + ], + }); + + await Assert.That(Build) + .Throws() + .And.HasMessageContaining("end-of-options marker"); + } + + [Test] + public async Task Build_Rejects_Additional_Terminator_Outside_Legacy_Phase() + { + var builder = await GetService(); + CommandLinePhase[] phases = + [ + CommandLinePhase.EarlyOperand, + CommandLinePhase.Normal, + CommandLinePhase.Passthrough, + CommandLinePhase.Terminal, + ]; + + foreach (var phase in phases) + { + CommandLine Build() => builder.Build(new TestTerminalOptions + { + AdditionalArguments = [new("--", phase)], + RunTests = "tests.jq", + }); + + await Assert.That(Build) + .Throws() + .And.HasMessageContaining("legacy end-of-options phase"); + } + } + [Test] public async Task Build_Keeps_MultiLevel_Command_Chain_Atomic() { diff --git a/tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/ModularPipelines.OptionsGenerator.csproj b/tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/ModularPipelines.OptionsGenerator.csproj index 0269ded5106..4bfdc24b084 100644 --- a/tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/ModularPipelines.OptionsGenerator.csproj +++ b/tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/ModularPipelines.OptionsGenerator.csproj @@ -38,6 +38,7 @@ +