From 5aa6faf4fd436b6a31f55b2afcc8bc9ce2e907b0 Mon Sep 17 00:00:00 2001 From: Evangelink <11340282+Evangelink@users.noreply.github.com> Date: Fri, 22 May 2026 14:54:53 +0200 Subject: [PATCH 1/2] Add RFC 014 - Command-line option mappings Introduces a proposal for command-line option mappings: a new extensibility point that lets an extension declaratively accept a user-facing option (e.g. `--logger trx`, `--collect "XPlat Code Coverage"`) and rewrite it, at parse time, into one or more first-class MTP options. The primary scenario is easing migration from VSTest to MTP without polluting the canonical MTP option set. Multiple extensions are allowed to register the same mapping name, with exactly one expected to claim responsibility for a given argument value. Refs #7249 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/RFCs/014-Command-Line-Option-Mappings.md | 354 ++++++++++++++++++ 1 file changed, 354 insertions(+) create mode 100644 docs/RFCs/014-Command-Line-Option-Mappings.md diff --git a/docs/RFCs/014-Command-Line-Option-Mappings.md b/docs/RFCs/014-Command-Line-Option-Mappings.md new file mode 100644 index 0000000000..fdf8f4f623 --- /dev/null +++ b/docs/RFCs/014-Command-Line-Option-Mappings.md @@ -0,0 +1,354 @@ +# RFC 014 - Command-line option mappings + +- [ ] Approved in principle +- [ ] Under discussion +- [ ] Implementation +- [ ] Shipped + +## Summary + +Introduce **command-line option mappings**: a new extensibility point in Microsoft.Testing.Platform (MTP) that lets an extension declaratively accept a user-facing option (e.g. `--logger trx`, `--collect "XPlat Code Coverage"`) and rewrite it, at parse time, into one or more first-class MTP options (e.g. `--report-trx`, `--coverage`). Mappings differ from regular `CommandLineOption` instances in two ways: multiple extensions are allowed to register the *same* mapping name, and exactly one of them is expected to claim responsibility for a given argument value. The primary scenario is making the migration from VSTest to MTP feel less abrupt without polluting the canonical MTP option set. + +## Motivation + +### The migration pain point + +VSTest has long-established, user-facing options that fan out to many independent extensions: + +```text +dotnet test --logger trx --logger "console;verbosity=detailed" --collect "XPlat Code Coverage" --collect "blame" +``` + +In MTP, each of these capabilities is exposed by a distinct extension that registers its own option(s): + +| VSTest | MTP equivalent | +|---|---| +| `--logger trx` | `--report-trx` (`Microsoft.Testing.Extensions.TrxReport`) | +| `--logger "console;verbosity=detailed"` | `--output detailed` (`Microsoft.Testing.Platform`) | +| `--collect "XPlat Code Coverage"` | `--coverage` (`Microsoft.Testing.Extensions.CodeCoverage`) | +| `--collect "blame"` | `--crashdump` / `--hangdump` (`Microsoft.Testing.Extensions.CrashDump` / `…HangDump`) | + +This is a deliberate design choice: each capability is its own MTP extension with its own option schema, its own validation, and its own help entry. It is also a real source of friction for the very large population of users who already type `--logger` and `--collect` from muscle memory, who have CI pipelines that pass these flags, and who follow blog posts and Stack Overflow answers that were written for VSTest. + +Today, MTP responds to these flags with `Unknown option '--logger'`. That answer is correct but unhelpful. + +### Why this can't be solved with regular `CommandLineOption` + +The existing `ICommandLineOptionsProvider` contract forbids two extensions from registering the same option name (`CommandLineOptionsValidator` treats duplicate names as a fatal error). That rule is essential: it guarantees that for any option, there is exactly one provider responsible for its validation, its argument arity, and the meaning of `ICommandLineOptions.TryGetOptionArgumentList`. + +But `--logger` and `--collect` are intrinsically polyvalent: the *value* selects which extension is responsible. There is no extension that owns `--logger` as a whole — there is one extension that owns `--logger trx`, another that would own `--logger console`, etc. The single-owner-per-option rule that protects normal options is exactly what blocks expressing this. + +### Why not just add `--logger` to a single extension + +A naive workaround — making, say, the TRX extension own `--logger` and dispatch by value — collapses the moment a second VSTest-compatible extension wants to participate. It also leaks an unrelated extension's identity (TRX) into the dispatcher for unrelated values (`console`, `html`). The mapping mechanism described below is the smallest surface that lets multiple extensions cooperate without breaking the single-owner invariant for canonical MTP options. + +## Naming + +Earlier discussion (issue [#7249](https://github.com/microsoft/testfx/issues/7249)) proposed the term **alias**. Feedback noted that "alias" in CLI conventions usually means a pure rename (`-v` ≡ `--verbose`) with identical arity and semantics, whereas this feature actually rewrites one option into one *or more* options with potentially different arity. Shell aliases (`alias ll='ls -la'`) and Git aliases are precedents for the broader meaning, but the ambiguity is real for API consumers. + +This RFC uses **mapping** in the public API surface (`ICommandLineOptionMapping`, `CommandLineOptionMapping`, `CommandLineMappings`). Rationale: + +- "Mapping" honestly conveys *one user-facing token mapped to one or more canonical options*. +- It avoids the rename connotation of "alias". +- It avoids the compiler-pass connotation of "transformation". +- It avoids the deprecation connotation of "shim" or "legacy", leaving room for non-VSTest uses. + +User-facing documentation may still refer to these informally as "compatibility aliases" or "VSTest-style options" — the public API name does not constrain documentation prose. + +## Design principles + +1. **Mappings never replace canonical options.** Every capability addressable through a mapping is *also* addressable through its canonical MTP option. Mappings are purely additive sugar. +2. **Mappings are resolved before any option provider sees the command line.** A mapping turns `--logger trx` into `--report-trx` *before* `ICommandLineOptionsProvider.ValidateOptionArgumentsAsync` is called on any provider. Providers therefore never need to know that mappings exist. +3. **Exactly one mapping handles each occurrence.** Zero handlers → error. Multiple handlers → error. There is no "first match wins" silent ambiguity. +4. **Mappings cannot loop.** A mapping rewrites to canonical options only, never to other mapping names. This is a property checked at registration / validation time. +5. **Mappings have no service dependencies.** They run at command-line parse time, before the DI container is built. The contract is intentionally narrow. +6. **Mappings are opt-in for extension authors.** Existing extensions are unaffected. A new MTP project that does not register any mapping behaves exactly as today. + +## Detailed design + +### Public API surface + +A new namespace under the existing command-line extensibility surface: + +```csharp +namespace Microsoft.Testing.Platform.Extensions.CommandLine; + +/// +/// Provides one or more entries that rewrite a user-facing +/// option name into one or more canonical MTP command-line options at parse time. +/// +public interface ICommandLineOptionMappingProvider : IExtension +{ + IReadOnlyCollection GetCommandLineOptionMappings(); +} + +/// +/// A single mapping registration. Multiple providers may register mappings with the same +/// ; exactly one of them is expected to return true from +/// for any given argument list. +/// +public sealed class CommandLineOptionMapping +{ + public CommandLineOptionMapping( + string name, + string description, + ArgumentArity arity, + CommandLineOptionMapper map); + + /// The user-facing option name (without leading dashes), e.g. "logger" or "collect". + public string Name { get; } + + /// Help-text description. Multiple providers registering the same name MAY have different descriptions; the help renderer concatenates them. + public string Description { get; } + + /// Arity of the user-facing option (typically ). + public ArgumentArity Arity { get; } + + /// The rewriter delegate. See . + public CommandLineOptionMapper Map { get; } +} + +/// +/// Attempts to rewrite a single occurrence of a mapped option. +/// +/// The argument list provided to that occurrence. Never null; length respects . +/// When the method returns true, the canonical options this occurrence expands into; otherwise empty. +/// True if this mapping claims the occurrence; false otherwise. +public delegate bool CommandLineOptionMapper( + ReadOnlySpan arguments, + out IReadOnlyList result); + +/// +/// A single canonical option produced by a mapping. +/// +public sealed class CommandLineOptionMappingResult +{ + public CommandLineOptionMappingResult(string optionName, params string[] arguments); + + /// The canonical MTP option name (without leading dashes). + public string OptionName { get; } + + /// Arguments for the canonical option. May be empty for switch-style options. + public IReadOnlyList Arguments { get; } +} +``` + +Registration mirrors the existing `ICommandLineOptionsProvider` pattern: + +```csharp +namespace Microsoft.Testing.Platform.Builder; + +public interface ICommandLineMappingsManager +{ + void AddProvider(Func providerFactory); +} + +public interface ITestApplicationBuilder +{ + // existing members ... + ICommandLineMappingsManager CommandLineMappings { get; } +} +``` + +### Example: TRX + +```csharp +testApplicationBuilder.CommandLineMappings.AddProvider(_ => new TrxLoggerMapping()); + +internal sealed class TrxLoggerMapping : ICommandLineOptionMappingProvider +{ + public string Uid => nameof(TrxLoggerMapping); + public string Version => "1.0.0"; + public string DisplayName => "TRX logger compatibility mapping"; + public string Description => "Accepts the VSTest '--logger trx[;LogFileName=...]' syntax."; + public Task IsEnabledAsync() => Task.FromResult(true); + + public IReadOnlyCollection GetCommandLineOptionMappings() => + [ + new( + name: "logger", + description: "VSTest-compatible logger selector. Supported: trx[;LogFileName=].", + arity: ArgumentArity.ExactlyOne, + map: TryMap), + ]; + + private static bool TryMap(ReadOnlySpan arguments, out IReadOnlyList result) + { + string value = arguments[0]; + + if (value == "trx") + { + result = [new CommandLineOptionMappingResult("report-trx")]; + return true; + } + + if (value.StartsWith("trx;", StringComparison.OrdinalIgnoreCase)) + { + List expanded = [new CommandLineOptionMappingResult("report-trx")]; + foreach (string segment in value.AsSpan(4).ToString().Split(';')) + { + if (segment.StartsWith("LogFileName=", StringComparison.OrdinalIgnoreCase)) + { + expanded.Add(new CommandLineOptionMappingResult("report-trx-filename", segment.Substring("LogFileName=".Length))); + } + // unknown segments fall through; see "Unknown sub-options" below. + } + + result = expanded; + return true; + } + + result = []; + return false; + } +} +``` + +### Example: Code coverage + +```csharp +internal sealed class CodeCoverageCollectMapping : ICommandLineOptionMappingProvider +{ + // ... metadata omitted ... + + public IReadOnlyCollection GetCommandLineOptionMappings() => + [ + new( + name: "collect", + description: "VSTest-compatible data collector. Supported: 'XPlat Code Coverage', 'Code Coverage'.", + arity: ArgumentArity.ExactlyOne, + map: static (args, out result) => + { + if (string.Equals(args[0], "XPlat Code Coverage", StringComparison.OrdinalIgnoreCase) || + string.Equals(args[0], "Code Coverage", StringComparison.OrdinalIgnoreCase)) + { + result = [new CommandLineOptionMappingResult("coverage")]; + return true; + } + + result = []; + return false; + }), + ]; +} +``` + +### Resolution algorithm + +`CommandLineParser` is augmented with a post-parse pass that runs **after** tokenization and response-file expansion, **before** option validation: + +1. Build a lookup table `MappingsByName : string → List` from every registered provider. +2. For each parsed option occurrence `--X v1 v2`: + - If `X` is not in `MappingsByName`, keep the occurrence unchanged. + - Otherwise, for every mapping registered under `X`, call `TryMap` exactly once. + - **Zero claimants** → emit an error: `No registered mapping for '--X' can handle value 'v1'. Run '--help' to see canonical alternatives.` + - **Multiple claimants** → emit an error: `Multiple mappings claim '--X v1': . This is a configuration bug — at most one mapping may handle a given value.` + - **Exactly one claimant** → replace the occurrence with the canonical occurrences it returned. +3. After all occurrences are resolved, re-run aggregation and arity validation against the resulting canonical-option set (existing `CommandLineOptionsValidator` code path, unchanged). + +The user's original `string[] args` is preserved alongside the expanded set for diagnostics (`--info`, error messages, telemetry). + +### Cross-cutting validation rules + +Enforced at platform startup, before any user input is processed: + +- A mapping name MUST NOT collide with any registered canonical `CommandLineOption.Name`. Diagnostic: `Mapping '--X' conflicts with the canonical option '--X' provided by . Rename one.` +- A `CommandLineOptionMappingResult.OptionName` MUST refer to a registered canonical `CommandLineOption`. Diagnostic: `Mapping '--X' rewrites to unknown option '--Y'.` +- A `CommandLineOptionMappingResult.OptionName` MUST NOT itself be a mapping name (no mapping-to-mapping rewriting; prevents loops and makes the resolution algorithm finite by construction). +- Mapping names follow the same character set as canonical option names (letters, digits, `-`, `?`). + +### Interaction with existing surfaces + +| Surface | Behaviour | +|---|---| +| `ICommandLineOptions.IsOptionSet("report-trx")` | Returns `true` if the user passed `--report-trx` *or* `--logger trx`. Providers don't need to know which path was taken. | +| `ICommandLineOptions.TryGetOptionArgumentList` | Same — the canonical option is what providers query. | +| `--help` | Help renderer adds a new section, **VSTest-style options (compatibility)**, listing every mapping with its description. Canonical options remain the primary listing. | +| `--info` | Echoes the original `args` *and* the resolved canonical form, so users can self-diagnose how a mapping expanded. | +| Response files (`@file.rsp`) | Mappings run after response-file expansion, so `--logger trx` works whether it was typed at the prompt or pulled from a `.rsp` file. | +| Error messages | "Unknown option '--logger'" becomes either a mapping error (if no provider claims the value) or a missing-extension hint (if the *value* is not recognised). | +| Telemetry | The resolved canonical options are what is reported. The fact that a mapping was used is reported as a single boolean (`used_compat_mapping`) without value-level detail, to avoid PII risk. | + +### Backwards compatibility for mapping authors + +The mapping API is additive. Existing `ICommandLineOptionsProvider` implementations are untouched. A test app that does not register `ICommandLineOptionMappingProvider` is byte-for-byte identical in behaviour to today. + +### What about `--logger console;verbosity=detailed`? + +In VSTest, the console logger options govern terminal output. In MTP, terminal output is governed by `--output` and the terminal-test-reporter options. A mapping for `--logger console` would naturally live in the same package that owns `--output`, and would translate `verbosity=detailed` to `--output detailed`, `verbosity=normal` to `--output normal`, etc. This is exemplary, not normative for this RFC — the mechanism is what we're shipping, not a fixed list of mappings. + +### Sub-option handling and the `trx;LogFileName=` shape + +`--logger trx;LogFileName=foo.trx` parses as a single argument value `trx;LogFileName=foo.trx` from MTP's standpoint (the `;` is inside the value, not a separator). The mapping is responsible for splitting on `;` and producing the right canonical options (`--report-trx --report-trx-filename foo.trx`). The platform doesn't prescribe a sub-option syntax — mappings are free to mimic VSTest's `name;k=v;k=v` exactly. + +**Unknown sub-options** (`trx;UnsupportedKnob=42`): the mapping author decides. Recommended default is to ignore unknown sub-options and surface a warning via the `IOutputDisplay` available *after* resolution; mappings themselves cannot warn synchronously because they run pre-DI. A future iteration could add a structured warning channel to the `CommandLineOptionMapper` delegate. + +## Drawbacks + +1. **Two ways to do the same thing.** `--report-trx` and `--logger trx` will both be valid. We accept this as the price of migration ergonomics; mitigation is the dedicated "compatibility" section in `--help` and `--info`'s "resolved-to" output. +2. **Extension surface gains a concept.** Authors must now distinguish between "register an option" and "register a mapping". Mitigated by the fact that most extensions will never need a mapping — only those interested in courting VSTest users will. +3. **Mappings run before DI.** Mapping authors cannot read configuration, call services, or log diagnostics from inside `TryMap`. This is a deliberate constraint to keep the resolution step early and side-effect-free. +4. **Cross-extension cooperation by convention.** Two extensions both registering a `--collect` mapping must agree on disjoint value sets (`XPlat Code Coverage` vs. `blame`). The platform detects collisions and refuses to start, but it cannot prevent a future third party from grabbing a value another extension expected. This is identical to the existing risk for any extension namespace. +5. **VSTest semantic drift.** Some VSTest options (`--collect "blame"` collecting hangs *and* crashes, `--collect "blame;CollectHangDump"` toggling between them) have multi-extension semantics that no single mapping can express. Documented as "best-effort compatibility, not bug-for-bug compatibility". + +## Alternatives + +### Alternative 1 — Per-option ownership in a single dispatcher extension + +Have one extension own `--logger` and dispatch to a registry. Rejected: collapses the moment a second extension wants to participate, and couples unrelated extensions' identities to the dispatcher. + +### Alternative 2 — `init`/append style multi-ownership of regular options + +Allow multiple providers to register the same `CommandLineOption` and call all of their `ValidateOptionArgumentsAsync` methods. Rejected: breaks the single-owner invariant for canonical options, which would force every existing provider to defensively check that *its* extension is the one responsible — the same dispatching logic, but smeared across every option in the platform. + +### Alternative 3 — Pre-process `args` in the user's `Program.cs` + +Document the recommended rewriting in prose and let users add a `if (args.Contains("--logger")) { … }` shim in their `Main`. Rejected: every consumer has to re-implement the same logic, every test framework's MSBuild integration needs a separate plan, and IDEs that invoke the test host directly don't get the benefit. + +### Alternative 4 — Don't do this; tell users to migrate + +Reject the proposal entirely and rely on documentation. This is the current state. It works for greenfield projects but is a real source of friction for the very large body of existing CI pipelines, scripts, and tribal knowledge that says "pass `--logger trx`". + +### Alternative 5 — Naming variants considered + +| Considered name | Why rejected | +|---|---| +| `CommandLineAlias` | Misleading; "alias" implies a pure rename. (See **Naming** above.) | +| `CommandLineOptionTransformation` | Accurate but evokes compiler passes / heavy machinery. | +| `CommandLineOptionExpander` | Accurate for 1→N case but awkward for the 1→1 case. | +| `CommandLineOptionShim` | "Shim" carries negative / temporary connotation; we may want non-VSTest uses. | +| `LegacyCommandLineOption` | Too narrow; precludes any non-compat use. | + +`CommandLineOptionMapping` was chosen as the least misleading and most honest about 1→N behaviour. + +## Compatibility + +- **Not a breaking change.** All new types live in `Microsoft.Testing.Platform.Extensions.CommandLine` and are opt-in. Existing `ICommandLineOptionsProvider` implementations are untouched. A test app that doesn't reference any mapping provider behaves identically to today. +- **Public API additions** are listed in `PublicAPI.Unshipped.txt` of `Microsoft.Testing.Platform` per repo policy. +- **Help-text additions** require updating the wildcard expectations in: + - `test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoTests.cs` + - `test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs` + - `test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MSBuild.KnownExtensionRegistration.cs` + - `test/IntegrationTests/MSTest.Acceptance.IntegrationTests/HelpInfoTests.cs` +- **VSTest behavioural compatibility is best-effort.** Where VSTest's `--logger` / `--collect` have semantics that no single mapping can express (cross-extension `blame`, free-form data-collector keys), the mapping documents the supported subset and produces a clear error for unsupported values. We do not promise bug-for-bug compatibility. +- **No init accessors** introduced (per repo public-API guidelines). + +## Phasing + +| Phase | Deliverable | Owner | +|---|---|---| +| 1 | Mapping infrastructure (`ICommandLineOptionMappingProvider`, parser integration, validation, help/info plumbing). | `Microsoft.Testing.Platform` | +| 2 | First mapping shipped: `--logger trx` in `Microsoft.Testing.Extensions.TrxReport`. | TRX extension | +| 3 | `--collect "XPlat Code Coverage"` / `--collect "Code Coverage"` in `Microsoft.Testing.Extensions.CodeCoverage`. | Code coverage extension | +| 4 | `--logger console;verbosity=…` mapping co-located with the terminal test reporter options. | `Microsoft.Testing.Platform` | +| 5 | `--collect "blame"` shape mapping in `Microsoft.Testing.Extensions.CrashDump` / `…HangDump` (split-extension story documented). | Crash/Hang dump extensions | + +Each phase is independently shippable. Phase 1 alone is useless to users; phase 1 + phase 2 already covers the single most common VSTest pipeline. + +## Unresolved questions + +1. **Mapping-author warning channel.** Should `TryMap` be able to surface a non-fatal warning (e.g. "ignored unknown sub-option `Foo`")? A pre-DI synchronous callback into a buffered `List` is the cheapest design; this RFC defers the decision but reserves the option of adding a `warnings` out-parameter later in an additive way. +2. **Help integration depth.** Should mappings appear interleaved with the canonical options that they expand to, or in a separate "VSTest-style options" section? Current proposal is a separate section; reviewers may prefer interleaving for discoverability. +3. **MSBuild / `RunSettings` interplay.** VSTest's `--logger`/`--collect` can also be specified through `.runsettings`. Whether MTP's MSBuild integration should translate `.runsettings` data-collector entries into mapping invocations is out of scope for this RFC but is the natural follow-up. +4. **Telemetry granularity.** Reporting the *value* a mapping handled (e.g. `trx`, `XPlat Code Coverage`) is potentially valuable for prioritising which mappings to invest in, but mapping authors might receive arbitrary user-supplied strings. The current proposal logs only a boolean; an allowlist of well-known values could be added later. +5. **Should mappings be allowed to opt into running after configuration is loaded?** Some compatibility scenarios (e.g. respecting a `runsettings` opt-out) would benefit from access to configuration. The current proposal keeps mappings pre-DI for simplicity; a future RFC could add a "deferred mapping" tier if a concrete need arises. From 115828ad9e802becc35f7c03cb26c6f4dfdce4de Mon Sep 17 00:00:00 2001 From: Amaury Leveugle <11340282+Evangelink@users.noreply.github.com> Date: Sat, 23 May 2026 17:23:36 +0200 Subject: [PATCH 2/2] Address RFC 014 review feedback - Renumber to 015 to avoid collision with the existing 014-TestRun-Current-PlannedTests RFC. - Align Naming section with the actual proposed types (ICommandLineOptionMappingProvider / ICommandLineMappingManager, matching the singular convention used elsewhere). - Fix the broken reference to point at the Map property. - Make the TRX example case-insensitive for both the exact "trx" check and the "trx;" prefix, matching VSTest-compat expectations. - Replace ReadOnlySpan with IReadOnlyList in the public delegate; the platform targets netstandard2.0 and Span-shaped APIs add a System.Memory dependency to the public surface for limited benefit. - Replace params string[] with IReadOnlyList in CommandLineOptionMappingResult to keep the constructor open for future parameters and avoid unnecessary array allocations for the (common) zero/one-argument case. - Document the empty-result contract violation explicitly and split startup-time vs runtime cross-cutting validation rules so the RFC matches what can actually be enforced when the Map delegate is opaque. - Expand the TRX example with malformed-segment handling and a sub- option cap to address the defensive-coding concerns about unbounded user input. - Note in Unresolved Questions that adding an out-parameter to the delegate is a binary-breaking change, so the warning-channel decision must be made before shipping (or absorbed via a sibling delegate). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...md => 015-Command-Line-Option-Mappings.md} | 77 ++++++++++++------- 1 file changed, 51 insertions(+), 26 deletions(-) rename docs/RFCs/{014-Command-Line-Option-Mappings.md => 015-Command-Line-Option-Mappings.md} (82%) diff --git a/docs/RFCs/014-Command-Line-Option-Mappings.md b/docs/RFCs/015-Command-Line-Option-Mappings.md similarity index 82% rename from docs/RFCs/014-Command-Line-Option-Mappings.md rename to docs/RFCs/015-Command-Line-Option-Mappings.md index fdf8f4f623..cab631e38c 100644 --- a/docs/RFCs/014-Command-Line-Option-Mappings.md +++ b/docs/RFCs/015-Command-Line-Option-Mappings.md @@ -1,4 +1,4 @@ -# RFC 014 - Command-line option mappings +# RFC 015 - Command-line option mappings - [ ] Approved in principle - [ ] Under discussion @@ -46,7 +46,7 @@ A naive workaround — making, say, the TRX extension own `--logger` and dispatc Earlier discussion (issue [#7249](https://github.com/microsoft/testfx/issues/7249)) proposed the term **alias**. Feedback noted that "alias" in CLI conventions usually means a pure rename (`-v` ≡ `--verbose`) with identical arity and semantics, whereas this feature actually rewrites one option into one *or more* options with potentially different arity. Shell aliases (`alias ll='ls -la'`) and Git aliases are precedents for the broader meaning, but the ambiguity is real for API consumers. -This RFC uses **mapping** in the public API surface (`ICommandLineOptionMapping`, `CommandLineOptionMapping`, `CommandLineMappings`). Rationale: +This RFC uses **mapping** in the public API surface (`ICommandLineOptionMappingProvider`, `CommandLineOptionMapping`, `ICommandLineMappingManager`). Rationale: - "Mapping" honestly conveys *one user-facing token mapped to one or more canonical options*. - It avoids the rename connotation of "alias". @@ -84,8 +84,8 @@ public interface ICommandLineOptionMappingProvider : IExtension /// /// A single mapping registration. Multiple providers may register mappings with the same -/// ; exactly one of them is expected to return true from -/// for any given argument list. +/// ; exactly one of them is expected to return true from the +/// delegate for any given argument list. /// public sealed class CommandLineOptionMapping { @@ -111,11 +111,11 @@ public sealed class CommandLineOptionMapping /// /// Attempts to rewrite a single occurrence of a mapped option. /// -/// The argument list provided to that occurrence. Never null; length respects . -/// When the method returns true, the canonical options this occurrence expands into; otherwise empty. -/// True if this mapping claims the occurrence; false otherwise. +/// The argument list provided to that occurrence. Never null; count respects . +/// When the method returns true, the canonical options this occurrence expands into; MUST be non-null and non-empty. Otherwise empty. +/// True if this mapping claims the occurrence; false otherwise. Returning true with an empty is a contract violation and produces a platform-level error. public delegate bool CommandLineOptionMapper( - ReadOnlySpan arguments, + IReadOnlyList arguments, out IReadOnlyList result); /// @@ -123,7 +123,7 @@ public delegate bool CommandLineOptionMapper( /// public sealed class CommandLineOptionMappingResult { - public CommandLineOptionMappingResult(string optionName, params string[] arguments); + public CommandLineOptionMappingResult(string optionName, IReadOnlyList arguments); /// The canonical MTP option name (without leading dashes). public string OptionName { get; } @@ -138,7 +138,7 @@ Registration mirrors the existing `ICommandLineOptionsProvider` pattern: ```csharp namespace Microsoft.Testing.Platform.Builder; -public interface ICommandLineMappingsManager +public interface ICommandLineMappingManager { void AddProvider(Func providerFactory); } @@ -146,7 +146,7 @@ public interface ICommandLineMappingsManager public interface ITestApplicationBuilder { // existing members ... - ICommandLineMappingsManager CommandLineMappings { get; } + ICommandLineMappingManager CommandLineMappings { get; } } ``` @@ -172,24 +172,44 @@ internal sealed class TrxLoggerMapping : ICommandLineOptionMappingProvider map: TryMap), ]; - private static bool TryMap(ReadOnlySpan arguments, out IReadOnlyList result) + private static bool TryMap(IReadOnlyList arguments, out IReadOnlyList result) { string value = arguments[0]; - if (value == "trx") + if (string.Equals(value, "trx", StringComparison.OrdinalIgnoreCase)) { - result = [new CommandLineOptionMappingResult("report-trx")]; + result = [new CommandLineOptionMappingResult("report-trx", Array.Empty())]; return true; } if (value.StartsWith("trx;", StringComparison.OrdinalIgnoreCase)) { - List expanded = [new CommandLineOptionMappingResult("report-trx")]; - foreach (string segment in value.AsSpan(4).ToString().Split(';')) + // Defensive coding: callers cannot bound the number of sub-options the user types + // (e.g. `--logger "trx;a=1;b=2;...;z=99"`). Mapping authors SHOULD apply a sensible + // upper bound on the number of segments they walk through to avoid quadratic work + // or excessive allocations from malformed input. + const int MaxTrxSubOptions = 16; + List expanded = [new CommandLineOptionMappingResult("report-trx", Array.Empty())]; + string[] segments = value.Substring("trx;".Length).Split(';'); + int processed = 0; + foreach (string segment in segments) { + if (++processed > MaxTrxSubOptions) + { + break; + } + + // Recommended handling for malformed known options (e.g. "LogFileName" with no + // '=' or with empty value): treat as a no-op and surface a post-DI warning rather + // than failing the whole command line. The platform does not prescribe a specific + // policy — mappings are free to be stricter if they prefer. if (segment.StartsWith("LogFileName=", StringComparison.OrdinalIgnoreCase)) { - expanded.Add(new CommandLineOptionMappingResult("report-trx-filename", segment.Substring("LogFileName=".Length))); + string fileName = segment.Substring("LogFileName=".Length); + if (!string.IsNullOrEmpty(fileName)) + { + expanded.Add(new CommandLineOptionMappingResult("report-trx-filename", new[] { fileName })); + } } // unknown segments fall through; see "Unknown sub-options" below. } @@ -198,7 +218,7 @@ internal sealed class TrxLoggerMapping : ICommandLineOptionMappingProvider return true; } - result = []; + result = Array.Empty(); return false; } } @@ -222,11 +242,11 @@ internal sealed class CodeCoverageCollectMapping : ICommandLineOptionMappingProv if (string.Equals(args[0], "XPlat Code Coverage", StringComparison.OrdinalIgnoreCase) || string.Equals(args[0], "Code Coverage", StringComparison.OrdinalIgnoreCase)) { - result = [new CommandLineOptionMappingResult("coverage")]; + result = [new CommandLineOptionMappingResult("coverage", Array.Empty())]; return true; } - result = []; + result = Array.Empty(); return false; }), ]; @@ -240,23 +260,28 @@ internal sealed class CodeCoverageCollectMapping : ICommandLineOptionMappingProv 1. Build a lookup table `MappingsByName : string → List` from every registered provider. 2. For each parsed option occurrence `--X v1 v2`: - If `X` is not in `MappingsByName`, keep the occurrence unchanged. - - Otherwise, for every mapping registered under `X`, call `TryMap` exactly once. + - Otherwise, for every mapping registered under `X`, call its `Map` delegate exactly once. - **Zero claimants** → emit an error: `No registered mapping for '--X' can handle value 'v1'. Run '--help' to see canonical alternatives.` - **Multiple claimants** → emit an error: `Multiple mappings claim '--X v1': . This is a configuration bug — at most one mapping may handle a given value.` - - **Exactly one claimant** → replace the occurrence with the canonical occurrences it returned. + - **Exactly one claimant returning a non-empty result** → replace the occurrence with the canonical occurrences it returned. + - **Exactly one claimant returning true with an empty or null result** → contract violation; emit an error: `Mapping '--X' claimed value 'v1' but produced no canonical options. This is a bug in the mapping provider .` 3. After all occurrences are resolved, re-run aggregation and arity validation against the resulting canonical-option set (existing `CommandLineOptionsValidator` code path, unchanged). The user's original `string[] args` is preserved alongside the expanded set for diagnostics (`--info`, error messages, telemetry). ### Cross-cutting validation rules -Enforced at platform startup, before any user input is processed: +Some rules can be enforced **at platform startup**, before any user input is processed: - A mapping name MUST NOT collide with any registered canonical `CommandLineOption.Name`. Diagnostic: `Mapping '--X' conflicts with the canonical option '--X' provided by . Rename one.` -- A `CommandLineOptionMappingResult.OptionName` MUST refer to a registered canonical `CommandLineOption`. Diagnostic: `Mapping '--X' rewrites to unknown option '--Y'.` -- A `CommandLineOptionMappingResult.OptionName` MUST NOT itself be a mapping name (no mapping-to-mapping rewriting; prevents loops and makes the resolution algorithm finite by construction). +- A mapping name MUST NOT collide with another mapping registered with a different `description` from a provider that has the same `Uid` (per-provider uniqueness check). - Mapping names follow the same character set as canonical option names (letters, digits, `-`, `?`). +Other rules are inherently **runtime** because the `Map` delegate is opaque at registration time: + +- A `CommandLineOptionMappingResult.OptionName` MUST refer to a registered canonical `CommandLineOption`. Checked when the delegate runs; on violation the platform emits a clear error naming the offending mapping and the unknown option (`Mapping '--X' rewrites to unknown option '--Y'.`) and aborts with `ExitCode.InvalidCommandLine`. There is no startup-time scan because the set of returned options can depend on user input. +- A `CommandLineOptionMappingResult.OptionName` MUST NOT itself be a mapping name (no mapping-to-mapping rewriting; prevents loops). Same runtime enforcement strategy as above. + ### Interaction with existing surfaces | Surface | Behaviour | @@ -347,7 +372,7 @@ Each phase is independently shippable. Phase 1 alone is useless to users; phase ## Unresolved questions -1. **Mapping-author warning channel.** Should `TryMap` be able to surface a non-fatal warning (e.g. "ignored unknown sub-option `Foo`")? A pre-DI synchronous callback into a buffered `List` is the cheapest design; this RFC defers the decision but reserves the option of adding a `warnings` out-parameter later in an additive way. +1. **Mapping-author warning channel.** Should the `Map` delegate be able to surface a non-fatal warning (e.g. "ignored unknown sub-option `Foo`")? A pre-DI synchronous callback into a buffered `List` is the cheapest design. **Because adding a third out-parameter to a delegate is a binary-breaking change**, this RFC must pick a forward-compatible shape *before* shipping. The current proposal keeps the binary delegate signature minimal and defers the warning channel; an additive future iteration would introduce a sibling delegate type (`CommandLineOptionMapperV2`) or a richer context-object overload rather than mutate `CommandLineOptionMapper`. Reviewers should confirm whether they prefer to introduce the context-object shape upfront to avoid the future bifurcation. 2. **Help integration depth.** Should mappings appear interleaved with the canonical options that they expand to, or in a separate "VSTest-style options" section? Current proposal is a separate section; reviewers may prefer interleaving for discoverability. 3. **MSBuild / `RunSettings` interplay.** VSTest's `--logger`/`--collect` can also be specified through `.runsettings`. Whether MTP's MSBuild integration should translate `.runsettings` data-collector entries into mapping invocations is out of scope for this RFC but is the natural follow-up. 4. **Telemetry granularity.** Reporting the *value* a mapping handled (e.g. `trx`, `XPlat Code Coverage`) is potentially valuable for prioritising which mappings to invest in, but mapping authors might receive arbitrary user-supplied strings. The current proposal logs only a boolean; an allowlist of well-known values could be added later.