Skip to content

ArgParser: enforce CommandOptionDescriptor.ValueType for non-bool command options (e.g. int) #42

Description

@tig

Summary

CommandOptionDescriptor.ValueType is documented / advertised (help, OpenCLI wire types) but command option values are not type-checked at parse time. Non-bool options are always stored as raw strings. Consumers that declare typeof(int) still receive garbage like "2--printer" and must validate themselves — or silently mis-handle the value.

Framework options already validate (--rows rejects non-integers; --timeout rejects bad durations). Command options should do the same for declared types.

Repro (consumer: winprint wp)

print declares --from-sheet / --to-sheet as typeof(int) via CommandOptionDescriptor.

Typo / missing space (one shell token for the value):

wp print file.md --from-sheet 1 --to-sheet 2--printer "Some Printer" --what-if

Actual

  1. ArgParser accepts --to-sheet value = "2--printer" (no type error).
  2. "Some Printer" becomes a positional argument (second "file").
  3. --printer never appears as an option.
  4. App-layer binder does int.TryParse → fails → falls back to 0 (or similar), so the host never surfaces a usage error.

With a clean invalid int alone:

wp print file.md --to-sheet 2--printer --what-if
# host parse succeeds; app may treat range as "all sheets"

Expected

Parse fails at the host with something like:

Invalid value for --to-sheet: '2--printer' (expected int).

exit usage/error, before command dispatch.

Code pointer

ArgParser.AddCommandOptionValue only treats ValueType == typeof(bool) as a flag; otherwise ReadValue stores the next token as a string with no conversion against ValueType:

var isFlag = commandOption!.ValueType == typeof (bool);
// ...
values.Add (name, value); // always string

Compare framework path for --rows, which does int.TryParse and sets error on failure.

ValueType is also used in OpenCLI/help as the advertised type — so the public contract implies typed options, but parse does not enforce it.

Suggested fix

After reading a non-flag command option value, validate (and optionally normalize) against commandOption.ValueType:

ValueType Behavior
typeof(bool) flag (current)
typeof(int) int.TryParse (invariant); fail with clear message on miss
typeof(string) / default keep raw string
other (future) same pattern as needed, or document "string only + bool + int"

Store the canonical string form of the parsed value (e.g. "2") so existing string-keyed CommandOptions stays stable, or document that consumers still receive strings but host guarantees they parse.

Reject missing value (already done via ReadValue).

Optional UX (not required for this issue): if a non-string option's value token starts with - and looks like an option, error "option requires a value" rather than consuming --foo as the value — separate from glued 2--printer which does not start with -.

Tests

  • Command option typeof(int), value 42 → success, options dict has "42" (or equivalent).
  • Value 2--printer / abc / empty → ParseResult.Success == false, error mentions option name and expected type.
  • typeof(bool) flag still works without a following value.
  • typeof(string) still accepts any token including non-numeric / 2--printer.
  • Regression: required int option missing still fails as today.

Why

Without host-side validation, every consumer reimplements binders that either:

  • silently coerce bad values (winprint GetInt0), or
  • fail late with domain errors that look unrelated (e.g. treating the next token as a file path).

Typed descriptors without typed parse is a footgun for every app on this stack.

Context

Discovered while debugging winprint CLI misuse against a Brother printer:

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions