fix: panics on named types, unbounded exponent, ignored "-" tag - #24
Merged
Conversation
Parsing built the value with a hard-coded concrete type and assigned it to
the field, which panics as soon as the field is not exactly that type:
type Level int
type Conf struct{ L []Level } // []int is not assignable to []Level
type Conf struct{ D *time.Duration } // *int64 is not assignable to *time.Duration
Both paths now build the value through the field's own reflect.Type, so
named types work like their underlying kind. This replaces the fifteen
near identical branches of parseSlice and all of parsePointer with a single
setValue helper, and *time.Duration gains the "10s" syntax time.Duration
already had.
Other fixes:
- parseScientific expanded the exponent one byte at a time with no upper
bound: PORT=1e1000000 spent ~100s building a one megabyte string that
cannot fit in any integer type. It is now bounded and rejects the value
right away. "1.5e3" is 1500 instead of the unparsable "1.5000".
- `yaml:"-"`, `json:"-"` and `env:"-"` were honoured by List but ignored by
Parse, which still applied the default and bound the field to a key
literally named "-". They are now skipped by both.
- Parse(config) instead of Parse(&config) returned nil after filling in
nothing at all. It now returns an error.
- A *struct field was never walked into: its defaults and keys were
silently dropped and List emitted a bogus flat key for it. It is now an
optional section, allocated only when one of its fields is set, with a
guard so a self referencing type terminates.
- `default:"false"` on a bool overwrote a value already set by the caller,
unlike every other kind.
- Kinds that cannot be filled from a string (map, array, ...) were ignored
without a word; they are now reported and no longer listed.
- The default whitespace separator collapses repeated separators, so
"a b" is two elements instead of three.
- List quotes defaults with strconv.Quote instead of bare quotes.
- Get and friends take the prefix Parse was called with, work on a
read-only config, and no longer miss named types.
- Parse errors are wrapped with %w.
- The parser type is exported as ECP, so callers can name it.
Tests: TestGetKeyLookupValue replaced globalEcp.LookupValue and never
restored it, leaking the mock into every test that ran after it.
CI runs vet, race and gofmt over a version matrix; the dead Travis config
and badge are gone, and the README example now prints what it claims.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Panics
Parsing built the value with a hard-coded concrete type and assigned it to the field, which panics as soon as the field is not exactly that type:
Both are reachable from an ordinary env value, and
*time.Durationis a natural way to express "unset vs zero".Both paths now build the value through the field's own
reflect.Type, so named types work like their underlying kind. That singlesetValuehelper replaces the fifteen near-identical branches ofparseSliceand all ofparsePointer(parse.go: 291 → 78 lines), and*time.Durationgains the"10s"syntaxtime.Durationalready had.Other fixes
PORT=1e10000001.5e3"1.5000"1500yaml:"-"/json:"-"/env:"-"List, ignored byParse(default applied, field bound to a key named-)Parse(config)without&nilerror, nothing filled*SubStructsectionListdefault:"false"on a booltruemap/ array fields"a b"(default separator)ListquotingA="a "b" c"strconv.QuoteGet%s%w*ecp*ECPTests
TestGetKeyLookupValuereplacedglobalEcp.LookupValueand only restoredBuildKey, leaking the mock into every test that ran after it — new tests were silently reading from it instead of the environment.Regression coverage added for every item above; coverage 91.5% → 94.2%, suite passes under
-race -shuffle=on -count=2.Chores
CI runs
vet,-raceandgofmtover a Go 1.19/1.21/1.23/stable matrix (actions/*@v4/v5). Dead Travis config and badge removed. The README example printed output it did not actually produce (new log level: [ info ]was reallydebug,LOGLEVEL=debugwas reallyECP_LOGLEVEL=debug) — it is corrected and verified by running it.Compatibility
Behaviour changes on purpose, hence a minor bump:
Parsewithout a pointer now errors,--tagged fields are no longer filled, a caller-set bool survivesdefault:"false", and unsupported kinds error instead of being ignored. The exported-type andGetprefix changes are source-compatible.🤖 Generated with Claude Code