diff --git a/cli/options.go b/cli/options.go index 85b6e3d..ecb3610 100644 --- a/cli/options.go +++ b/cli/options.go @@ -9,15 +9,17 @@ import ( // Options is the resolved configuration: a flat name->value map plus a record // of which names were explicitly set (so callers can tell a default from a -// chosen value). Values are kept as strings end-to-end and converted only at -// the point of use by the typed getters below. +// chosen value). The raw strings carry the layered override merge; finalize() +// then parses each one exactly once into typed, and the numeric/bool getters +// read that — no value is parsed twice. type Options struct { - vals map[string]string - set map[string]bool + vals map[string]string + set map[string]bool + typed map[string]any // parsed value per option, filled once by finalize() } func newOptions() *Options { - return &Options{vals: map[string]string{}, set: map[string]bool{}} + return &Options{vals: map[string]string{}, set: map[string]bool{}, typed: map[string]any{}} } // IsSet reports whether name was given on the command line or in the config @@ -44,30 +46,13 @@ func boolWord(s string) (val, ok bool) { return val, ok } -// Bool reports whether the value is a truthy boolean word. -func (o *Options) Bool(name string) bool { - val, _ := boolWord(o.vals[name]) - return val -} - -// Int returns the value as an int, or 0 if empty/invalid. The value has already -// passed validate(), so a parse failure here is unreachable in normal flow. -func (o *Options) Int(name string) int { - n, _ := strconv.ParseInt(strings.TrimSpace(o.vals[name]), 10, 64) - return int(n) -} - -// Float returns the value as a float64, or 0 if empty/invalid. -func (o *Options) Float(name string) float64 { - f, _ := strconv.ParseFloat(strings.TrimSpace(o.vals[name]), 64) - return f -} - -// Size returns a byte count parsed from a value like "20M" or "512K". -func (o *Options) Size(name string) int64 { - n, _ := parseSize(o.vals[name]) - return n -} +// Bool, Int, Float and Size read the value finalize() already parsed and stored +// in typed; an unset option (no entry) reads as the zero value. The conversion +// happened once, at finalize, so these never re-parse a string. +func (o *Options) Bool(name string) bool { v, _ := o.typed[name].(bool); return v } +func (o *Options) Int(name string) int { v, _ := o.typed[name].(int64); return int(v) } +func (o *Options) Float(name string) float64 { v, _ := o.typed[name].(float64); return v } +func (o *Options) Size(name string) int64 { v, _ := o.typed[name].(int64); return v } // List returns a repeatable option's accumulated values. func (o *Options) List(name string) []string { diff --git a/cli/parse.go b/cli/parse.go index e4e6519..0bccfe3 100644 --- a/cli/parse.go +++ b/cli/parse.go @@ -55,9 +55,34 @@ func Parse(args []string) (*Result, error) { return nil, err } } + if err := opts.finalize(); err != nil { + return nil, err + } return &Result{Action: Run, Options: opts, URIs: uris}, nil } +// finalize parses every resolved value once into its typed form, validating it +// in the same pass; the numeric/bool getters then just read the result. It runs +// after all layering, so each option is converted exactly once, from its final +// (last-wins) string — not once per layer and again per read. +func (o *Options) finalize() error { + for i := range options { + opt := &options[i] + raw, ok := o.vals[opt.Long] + if !ok { + continue + } + v, err := parseValue(opt, raw) + if err != nil { + return err + } + if v != nil { + o.typed[opt.Long] = v + } + } + return nil +} + // parseArgs walks the argument list once. It recognises --long, --long=val, // -x, -xval, -x val and short bool bundles like -cq. Like getopt_long, it // permutes: a non-flag token becomes a URI but scanning continues so flags may @@ -214,16 +239,13 @@ func applyEnv(o *Options) { } } -// set validates a value against its option descriptor and stores it, marking -// the option as explicitly set. +// set stores a raw value, marking the option as explicitly set. Validation and +// conversion happen later, once, in finalize(); set only records the string. func set(o *Options, name, val string) error { opt := byLong[name] if opt == nil { return fmt.Errorf("unknown option %q", name) } - if err := validate(opt, val); err != nil { - return err - } if opt.Kind == List && o.set[name] { val = appendList(o.vals[name], val) } @@ -232,52 +254,58 @@ func set(o *Options, name, val string) error { return nil } -func validate(o *Opt, val string) error { +// parseValue converts a raw string to the typed value for o.Kind, validating it +// in the process: it is the single place a value is parsed. Bool/Int/Size/Float +// return their typed value (read back by the getters); Str/List/Enum need no +// stored conversion and return nil, with Enum still checked for membership. +func parseValue(o *Opt, val string) (any, error) { switch o.Kind { case Bool: - // Booleans accept true/false plus the yes/no/1/0/on/off spellings, but - // nothing else, so e.g. --enable-dht=flase is a parse error. The accepted - // set is boolWords, the same vocabulary the readers honour. - if _, ok := boolWord(val); !ok { - return fmt.Errorf("--%s: %q is not a boolean (true/false)", o.Long, val) + // Booleans accept exactly true/false (the boolWords vocabulary the Bool + // getter honours); --enable-dht=flase is a parse error. + v, ok := boolWord(val) + if !ok { + return nil, fmt.Errorf("--%s: %q is not a boolean (true/false)", o.Long, val) } + return v, nil case Int: n, err := strconv.ParseInt(strings.TrimSpace(val), 10, 64) if err != nil { - return fmt.Errorf("--%s: %q is not an integer", o.Long, val) + return nil, fmt.Errorf("--%s: %q is not an integer", o.Long, val) } if err := checkBounds(o, n); err != nil { - return err + return nil, err } + return n, nil case Size: n, err := parseSize(val) if err != nil { - return fmt.Errorf("--%s: %v", o.Long, err) + return nil, fmt.Errorf("--%s: %v", o.Long, err) } if err := checkBounds(o, n); err != nil { - return err + return nil, err } + return n, nil case Float: - // seed-time and seed-ratio are both rates/durations that must be - // non-negative; a negative SeedTime would fire the seed timer immediately - // and a negative SeedRatio is silently ignored. Reject negatives here. - // Opt.Min/Max are int64, so we only enforce the floor, not full bounds. + // seed-time and seed-ratio must be non-negative: a negative SeedTime would + // fire the seed timer immediately and a negative SeedRatio is meaningless. f, err := strconv.ParseFloat(strings.TrimSpace(val), 64) if err != nil { - return fmt.Errorf("--%s: %q is not a number", o.Long, val) + return nil, fmt.Errorf("--%s: %q is not a number", o.Long, val) } if f < 0 { - return fmt.Errorf("--%s: %v below minimum 0", o.Long, f) + return nil, fmt.Errorf("--%s: %v below minimum 0", o.Long, f) } + return f, nil case Enum: for _, e := range o.Enum { if val == e { - return nil + return nil, nil } } - return fmt.Errorf("--%s: %q not one of %s", o.Long, val, strings.Join(o.Enum, "|")) + return nil, fmt.Errorf("--%s: %q not one of %s", o.Long, val, strings.Join(o.Enum, "|")) } - return nil + return nil, nil } // checkBounds enforces an Int/Size option's numeric range. Min defaults to a diff --git a/cli/parse_test.go b/cli/parse_test.go index c795096..a14a75d 100644 --- a/cli/parse_test.go +++ b/cli/parse_test.go @@ -94,79 +94,79 @@ func TestParseSize(t *testing.T) { } } -func TestValidateRange(t *testing.T) { +func TestParseValueRange(t *testing.T) { x := byLong["max-connection-per-server"] - if err := validate(x, "16"); err != nil { - t.Errorf("validate 16: %v", err) + if _, err := parseValue(x, "16"); err != nil { + t.Errorf("parseValue 16: %v", err) } - if err := validate(x, "99"); err == nil { - t.Errorf("validate 99: want out-of-range error") + if _, err := parseValue(x, "99"); err == nil { + t.Errorf("parseValue 99: want out-of-range error") } a := byLong["file-allocation"] - if err := validate(a, "bogus"); err == nil { - t.Errorf("validate enum bogus: want error") + if _, err := parseValue(a, "bogus"); err == nil { + t.Errorf("parseValue enum bogus: want error") } } -func TestValidateIntFloor(t *testing.T) { - // Min:0 now enforces a floor of 0: negatives are rejected. +func TestParseValueIntFloor(t *testing.T) { + // Min:0 enforces a floor of 0: negatives are rejected. m := byLong["max-tries"] // Min 0, "0 = unlimited" runtime meaning preserved - if err := validate(m, "0"); err != nil { - t.Errorf("validate max-tries 0: %v, want nil (0 stays a valid input)", err) + if _, err := parseValue(m, "0"); err != nil { + t.Errorf("parseValue max-tries 0: %v, want nil (0 stays a valid input)", err) } - if err := validate(m, "-1"); err == nil { - t.Errorf("validate max-tries -1: want below-minimum error") + if _, err := parseValue(m, "-1"); err == nil { + t.Errorf("parseValue max-tries -1: want below-minimum error") } } -func TestValidateFloatNonNegative(t *testing.T) { +func TestParseValueFloatNonNegative(t *testing.T) { // Float options (seed-time, seed-ratio) reject negatives: a negative // SeedTime would fire the seed timer immediately and a negative SeedRatio // is silently ignored, so neither is a meaningful input. for _, name := range []string{"seed-ratio", "seed-time"} { o := byLong[name] - if err := validate(o, "-1"); err == nil { - t.Errorf("validate %s -1: want below-minimum error", name) + if _, err := parseValue(o, "-1"); err == nil { + t.Errorf("parseValue %s -1: want below-minimum error", name) } - if err := validate(o, "0"); err != nil { - t.Errorf("validate %s 0: %v, want nil", name, err) + if _, err := parseValue(o, "0"); err != nil { + t.Errorf("parseValue %s 0: %v, want nil", name, err) } - if err := validate(o, "1.5"); err != nil { - t.Errorf("validate %s 1.5: %v, want nil", name, err) + if _, err := parseValue(o, "1.5"); err != nil { + t.Errorf("parseValue %s 1.5: %v, want nil", name, err) } } } -func TestValidateBool(t *testing.T) { +func TestParseValueBool(t *testing.T) { b := byLong["enable-dht"] // Only literal true/false are accepted (case-sensitive), with surrounding // whitespace trimmed. for _, ok := range []string{"true", "false", " true ", "false\t"} { - if err := validate(b, ok); err != nil { - t.Errorf("validate bool %q: %v", ok, err) + if _, err := parseValue(b, ok); err != nil { + t.Errorf("parseValue bool %q: %v", ok, err) } } - // The invented spellings and any case variant must now fail. + // The invented spellings and any case variant must fail. for _, bad := range []string{"yes", "no", "1", "0", "on", "off", "flase", "True", "TRUE", "tRuE", "False", "FALSE"} { - if err := validate(b, bad); err == nil { - t.Errorf("validate bool %q: want error (only true/false accepted)", bad) + if _, err := parseValue(b, bad); err == nil { + t.Errorf("parseValue bool %q: want error (only true/false accepted)", bad) } } } -func TestValidateSizeBounds(t *testing.T) { +func TestParseValueSizeBounds(t *testing.T) { k := byLong["min-split-size"] // 1M..1024M (no G unit) - if err := validate(k, "20M"); err != nil { - t.Errorf("validate min-split-size 20M: %v", err) + if _, err := parseValue(k, "20M"); err != nil { + t.Errorf("parseValue min-split-size 20M: %v", err) } - if err := validate(k, "512K"); err == nil { - t.Errorf("validate min-split-size 512K: want below-minimum error") + if _, err := parseValue(k, "512K"); err == nil { + t.Errorf("parseValue min-split-size 512K: want below-minimum error") } // 1025M exceeds the 1<<30 cap and still parses, so it tests the bounds path // (where "2G" would have failed earlier as an unknown unit). - if err := validate(k, "1025M"); err == nil { - t.Errorf("validate min-split-size 1025M: want above-maximum error") + if _, err := parseValue(k, "1025M"); err == nil { + t.Errorf("parseValue min-split-size 1025M: want above-maximum error") } } @@ -207,6 +207,9 @@ func TestOptionsGetters(t *testing.T) { o.vals["min-split-size"] = "20M" o.vals["continue"] = "true" o.vals["seed-ratio"] = "1.5" + if err := o.finalize(); err != nil { // parse the raw values once, as Parse does + t.Fatalf("finalize: %v", err) + } if o.Int("split") != 8 { t.Errorf("Int split = %d", o.Int("split")) }