diff --git a/go/README.md b/go/README.md index 2dee4f860..24aa701e5 100644 --- a/go/README.md +++ b/go/README.md @@ -156,6 +156,17 @@ rules drift, so both are run over mise's real spec and compared — the same che Two implementations checked against one oracle beats two checked against each other. +## Typed values + +Binding collects text, deliberately — the grammar decides which token becomes +which flag, not what it means. `argv.Int`, `Uint`, `Float`, `Bool`, `Duration` +and `Each` are where that text becomes a value, each failure carrying the word +that would not convert and the type it was going to. + +Note `Bool` and `EnvTruth` are different widths on purpose: `Bool` takes Go's +spellings for a value somebody typed, `EnvTruth` is the narrower allow-list +usage-lib uses to decide whether a variable sets a value-less flag at all. + ## Errors `argv.Render` turns a failure into what a CLI should print to stderr: @@ -207,9 +218,8 @@ claim is measured at real scale rather than against a fixture with four flags: ## What is missing -- **Typed values.** Binding collects text. Something still has to turn `"8"` into - an `int` and `"1m"` into a `time.Duration`, and report the ones that will not - convert. +- **A typed front door.** The conversions exist; what is missing is generated + code that calls them, so a CLI author gets a struct rather than events. - **Completions.** The Rust side serves these from the parser's own scope rules so that what is offered and what is accepted cannot disagree; the hooks for it (`Collecting`, `PendingArg`, `FlagsInScope`, `CommandStart`) are already here. diff --git a/go/argv/argv.go b/go/argv/argv.go index 0cfe7e97a..5a2907824 100644 --- a/go/argv/argv.go +++ b/go/argv/argv.go @@ -260,6 +260,9 @@ const ( CodeVarTooMany // CodeConflictingFlags means two flags declared to conflict were both given. CodeConflictingFlags + // CodeInvalidValue means a value was given that the target type could not be + // built from. + CodeInvalidValue ) var codeNames = [...]string{ @@ -276,6 +279,7 @@ var codeNames = [...]string{ CodeVarTooFew: "var_too_few", CodeVarTooMany: "var_too_many", CodeConflictingFlags: "conflicting_flags", + CodeInvalidValue: "invalid_value", } // String gives the code the corpus spells it with. @@ -330,6 +334,11 @@ type Error struct { // two var codes. Bound uint32 Got int + // Value is the text that would not convert, and Want the type it was being + // converted to, for CodeInvalidValue. The text is carried because the whole + // point of the error is to show it back. + Value string + Want string // Other is the flag [Name] cannot be given with, for CodeConflictingFlags. // Both are carried because either alone reads as a puzzle: which flag is // unwelcome depends on what else was given. @@ -369,6 +378,12 @@ func (e *Error) Error() string { return "too many occurrences of " + e.Name case CodeConflictingFlags: return e.Name + " cannot be given with " + e.Other + case CodeInvalidValue: + // Through `safe` for the same reason the tokens are: the rejected text came + // off the command line, and this one is likelier than most to hold + // something strange — it exists because the text was not what the type + // expected. + return "invalid value for " + e.Name + ": " + safe(e.Value) } return "parse error" } diff --git a/go/argv/render.go b/go/argv/render.go index 1daf1ac8f..2917e6fa8 100644 --- a/go/argv/render.go +++ b/go/argv/render.go @@ -101,6 +101,15 @@ func explain(err *Error, help HelpTable) string { return "`" + typedAs(err.Spelling, err.Name) + "` accepts at most " + plural(int(err.Bound), "time") + ", given " + itoa(err.Got) + case CodeInvalidValue: + // Through `safe` like the other two that quote what the user typed. This + // one is the likeliest of the three to carry something strange: it exists + // precisely because the text was not what the target type expected. + msg := "`" + err.Name + "` does not accept `" + safe(err.Value) + "`" + if err.Want != "" { + msg += " (expected " + err.Want + ")" + } + return msg case CodeConflictingFlags: other := err.Other if other == "" { diff --git a/go/argv/render_test.go b/go/argv/render_test.go index 8b72d37db..115e01e57 100644 --- a/go/argv/render_test.go +++ b/go/argv/render_test.go @@ -187,6 +187,7 @@ func TestAnErrorValueIsSafeToPrintToo(t *testing.T) { for _, e := range []*Error{ {Code: CodeUnknownFlag, Token: "--x\x1b[31m\r\nerror: forged"}, {Code: CodeUnexpectedArg, Token: "wat\x1b[31m\r\nerror: forged"}, + {Code: CodeInvalidValue, Name: "jobs", Value: "8\x1b[31m\r\nerror: forged"}, } { got := e.Error() for _, forbidden := range []string{"\x1b", "\r", "\n"} { diff --git a/go/argv/value.go b/go/argv/value.go new file mode 100644 index 000000000..2f5cd4be5 --- /dev/null +++ b/go/argv/value.go @@ -0,0 +1,147 @@ +package argv + +import ( + "errors" + "math" + "strconv" + "strings" + "time" +) + +// Turning bound text into the type a field wants. +// +// Binding collects text, deliberately: the grammar decides which token becomes +// which flag or argument, not what it means, so `"8"` stays a string until +// something that knows the target type asks. This is where that asking happens. +// +// The functions are separate rather than one generic `Convert`, because the set +// of types a CLI wants is small and closed, and each one has its own idea of what +// it accepts — `1h30m` is a duration and not a number, `yes` is neither. A +// generated struct calls the one matching its field. +// +// Every failure carries the text that would not convert and the type it was being +// converted to. A message that says only "invalid value" makes the user guess +// which of their words was wrong. +// +// Nothing is trimmed on the way in. `" 8 "` is refused, as `" 8 ".parse::()` +// is on the Rust side — checked rather than assumed. The parser goes out of its +// way to hand over the bytes the operating system gave it, and a converter +// quietly tidying them would mean a quoted argument means one thing in Go and +// another in Rust from the same spec. + +// Int converts a bound value, naming the entry in any failure. +func Int(name, value string) (int64, *Error) { + n, err := strconv.ParseInt(value, 10, 64) + if err != nil { + return 0, invalid(name, value, "a whole number") + } + return n, nil +} + +// Uint is [Int] for a value that may not be negative. +// +// A leading `+` is a sign, not a digit, and Rust takes it: `"+8".parse::()` +// is 8, while `strconv.ParseUint` refuses the string outright. Refusing `+8` here +// would have the same spec accept a value in Rust and reject it in Go — and the +// message would have said it was not a non-negative whole number, about a value +// that is plainly both. +func Uint(name, value string) (uint64, *Error) { + digits := value + if rest, found := strings.CutPrefix(digits, "+"); found { + digits = rest + } + n, err := strconv.ParseUint(digits, 10, 64) + if err != nil { + return 0, invalid(name, value, "a whole number, not negative") + } + return n, nil +} + +// Float converts a bound value to a float. +// +// The two standard libraries disagree at three edges, and each one is settled the +// way Rust settles it — a spec that means one thing compiled through the derive +// and another through this is the failure mode the whole port is written against. +// +// - Go takes digit separators (`1_5`) and hexadecimal floats (`0x1.8p0`); +// `f64::from_str` takes neither, so they are refused before parsing. Neither +// `_` nor `x` appears in any float Rust accepts, which is why this is a check +// on two characters rather than a grammar of its own. +// - A number too large to hold is `inf` in Rust and a range error in Go, which +// hands back the same ±Inf beside it. The value is kept and the error is not. +// - A signed NaN — `+nan`, `-NaN` — parses in Rust and not in Go. The sign +// means nothing on either side. +// +// Everything else agrees: `inf`, `infinity`, a bare `.5` or `5.`, and an +// underflow to zero. +func Float(name, value string) (float64, *Error) { + if strings.ContainsAny(value, "_xX") { + return 0, invalid(name, value, "a number") + } + n, err := strconv.ParseFloat(value, 64) + switch { + case err == nil: + return n, nil + case errors.Is(err, strconv.ErrRange): + // ±Inf for an overflow, zero for an underflow — the value Go returns is + // the one Rust would have. + return n, nil + case signedNaN(value): + return math.NaN(), nil + } + return 0, invalid(name, value, "a number") +} + +// signedNaN reports the one spelling Rust's f64 takes that Go's does not. +func signedNaN(value string) bool { + return len(value) > 1 && (value[0] == '+' || value[0] == '-') && + strings.EqualFold(value[1:], "nan") +} + +// Bool converts a bound value to a bool. +// +// The spellings are Go's own, which are also the ones `strconv.ParseBool` takes: +// `1`, `t`, `T`, `true`, `TRUE`, `True` and their false counterparts. Note this +// is *wider* than [EnvTruth], which an environment variable setting a value-less +// flag goes through — that one is an allow-list matching usage-lib, and the two +// answer different questions: this converts a value somebody typed, that one +// decides whether a variable counts as setting a flag at all. +func Bool(name, value string) (bool, *Error) { + b, err := strconv.ParseBool(value) + if err != nil { + return false, invalid(name, value, "true or false") + } + return b, nil +} + +// Duration converts a bound value to a duration, in Go's notation: `1h30m`, +// `250ms`, `2s`. +func Duration(name, value string) (time.Duration, *Error) { + d, err := time.ParseDuration(value) + if err != nil { + return 0, invalid(name, value, "a duration such as 30s or 1h30m") + } + return d, nil +} + +// Each maps a conversion over the values a variadic or repeatable entry +// collected, stopping at the first that will not convert. +// +// Written out because the alternative is every caller writing the same loop, and +// getting the early return wrong in a way that reports the last failure instead +// of the first. +func Each[T any](name string, values []string, convert func(string, string) (T, *Error)) ([]T, *Error) { + out := make([]T, 0, len(values)) + for _, v := range values { + converted, err := convert(name, v) + if err != nil { + return nil, err + } + out = append(out, converted) + } + return out, nil +} + +func invalid(name, value, want string) *Error { + return &Error{Code: CodeInvalidValue, Name: name, Value: value, Want: want} +} diff --git a/go/argv/value_test.go b/go/argv/value_test.go new file mode 100644 index 000000000..8da4e646a --- /dev/null +++ b/go/argv/value_test.go @@ -0,0 +1,178 @@ +package argv + +import ( + "math" + "strings" + "testing" + "time" +) + +func TestConversions(t *testing.T) { + if n, err := Int("jobs", "8"); err != nil || n != 8 { + t.Errorf("want 8, got %v %v", n, err) + } + if n, err := Uint("jobs", "8"); err != nil || n != 8 { + t.Errorf("want 8, got %v %v", n, err) + } + if f, err := Float("ratio", "1.5"); err != nil || f != 1.5 { + t.Errorf("want 1.5, got %v %v", f, err) + } + if b, err := Bool("force", "true"); err != nil || !b { + t.Errorf("want true, got %v %v", b, err) + } + if d, err := Duration("wait", "1h30m"); err != nil || d != 90*time.Minute { + t.Errorf("want 1h30m, got %v %v", d, err) + } + // And nothing is trimmed: `" 8 "` is a value the user quoted, and the Rust + // sibling refuses it too. Verified against `" 8 ".parse::()`, which is + // false. + if _, err := Int("jobs", " 8 "); err == nil { + t.Error("padded text should be refused, as it is in Rust") + } +} + +// Where Go's conversions and Rust's disagree, the spec wins the same way in both. +// +// Every case here was checked by running both — the standard libraries do not +// agree by default, and the same spec compiled two ways would otherwise accept +// different values. +func TestTheConvertersAgreeWithRustAtTheEdges(t *testing.T) { + // A leading `+` is a sign, and Rust's u64 takes one. Go's ParseUint refuses + // the string outright. + if n, err := Uint("jobs", "+8"); err != nil || n != 8 { + t.Errorf("`+8` is 8 in Rust, got %v %v", n, err) + } + if _, err := Uint("jobs", "++8"); err == nil { + t.Error("one sign, not two") + } + if _, err := Uint("jobs", "+"); err == nil { + t.Error("a sign with no digits is not a number") + } + + // Go's ParseFloat takes digit separators and hexadecimal floats. Rust's f64 + // takes neither. + for _, v := range []string{"1_5", "0x1.8p0", "0x10"} { + if _, err := Float("ratio", v); err == nil { + t.Errorf("%q parses in Go and not in Rust, so it is refused here", v) + } + } + // The spellings both do take, so the check above is two characters rather + // than a grammar of its own. + for _, v := range []string{"1e3", ".5", "5.", "inf", "-inf", "NaN", "infinity"} { + if _, err := Float("ratio", v); err != nil { + t.Errorf("%q parses in Rust, so it should here: %v", v, err) + } + } + + // Too large to hold is `inf` in Rust; Go returns the same value beside a range + // error, so the value is kept and the error is not. + if f, err := Float("ratio", "1e1000"); err != nil || !math.IsInf(f, 1) { + t.Errorf("an overflow is +Inf in Rust, got %v %v", f, err) + } + if f, err := Float("ratio", "-1e1000"); err != nil || !math.IsInf(f, -1) { + t.Errorf("an overflow is -Inf in Rust, got %v %v", f, err) + } + // And an underflow is zero on both sides, without a complaint on either. + if f, err := Float("ratio", "1e-1000"); err != nil || f != 0 { + t.Errorf("an underflow is 0, got %v %v", f, err) + } + + // A signed NaN parses in Rust and not in Go. The sign means nothing either + // way. + for _, v := range []string{"+nan", "-nan", "+NaN"} { + if f, err := Float("ratio", v); err != nil || !math.IsNaN(f) { + t.Errorf("%q is NaN in Rust, got %v %v", v, f, err) + } + } + // Not a licence for anything else wearing a sign. + for _, v := range []string{"+n", "-nano", "+"} { + if _, err := Float("ratio", v); err == nil { + t.Errorf("%q is not a number in either language", v) + } + } +} + +// A failure carries the text that would not convert and the type it was going +// to: a message saying only "invalid value" makes the user guess which of their +// words was wrong. +func TestAFailureNamesTheValueAndTheType(t *testing.T) { + cases := []struct { + what func() *Error + want string + }{ + {func() *Error { _, e := Int("jobs", "lots"); return e }, "whole number"}, + {func() *Error { _, e := Uint("jobs", "-1"); return e }, "not negative"}, + {func() *Error { _, e := Float("ratio", "half"); return e }, "a number"}, + {func() *Error { _, e := Bool("force", "yes"); return e }, "true or false"}, + {func() *Error { _, e := Duration("wait", "soon"); return e }, "duration"}, + } + for _, c := range cases { + err := c.what() + if err == nil { + t.Fatalf("want a failure for %q", c.want) + } + if err.Code != CodeInvalidValue { + t.Errorf("want invalid_value, got %q", err.Code) + } + if err.Value == "" || err.Name == "" { + t.Errorf("the failure should name both the entry and the value: %+v", err) + } + if !strings.Contains(err.Want, strings.Fields(c.want)[0]) { + t.Errorf("want %q described, got %q", c.want, err.Want) + } + // And it renders as something a person can act on. + if msg := explain(err, nil); !strings.Contains(msg, err.Value) { + t.Errorf("the rendered message should show the value: %q", msg) + } + } +} + +// `yes` is a bool to some CLIs and not to Go. The two truthiness rules in this +// package answer different questions and are deliberately different widths. +func TestBoolIsWiderThanEnvTruth(t *testing.T) { + if _, err := Bool("force", "yes"); err == nil { + t.Error("Go's spellings do not include `yes`") + } + if b, err := Bool("force", "T"); err != nil || !b { + t.Errorf("Go's spellings do include `T`: %v %v", b, err) + } + // EnvTruth is an allow-list matching usage-lib, and narrower. + if EnvTruth("T") { + t.Error("EnvTruth takes only 1, true, True and TRUE") + } +} + +func TestEachStopsAtTheFirstFailure(t *testing.T) { + got, err := Each("jobs", []string{"1", "2", "3"}, Int) + if err != nil || len(got) != 3 || got[2] != 3 { + t.Errorf("want [1 2 3], got %v %v", got, err) + } + _, err = Each("jobs", []string{"1", "two", "three"}, Int) + if err == nil { + t.Fatal("want a failure") + } + // The first, not the last: reporting `three` would send the user to the wrong + // word. + if err.Value != "two" { + t.Errorf("want the first failure, got %q", err.Value) + } +} + +// The rejected value is quoted back, so it goes through the same escaping as the +// other messages that echo what the user typed — and it is the likeliest of them +// to carry something strange, since it exists because the text was unexpected. +func TestARejectedValueIsEscapedBeforeItIsShown(t *testing.T) { + _, err := Int("jobs", "\x1b[31m8\r\nerror: forged") + if err == nil { + t.Fatal("want a failure") + } + msg := explain(err, nil) + for _, forbidden := range []string{"\x1b", "\r", "\n"} { + if strings.Contains(msg, forbidden) { + t.Errorf("a control character survived into %q", msg) + } + } + if !strings.Contains(msg, `\x1b`) || !strings.Contains(msg, "forged") { + t.Errorf("the value should still be legible: %q", msg) + } +}