CLI: Add support for unsupported and deprecated arguments - #41462
CLI: Add support for unsupported and deprecated arguments#41462David Bennett (dkbennett) wants to merge 15 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends WSLC’s CLI parsing model to explicitly represent Supported, Unsupported, and Deprecated arguments/commands. It hides unsupported/deprecated items from help output, emits consistent warnings for deprecated-but-still-functional arguments, and reports intentionally unsupported features (notably --platform) with a dedicated error code (WSLC_E_NOT_SUPPORTED) plus telemetry.
Changes:
- Add
ArgumentState(Supported/Unsupported/Deprecated) with parser enforcement for unsupported arguments and warning emission for deprecated ones. - Add unsupported-command plumbing (recognized by the parser, rejected with a specific exception, and omitted from help).
- Introduce
WSLC_E_NOT_SUPPORTEDacross SDK/IDL surfaces, symbolic error mapping, localization, and unit tests; apply--platformas unsupported across targeted Docker-compatible commands.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/wslc/WSLCCLIParserUnitTests.cpp | Adds unit coverage verifying unsupported options/aliases/positionals throw the correct typed exception and don’t populate ArgMap. |
| test/windows/wslc/WSLCCLICommandUnitTests.cpp | Adds unit coverage for unsupported commands being hidden/recognized and for --platform being declared unsupported across 10 commands; validates symbolic error name mapping. |
| test/windows/wslc/WSLCCLIArgumentUnitTests.cpp | Adds unit coverage for argument state behavior, deprecated warning formatting, and help output hiding for non-visible args. |
| src/windows/WslcSDK/wslcsdk.h | Adds the WSLC_E_NOT_SUPPORTED HRESULT definition to the native SDK header. |
| src/windows/WslcSDK/winrt/wslcsdk.idl | Adds NotSupported to the WinRT error enum surface. |
| src/windows/wslc/core/Main.cpp | Emits deprecated-argument warnings during parsing, adds UnsupportedFeatureException handling with telemetry and standardized help/error output. |
| src/windows/wslc/core/Exceptions.h | Introduces UnsupportedFeatureType and UnsupportedFeatureException for structured unsupported-feature reporting. |
| src/windows/wslc/core/Command.h | Adds UnsupportedCommand and a virtual GetUnsupportedCommands() surface; adds OutputDeprecatedArgumentWarnings() API. |
| src/windows/wslc/core/Command.cpp | Filters non-visible args from help, recognizes unsupported subcommands, skips unsupported args during validation, and emits deprecated-argument warnings. |
| src/windows/wslc/commands/ImageSaveCommand.cpp | Marks ArgType::Platform as unsupported for image save. |
| src/windows/wslc/commands/ImageRemoveCommand.cpp | Marks ArgType::Platform as unsupported for image remove. |
| src/windows/wslc/commands/ImagePushCommand.cpp | Marks ArgType::Platform as unsupported for image push. |
| src/windows/wslc/commands/ImagePullCommand.cpp | Marks ArgType::Platform as unsupported for image pull. |
| src/windows/wslc/commands/ImageLoadCommand.cpp | Marks ArgType::Platform as unsupported for image load. |
| src/windows/wslc/commands/ImageInspectCommand.cpp | Marks ArgType::Platform as unsupported for image inspect. |
| src/windows/wslc/commands/ImageImportCommand.cpp | Marks ArgType::Platform as unsupported for image import. |
| src/windows/wslc/commands/ImageBuildCommand.cpp | Marks ArgType::Platform as unsupported for image build. |
| src/windows/wslc/commands/ContainerRunCommand.cpp | Marks ArgType::Platform as unsupported for container run. |
| src/windows/wslc/commands/ContainerCreateCommand.cpp | Marks ArgType::Platform as unsupported for container create. |
| src/windows/wslc/commands/ContainerCommand.h | Trivial whitespace cleanup in the container command declaration. |
| src/windows/wslc/arguments/ArgumentParser.cpp | Adds centralized unsupported-argument throwing and enforces it across named, alias, positional, and forwarded argument parsing paths. |
| src/windows/wslc/arguments/ArgumentDefinitions.h | Adds the Platform argument definition (name/kind/description) so it can be recognized where declared. |
| src/windows/wslc/arguments/Argument.h | Introduces ArgumentState and helper predicates (IsSupported, IsVisible, IsDeprecated). |
| src/windows/wslc/arguments/Argument.cpp | Implements CreateUnsupported / CreateDeprecated and updates argument-creation commentary accordingly. |
| src/windows/service/inc/wslc.idl | Adds WSLC_E_NOT_SUPPORTED to the internal IDL error defines. |
| src/windows/common/wslutil.cpp | Adds WSLC_E_NOT_SUPPORTED to common HRESULT→symbolic-name mapping. |
| localization/strings/en-US/Resources.resw | Adds localized strings for unsupported command/option/argument errors and deprecated warnings; adds description for --platform. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
localization/strings/en-US/Resources.resw:3279
- The Platform argument description is user-facing and reads ungrammatically (missing articles). Consider rephrasing so it’s a complete sentence (e.g., “Set the platform if the server is multi-platform capable”).
<data name="WSLCCLI_PlatformArgDescription" xml:space="preserve">
<value>Set platform if server is multi-platform capable</value>
</data>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/windows/wslc/arguments/Argument.cpp:74
- CreateUnsupported/CreateDeprecated always call Create(type, false, ...) which forces deprecated/unsupported args to be optional. If a deprecated argument needs to remain required for compatibility, this will silently disable required-argument validation.
Argument Argument::CreateUnsupported(ArgType type, std::optional<argument::Limit> limit)
{
auto argument = Create(type, false, limit);
argument.m_state = ArgumentState::Unsupported;
return argument;
}
Argument Argument::CreateDeprecated(ArgType type, std::optional<argument::Limit> limit, std::optional<std::wstring> desc)
{
auto argument = Create(type, false, limit, std::move(desc));
argument.m_state = ArgumentState::Deprecated;
return argument;
}
src/windows/wslc/arguments/Argument.h:74
- CreateUnsupported/CreateDeprecated hardcode required=false via their implementations, and their signatures don’t allow callers to mark a deprecated argument as required. That makes it impossible to deprecate a required positional/option while preserving existing validation behavior.
// Creates an argument that is recognized by the parser but rejected and omitted from help.
static Argument CreateUnsupported(ArgType type, std::optional<argument::Limit> limit = std::nullopt);
// Creates an argument that is accepted and omitted from help. Its description provides guidance in the warning.
static Argument CreateDeprecated(ArgType type, std::optional<argument::Limit> limit = std::nullopt, std::optional<std::wstring> desc = std::nullopt);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/windows/wslc/arguments/Argument.cpp:74
- Argument::CreateDeprecated currently forwards
desc = std::nulloptinto Argument::Create(), which means deprecated arguments default to the normal help description from ArgumentDefinitions.h. Since Command::OutputDeprecatedArgumentWarnings treatsDescription()as optional deprecation guidance, this will accidentally emit warnings like "'--force' is deprecated. " unless every caller remembers to pass an empty string.
Argument Argument::CreateDeprecated(ArgType type, std::optional<argument::Limit> limit, std::optional<std::wstring> desc)
{
auto argument = Create(type, false, limit, std::move(desc));
argument.m_state = ArgumentState::Deprecated;
return argument;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (1)
localization/strings/en-US/Resources.resw:2244
- The localized usage template now contains two spaces after
Usage:("Usage: {} {}"). This changes help/usage output formatting and can break downstream string-matching (including existing tests/scripts) without a clear functional reason. Prefer keeping a single space ("Usage: {} {}") and, if extra alignment is needed, handle it in the help-rendering code instead of the localized string.
<data name="WSLCCLI_Usage" xml:space="preserve">
<value>Usage: {} {}</value>
<comment>{FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 28 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/windows/wslc/arguments/ArgumentParser.cpp:77
GetUnsupportedArguments()currently only supports option-style arguments: the parser enforcesargument.IsOption()for every declared unsupported type. This contradicts the PR description claiming unsupported positional arguments are recognized/rejected via the standard argument-error path. Either update the PR description to reflect the current (options-only) behavior, or extend unsupported handling to cover positional/forward arguments (which would likely require a distinct localized error message and help rendering path).
THROW_HR_IF_MSG(E_INVALIDARG, !argument.IsOption(), "Unsupported argument type %d is not an option", static_cast<int>(type));
There was a problem hiding this comment.
🔵 Needs a closer look
The new WSLCCLI_DeprecatedArgumentWarning RESW <comment> format likely fails the repo’s localization comment validator (duplicate {FixedPlaceholder="{}"} marker).
Review details
Suppressed comments (1)
localization/strings/en-US/Resources.resw:2214
- The
<comment>forWSLCCLI_DeprecatedArgumentWarningincludes two{FixedPlaceholder="{}"}tokens. The repo’s localization validator (tools/devops/validate-localization.pygenerate_string_comment()) expects at most one{FixedPlaceholder="{}"}marker when a string uses any{}inserts, so this is likely to fail comment validation in CI.
<data name="WSLCCLI_DeprecatedArgumentWarning" xml:space="preserve">
<value>'{}' is deprecated. Use {} instead.</value>
<comment>{FixedPlaceholder="{}"}{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
- Files reviewed: 25/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary of the Pull Request
Adds command-level declarations for unsupported options and deprecated argument mappings. Unsupported options remain recognized for intentional WSLC/Docker divergences but are hidden from help and rejected through the normal argument-error path. Deprecated spellings act as hidden aliases for canonical replacement arguments and warn users to use the replacement.
This applies the unsupported option model to Docker-compatible
--platformoptions and provides reusable deprecation plumbing for compatibility syntax. Parsing, validation, and execution use only canonical arguments.PR Checklist
Detailed Description of the Pull Request / Additional comments
GetArguments()remains limited to executable arguments.ArgTypeand follow normal last-value or accumulation behavior.ArgTypevalues, and include hidden syntax in command-tree collision checks.--platformunsupported on 10 Docker-compatible commands and align generatedUsage:heading spacing with Docker.Unsupported positional arguments, unsupported commands, and replacementless deprecations are out of scope.
Validation Steps Performed
--platformon all 10 affected commands.