From 32a8c8cb12d45841e0f8fad6e0735e72109bec02 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 09:34:10 +0000 Subject: [PATCH] feat(cli)!: refuse an argument no command declares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A mistyped flag was accepted, ignored and reported as a success: `fce config show --verbosee` exited 0 having done nothing about verbosity. The parser collects an argument it does not recognise into the remaining arguments, and this tool never reads them — grep finds no use of context.Remaining anywhere in the CLI — so the leftovers were dropped in silence. A pipeline asking for something the tool does not do was told it had it. Rejecting an unknown option is what every comparable tool does: git exits 128, ls and grep exit 2, dotnet build exits 1. Tolerating one is the outlier, and it is the parser's default rather than a choice made here. Refused arguments now report as the usage error 64 that ADR-0067's set already carries, naming the offending argument. The parser's own strict mode says the same thing and is NOT used. In Spectre.Console.Cli 0.55 — the newest published version, so this is not an upgrade away — UseStrictParsing makes an option declared without a value swallow the internal "__default_command" token as that value: `fce generate --solution` then looks for a file by that name and exits 1, instead of reporting a usage error. It trades a silent wrong for a visible one that leaks a parser internal into a user-facing message. Refusing the leftovers through an interceptor keeps that diagnosis intact, which a test now pins. BREAKING CHANGE: a command line carrying an argument no command declares now exits 64 instead of running and exiting 0. Nothing can depend on the previous behaviour deliberately — the arguments were never read — so what breaks is a caller whose invocation is already wrong and silently ignored, which is the point. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SuSBsGG7wMPMnSeTHKZAom --- .../CliApplicationExitCodeTests.cs | 29 ++++++++++- FirstClassErrors.Cli/CliApplication.cs | 48 ++++++++++++++++++- .../CatalogVersioningReference.en.md | 4 +- .../CatalogVersioningReference.fr.md | 4 +- 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/FirstClassErrors.Cli.UnitTests/CliApplicationExitCodeTests.cs b/FirstClassErrors.Cli.UnitTests/CliApplicationExitCodeTests.cs index 062ac0a7..6bd06a1b 100644 --- a/FirstClassErrors.Cli.UnitTests/CliApplicationExitCodeTests.cs +++ b/FirstClassErrors.Cli.UnitTests/CliApplicationExitCodeTests.cs @@ -53,7 +53,34 @@ public void AnUnknownCommandInsideABranchIsAUsageError() { Check.That(error).Contains("frobnicate"); } - [Fact(DisplayName = "An option given without its value is a usage error (64).")] + [Fact(DisplayName = "An option no command declares is a usage error (64), naming the argument.")] + public void AnUndeclaredOptionIsAUsageError() { + // Exercise: config show succeeds on its own, so only the unknown option can be what fails this. + (int exitCode, string error) = Run("config", "show", "--nope"); + + // Verify + Check.That(exitCode).IsEqualTo(64); + Check.That(error).Contains("--nope"); + } + + // The regression this pins: a mistyped flag used to be collected into the parser's remaining arguments — which + // this tool never reads — so the command ran without it and reported success. A pipeline asking for something the + // tool did not do was told it had it. + [Fact(DisplayName = "A mistyped option is refused rather than ignored, and never reports success.")] + public void AMistypedOptionIsRefusedRatherThanIgnored() { + // Exercise + (int exitCode, string error) = Run("config", "show", "--verbosee"); + + // Verify + Check.That(exitCode).IsNotEqualTo(0); + Check.That(exitCode).IsEqualTo(64); + Check.That(error).Contains("--verbosee"); + } + + // Refusing undeclared arguments must not be bought by breaking the parser's own diagnosis. Spectre's strict + // parsing mode would have said the same thing about an unknown option, at the cost of this case: in 0.55 it makes + // a valueless option swallow the internal "__default_command" token, so the tool looks for a file by that name. + [Fact(DisplayName = "An option given without its value is still the parser's usage error (64).")] public void AnOptionWithoutItsValueIsAUsageError() { // Exercise (int exitCode, string error) = Run("generate", "--solution"); diff --git a/FirstClassErrors.Cli/CliApplication.cs b/FirstClassErrors.Cli/CliApplication.cs index 64ccce7e..a34c5961 100644 --- a/FirstClassErrors.Cli/CliApplication.cs +++ b/FirstClassErrors.Cli/CliApplication.cs @@ -1,5 +1,7 @@ #region Usings declarations +using System.Diagnostics.CodeAnalysis; + using Spectre.Console.Cli; #endregion @@ -39,6 +41,17 @@ private static void Configure(IConfigurator config) { config.SetApplicationName("fce"); config.SetExceptionHandler(HandleUncaught); + // An argument the command tree does not declare is refused rather than collected. The parser gathers such a + // token into the remaining arguments, which this tool never reads: a mistyped flag was accepted, ignored, and + // reported as a success, so a pipeline asking for something the tool does not do was told it had it. + // + // The parser's own strict mode (UseStrictParsing) would say the same thing, and cannot be used: in + // Spectre.Console.Cli 0.55 it makes an option declared without a value swallow the internal + // "__default_command" token as that value, so `fce generate --solution` looks for a file by that name instead + // of reporting a usage error. Refusing the leftovers ourselves keeps the diagnosis and leaves the parser's + // handling of a missing value intact. + config.SetInterceptor(new RefuseUndeclaredArguments()); + config.AddCommand("generate") .WithDescription("Generate error documentation from a solution or from assemblies."); @@ -73,7 +86,8 @@ private static void Configure(IConfigurator config) { /// private static int HandleUncaught(Exception exception, ITypeResolver? resolver) { _ = resolver; - bool usage = exception is CommandParseException or CommandTemplateException or CommandConfigurationException; + bool usage = exception is CommandParseException or CommandTemplateException or CommandConfigurationException + or UndeclaredArgumentException; Console.Error.WriteLine($"error: {exception.Message}"); if (usage) { Console.Error.WriteLine("Run 'fce --help' to see the available commands."); } @@ -83,4 +97,36 @@ private static int HandleUncaught(Exception exception, ITypeResolver? resolver) #endregion + /// + /// Refuses a command line carrying an argument no command declares, before the command runs. + /// + private sealed class RefuseUndeclaredArguments : ICommandInterceptor { + + /// + public void Intercept(CommandContext context, CommandSettings settings) { + if (context is null) { throw new ArgumentNullException(nameof(context)); } + + IReadOnlyList undeclared = [.. context.Remaining.Raw, .. context.Remaining.Parsed.Select(pair => pair.Key)]; + if (undeclared.Count == 0) { return; } + + throw new UndeclaredArgumentException(undeclared[0]); + } + + } + +} + +/// +/// Raised when the command line carries an argument the command tree does not declare. It is the tool's own +/// usage refusal rather than the parser's, so it names the offending argument and nothing else. +/// +[SuppressMessage("Minor Code Smell", "S3871:Exception types should be \"public\"", + Justification = + "The rule exists so a caller outside the assembly can catch the exception. This assembly is an " + + "executable: nothing references it, and the only code that catches this is the exit-code handler " + + "a few lines above. Making it public would advertise a type to callers that cannot exist.")] +internal sealed class UndeclaredArgumentException : Exception { + + internal UndeclaredArgumentException(string argument) : base($"Unknown argument '{argument}'.") { } + } diff --git a/doc/handwritten/for-users/CatalogVersioningReference.en.md b/doc/handwritten/for-users/CatalogVersioningReference.en.md index c4e828de..708be14c 100644 --- a/doc/handwritten/for-users/CatalogVersioningReference.en.md +++ b/doc/handwritten/for-users/CatalogVersioningReference.en.md @@ -43,7 +43,7 @@ Exit codes: | --- | --- | | `0` | Baseline created, already current, or replaced successfully. | | `1` | Execution error or baseline schema newer than the tool. | -| `64` | The command line was refused: unknown command, or an option given without its value. | +| `64` | The command line was refused: unknown command, unknown option, or an option given without its value. | | `130` | Execution interrupted. | ## `fce catalog diff` @@ -61,7 +61,7 @@ Exit codes: | `0` | No change reaches the threshold selected by `--fail-on`. | | `2` | At least one change reaches that threshold. | | `1` | Execution error: missing baseline, failed extraction, invalid file, and so on. | -| `64` | The command line was refused: unknown command, or an option given without its value. | +| `64` | The command line was refused: unknown command, unknown option, or an option given without its value. | | `130` | Execution interrupted. | ### Failure policy: `--fail-on` diff --git a/doc/handwritten/for-users/CatalogVersioningReference.fr.md b/doc/handwritten/for-users/CatalogVersioningReference.fr.md index ee4c0615..9bc54901 100644 --- a/doc/handwritten/for-users/CatalogVersioningReference.fr.md +++ b/doc/handwritten/for-users/CatalogVersioningReference.fr.md @@ -43,7 +43,7 @@ Codes de sortie : | --- | --- | | `0` | Baseline créée, déjà à jour ou remplacée avec succès. | | `1` | Erreur d'exécution ou schéma de baseline plus récent que l'outil. | -| `64` | Ligne de commande refusée : commande inconnue, ou option fournie sans sa valeur. | +| `64` | Ligne de commande refusée : commande inconnue, option inconnue, ou option fournie sans sa valeur. | | `130` | Exécution interrompue. | ## `fce catalog diff` @@ -61,7 +61,7 @@ Codes de sortie : | `0` | Aucun changement n'atteint le seuil défini par `--fail-on`. | | `2` | Au moins un changement atteint ce seuil. | | `1` | Erreur d'exécution : baseline manquante, extraction impossible, fichier invalide, etc. | -| `64` | Ligne de commande refusée : commande inconnue, ou option fournie sans sa valeur. | +| `64` | Ligne de commande refusée : commande inconnue, option inconnue, ou option fournie sans sa valeur. | | `130` | Exécution interrompue. | ### Politique d'échec : `--fail-on`