From 16e467db32d95e12471dc3d97f45291670f68c25 Mon Sep 17 00:00:00 2001 From: Ryan Tinianov Date: Wed, 30 Sep 2026 14:53:07 -0400 Subject: [PATCH 1/2] Add lists and maps as types the CLI and environment variables can hold --- x/config/cli/README.md | 12 +- x/config/cli/binder.go | 10 +- x/config/cli/binder_test.go | 299 +++++++++++++++++++++++-- x/config/cli/decode.go | 244 +++++++++++++++++++- x/config/cli/entry.go | 21 +- x/config/cli/examples/simple/README.md | 4 +- x/config/cli/list.go | 88 ++++++++ x/config/cli/maps.go | 85 +++++++ x/config/cli/register.go | 43 +++- x/config/cli/walk.go | 5 +- 10 files changed, 769 insertions(+), 42 deletions(-) create mode 100644 x/config/cli/list.go create mode 100644 x/config/cli/maps.go diff --git a/x/config/cli/README.md b/x/config/cli/README.md index 7f14095317..7f8dc50a0f 100644 --- a/x/config/cli/README.md +++ b/x/config/cli/README.md @@ -62,7 +62,8 @@ help text; `require.Empty(t, b.Undocumented())` in a test catches that. Help also names each flag's env vars, and marks `required` fields `(required)` unless they sit in an optional (pointer) section. A default is shown as its type's `MarshalText` renders it, so -hold a secret in `pkg/config.SecretString` or `SecretURL` to show it redacted. +hold a secret in `pkg/config.SecretString` or `SecretURL` to show it redacted. A type with +`UnmarshalText` but no `MarshalText` shows its default as ``. ## What gets a flag @@ -74,10 +75,9 @@ hold a secret in `pkg/config.SecretString` or `SecretURL` to show it redacted. | list of those | `--tags a,b` or `--tags a --tags b` | | map of those | `--labels env=prod,region=us` | -Map keys parse like values, so `map[uint32]string` binds as `--chains 1=mainnet`. In a config file, -where keys are text, a key type the markup reads whole is used as it is, and any other is parsed as -its flag would be; two texts that parse to one key, such as `16` and `0x10`, are an error. Anything more -structured is config file only. +Map keys parse like values, so `map[uint32]string` binds as `--chains 1=mainnet`. A config file's +keys are parsed as their flag would be, too; two texts that parse to one key, such as `16` and `0x10`, +are an error. Anything more structured is config file only. Lists and maps are CSV; quote an element or whole entry containing a comma: @@ -86,6 +86,8 @@ Lists and maps are CSV; quote an element or whole entry containing a comma: --labels '"env=a,b",region=us' env=a,b and region=us ``` +A map key can't contain `=`, since an entry splits at its first `=`; set such keys in a config file. + ## Types pflag parses every flag, including each list element and map value, so a bad value fails as the diff --git a/x/config/cli/binder.go b/x/config/cli/binder.go index 1efa18298f..7426bb1d82 100644 --- a/x/config/cli/binder.go +++ b/x/config/cli/binder.go @@ -10,7 +10,6 @@ import ( "github.com/spf13/cobra" - "github.com/smartcontractkit/chainlink-common/x/config/commentparsing" "github.com/smartcontractkit/chainlink-common/x/config/markup" ) @@ -49,8 +48,8 @@ func New(opts Options) (*Binder, error) { // Register attaches target to cmd. Before cmd or a subcommand runs, target is filled from flags, env vars, and config // files, then its `validate` tags are checked. // -// Scalars, durations, []byte, and [encoding.TextUnmarshaler] types get a persistent flag and env vars; other fields -// are config file only. +// Scalars, durations, []byte, [encoding.TextUnmarshaler] types, and lists and maps of those get a persistent flag and +// env vars; other fields are config file only. // // A command runs with its ancestors' structs too, so they must not share a key, flag, or env var; siblings may. A clash // fails every command in the tree when it is executed. @@ -140,8 +139,7 @@ func (b *Binder) commandConfig(c *cobra.Command) (commandConfig, error) { claimed := map[string]string{} for _, e := range cc.entries { for _, k := range e.keys() { - leaf := commentparsing.DerefType(k.goType) - if err := cc.keys.add(k.fileKey, leaf, lang); err != nil { + if err := cc.keys.add(k.fileKey, k.fileType, lang); err != nil { return commandConfig{}, fmt.Errorf("%s: %w", e.command().Name(), err) } @@ -271,7 +269,7 @@ func (b *Binder) loadConfigFiles(cmd *cobra.Command, keys *keyNode) (reflect.Val return fmt.Errorf("invalid %s: %w", name, err) } - keys.overlay(merged, v.Elem()) + keys.overlay(merged, v.Elem(), lang) return nil } diff --git a/x/config/cli/binder_test.go b/x/config/cli/binder_test.go index 001e92f2b4..417979629d 100644 --- a/x/config/cli/binder_test.go +++ b/x/config/cli/binder_test.go @@ -1,6 +1,7 @@ package cli import ( + "encoding" "os" "path/filepath" "strconv" @@ -361,6 +362,10 @@ func decodesTo[T any](t *testing.T, want T, args ...string) { assert.Equal(t, want, got) } +type opaqueWithPointerKeys struct{ byAddress map[*int]string } + +func (o *opaqueWithPointerKeys) UnmarshalText([]byte) error { o.byAddress = nil; return nil } + func TestWhatGetsAFlag(t *testing.T) { type nestsItself struct { Value string @@ -369,11 +374,18 @@ func TestWhatGetsAFlag(t *testing.T) { type hasEveryShape struct { String string Bytes []byte + ListOfStrings []string + ListOfPtrs []*int + MapOfPtrs map[string]*int ListOfMaps []map[string]string ListOfStructs []nestsItself MapOfStructs map[string]nestsItself MapOfBytes map[string][]byte TextReader readsItselfAsText + Opaque opaqueWithPointerKeys + Interface encoding.TextUnmarshaler + Interfaces []encoding.TextUnmarshaler + InterfaceMap map[string]encoding.TextUnmarshaler TextWriter writesItselfAsText Nested nestsItself Complex complex128 @@ -381,12 +393,12 @@ func TestWhatGetsAFlag(t *testing.T) { } flags := flagsOf(t, &hasEveryShape{}, testOptions) - for _, name := range []string{"string", "bytes", "text-reader", "text-writer.value", "nested.value"} { + for _, name := range []string{"string", "bytes", "list-of-strings", "map-of-bytes", "text-reader", "opaque", "text-writer.value", "nested.value"} { assert.NotNil(t, flags.Lookup(name), name) } // Without a single text form a flag's default could not be read back, so these are file-only. - for _, name := range []string{"map-of-bytes", "list-of-maps", "list-of-structs", "list-of-structs.value", "map-of-structs", + for _, name := range []string{"interface", "interfaces", "interface-map", "list-of-ptrs", "map-of-ptrs", "list-of-maps", "list-of-structs", "list-of-structs.value", "map-of-structs", "complex", "unexported", "text-reader.value", "nested.child.value"} { assert.Nil(t, flags.Lookup(name), name) } @@ -398,6 +410,31 @@ func TestRegistrationRejects(t *testing.T) { type embedsUnexportedPointer struct { *unexportedBasicConfig } + type hasPointerKeys struct { + Keys map[*int]string + } + type nestsAPointerKey struct { + Lists []map[string]hasPointerKeys + } + type hasAPointerInsideKeys struct { + Keys map[struct{ P [1]*int }]string + } + type hasAnInterfaceInsideKeys struct { + Keys map[struct{ V any }]string + } + type embedsAndHoldsIDs struct { + BasicConfig + IDs map[uint32]string + } + type holdsARecursiveUintMap struct { + Tree recursiveUintMap + } + type nestsAnEmbedder struct { + Inner embedsAndHoldsIDs + } + type listsEmbeddersOfIDs struct { + List []nestsAnEmbedder + } t.Run("nil pointer", func(t *testing.T) { require.ErrorContains(t, newBinder(t, testOptions).Register(newRoot(t), nilPointer), "target pointer cannot be nil") @@ -409,6 +446,26 @@ func TestRegistrationRejects(t *testing.T) { t.Run("an unexported embedded pointer", func(t *testing.T) { require.ErrorContains(t, newBinder(t, testOptions).Register(newRoot(t), &embedsUnexportedPointer{}), "embedded *unexportedBasicConfig is unexported") }) + t.Run("a pointer map key", func(t *testing.T) { + require.ErrorContains(t, newBinder(t, testOptions).Register(newRoot(t), &hasPointerKeys{}), "cli.hasPointerKeys: Keys: map[*int]string has keys that hold a pointer, channel or interface") + }) + t.Run("a pointer map key nested in a list, map and struct", func(t *testing.T) { + require.ErrorContains(t, newBinder(t, testOptions).Register(newRoot(t), &nestsAPointerKey{}), "cli.nestsAPointerKey: Lists: map[*int]string has keys that hold a pointer, channel or interface") + }) + t.Run("a pointer inside a struct or array key", func(t *testing.T) { + require.ErrorContains(t, newBinder(t, testOptions).Register(newRoot(t), &hasAPointerInsideKeys{}), "cli.hasAPointerInsideKeys: Keys: map[struct { P [1]*int }]string has keys that hold a pointer, channel or interface") + }) + t.Run("an interface inside a key", func(t *testing.T) { + require.ErrorContains(t, newBinder(t, testOptions).Register(newRoot(t), &hasAnInterfaceInsideKeys{}), "cli.hasAnInterfaceInsideKeys: Keys: map[struct { V interface {} }]string has keys that hold a pointer, channel or interface") + }) + t.Run("a recursive type with non-string keys", func(t *testing.T) { + require.ErrorContains(t, newBinder(t, testOptions).Register(newRoot(t), &holdsARecursiveUintMap{}), + "cli.holdsARecursiveUintMap: Tree: cli.recursiveUintMap holds itself and a map with non-string keys") + }) + t.Run("a struct nested in a list that embeds a field and holds non-string keys", func(t *testing.T) { + require.ErrorContains(t, newBinder(t, testOptions).Register(newRoot(t), &listsEmbeddersOfIDs{}), + "cli.listsEmbeddersOfIDs: List: cli.embedsAndHoldsIDs holds a map with non-string keys and embeds a field") + }) _, err := New(Options{}) require.ErrorIs(t, err, markup.Err) @@ -566,8 +623,8 @@ func TestValidationErrorsInsideAListOrMapUseConfigKeys(t *testing.T) { }{ {"a list element's field, by its tag", []string{"--config", writeConfig(t, "[[Structs]]\nrenamed = 'a'\n[[Structs]]\nrenamed = ''\n")}, "Structs.1.renamed failed on the 'required' tag"}, - {"a list element", []string{"--config", writeConfig(t, "Strings = ['a', '']\n")}, "Strings.1 failed on the 'required' tag"}, - {"a map entry", []string{"--config", writeConfig(t, "[Map]\nk = ''\n")}, "Map.k failed on the 'required' tag"}, + {"a list element", []string{"--strings", "a,"}, "Strings.1 failed on the 'required' tag"}, + {"a map entry", []string{"--map", "k="}, "Map.k failed on the 'required' tag"}, {"a rule's sibling in the same element", []string{"--config", writeConfig(t, "[[Rules]]\n")}, "Rules.0.Rule failed on the 'required_without=Rules.0.renamed' tag"}, {"a flattened embed adds no segment", []string{"--config", writeConfig(t, "[[Embeds]]\n")}, @@ -582,6 +639,25 @@ func TestValidationErrorsInsideAListOrMapUseConfigKeys(t *testing.T) { } } +func TestRepeatedListFlagAppends(t *testing.T) { + type hasStrings struct{ Strings []string } + + var c hasStrings + require.NoError(t, run(t, &c, testOptions, "--strings", "a", "--strings", "b")) + assert.Equal(t, []string{"a", "b"}, c.Strings) +} + +func TestListText(t *testing.T) { + type hasStrings struct{ Strings []string } + + c := hasStrings{Strings: []string{"default"}} + require.NoError(t, run(t, &c, testOptions, "--strings", "")) + assert.Empty(t, c.Strings) + + require.ErrorContains(t, run(t, &hasStrings{}, testOptions, "--strings", `"a`), `extraneous or missing " in quoted-field`) + require.ErrorContains(t, run(t, &hasStrings{}, testOptions, "--strings", "a\nb"), `"a\nb" must be one line`) +} + // A byte slice is text (JSON, PEM), so neither a flag nor an env var splits it on its commas. func TestByteSliceIsText(t *testing.T) { type hasByteSlice struct { @@ -625,16 +701,197 @@ func (p *selfSplittingList) UnmarshalText(b []byte) error { return nil } -func TestHelpDefaultsCanBePassedBackIn(t *testing.T) { - type hasText struct { - Text *config.URL +type hasStringMap struct { + Map map[string]string +} + +func TestStringMapFromEverySource(t *testing.T) { + cases := everySource([]string{"--map", "a=1,b=2"}, "a=1,b=2", "[Map]\na = '1'\nb = '2'") + cases = append(cases, sourceCase{name: "repeated flag", args: []string{"--map", "a=1", "--map", "b=2"}}) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var c hasStringMap + require.NoError(t, run(t, &c, testOptions, supplyConfig(t, "TEST_MAP", tc.env, tc.file, tc.args...)...)) + assert.Equal(t, map[string]string{"a": "1", "b": "2"}, c.Map) + }) } +} - c := hasText{Text: config.MustParseURL("https://x/rpc")} +func TestMapsMergeAcrossSources(t *testing.T) { + t.Setenv("TEST_MAP", "e=env,a=env") + c := hasStringMap{Map: map[string]string{"d": "default", "a": "default"}} + require.NoError(t, run(t, &c, testOptions, + "--config", writeConfig(t, "[Map]\na = 'first'\nf = 'first'\n"), + "--config", writeConfig(t, "[Map]\na = 'second'\ns = 'second'\n"), + "--map", "a=flag")) + assert.Equal(t, map[string]string{"d": "default", "f": "first", "s": "second", "e": "env", "a": "flag"}, c.Map) +} + +func TestMapEntryText(t *testing.T) { + var c hasStringMap + require.NoError(t, run(t, &c, testOptions, "--map", `"a=1,b=2",c=3`)) + assert.Equal(t, map[string]string{"a": "1,b=2", "c": "3"}, c.Map) + + var blank hasStringMap + require.NoError(t, run(t, &blank, testOptions, "--map", "a=1,,b=2")) + assert.Equal(t, map[string]string{"a": "1", "b": "2"}, blank.Map) + + require.ErrorContains(t, run(t, &hasStringMap{}, testOptions, "--map", `"a=1`), `extraneous or missing " in quoted-field`) + + t.Setenv("TEST_MAP", "a") + require.ErrorContains(t, run(t, &hasStringMap{}, testOptions), `invalid value "a" for TEST_MAP: "a" must be formatted as key=value`) +} + +type caseless string + +func (c *caseless) UnmarshalText(b []byte) error { + *c = caseless(strings.ToLower(string(b))) + return nil +} + +func TestMapKeysNeedNotBeStrings(t *testing.T) { + type hasNonStringKeys struct { + UintKeys map[uint32]string + TextKeys map[config.Duration]string + } + want := hasNonStringKeys{ + UintKeys: map[uint32]string{1: "a", 42: "b"}, + TextKeys: map[config.Duration]string{*config.MustNewDuration(45 * time.Second): "c"}, + } + + flag := []string{"--uint-keys", "1=a,0x2a=b", "--text-keys", "45s=c"} + for _, tc := range everySource(flag, "", "[UintKeys]\n1 = 'a'\n0x2a = 'b'\n[TextKeys]\n45s = 'c'\n") { + t.Run(tc.name, func(t *testing.T) { + var c hasNonStringKeys + require.NoError(t, run(t, &c, testOptions, supplyConfig(t, "", "", tc.file, tc.args...)...)) + assert.Equal(t, want, c) + }) + } + + for _, args := range []string{"16=a,0x10=b", "16=a --uint-keys 0x10=b"} { + require.ErrorContains(t, run(t, &hasNonStringKeys{}, testOptions, append([]string{"--uint-keys"}, strings.Fields(args)...)...), + `keys "16" and "0x10" are both 16`, args) + } + + require.ErrorContains(t, run(t, &hasNonStringKeys{}, testOptions, "--config", writeConfig(t, "[UintKeys]\n16 = 'a'\n0x10 = 'b'\n")), + `keys "0x10" and "16" are both 16`) + require.ErrorContains(t, run(t, &hasNonStringKeys{}, testOptions, "--config", writeConfig(t, "[TextKeys]\n45s = 'a'\n0m45s = 'b'\n")), + `keys "0m45s" and "45s" are both 45s`) + require.ErrorContains(t, run(t, &struct{ FloatKeys map[float64]string }{}, testOptions, "--float-keys", "NaN=a"), + `key "NaN" never equals itself`) + require.ErrorContains(t, run(t, &struct{ Caseless map[caseless]string }{}, testOptions, "--config", writeConfig(t, "[Caseless]\na = '1'\nA = '2'\n")), + `keys "A" and "a" are both a`) + require.ErrorContains(t, run(t, &hasNonStringKeys{}, testOptions, "--config", writeConfig(t, "[UintKeys]\nx = 'a'\n")), + `uint-keys: key "x": strconv.ParseUint: parsing "x": invalid syntax`) +} + +func TestMapKeysNestedInAConfigFileOnlyField(t *testing.T) { + type holdsIDs struct { + IDs map[uint32]string + IgnoredPtrs map[*int]string `toml:"-"` + ignore string //nolint:unused // the markup never sets it, so the rebuilt struct leaves it out + } + type hasNestedNonStringKeys struct { + Lists []map[uint32]string + Maps map[string]map[uint32]string + Pointers []*map[uint32]string + Arrays [1]map[uint32]string + Structs []holdsIDs + } + + var c hasNestedNonStringKeys + file := "Lists = [{1 = 'a'}]\nPointers = [{3 = 'c'}]\nArrays = [{4 = 'd'}]\n[Maps.m]\n2 = 'b'\n[[Structs]]\n[Structs.IDs]\n0x10 = 'e'\n" + require.NoError(t, run(t, &c, testOptions, "--config", writeConfig(t, file))) + assert.Equal(t, hasNestedNonStringKeys{ + Lists: []map[uint32]string{{1: "a"}}, + Maps: map[string]map[uint32]string{"m": {2: "b"}}, + Pointers: []*map[uint32]string{{3: "c"}}, + Arrays: [1]map[uint32]string{{4: "d"}}, + Structs: []holdsIDs{{IDs: map[uint32]string{16: "e"}}}, + }, c) + + for file, want := range map[string]string{ + "Lists = [{x = 'a'}]\n": `lists: key "x": strconv.ParseUint`, + "[Maps.m]\nx = 'b'\n": `maps: key "x": strconv.ParseUint`, + "Pointers = [{x = 'c'}]\n": `pointers: key "x": strconv.ParseUint`, + "Arrays = [{x = 'd'}]\n": `arrays: key "x": strconv.ParseUint`, + "[[Structs]]\n[Structs.IDs]\nx = 'e'\n": `structs: key "x": strconv.ParseUint`, + } { + require.ErrorContains(t, run(t, &hasNestedNonStringKeys{}, testOptions, "--config", writeConfig(t, file)), want, file) + } +} + +type nodeName string + +type recursiveMap map[nodeName]*recursiveMap + +type wholeMap map[string]string + +func (m *wholeMap) UnmarshalText(b []byte) error { *m = wholeMap{"text": string(b)}; return nil } + +func TestAMapReadWholeIsReplacedNotMerged(t *testing.T) { + type hasWholeMap struct { + Whole wholeMap + } + + c := hasWholeMap{Whole: wholeMap{"default": ""}} + require.NoError(t, run(t, &c, testOptions, "--config", writeConfig(t, "Whole = 'a'\n"), "--config", writeConfig(t, "Whole = 'b'\n"))) + assert.Equal(t, wholeMap{"text": "b"}, c.Whole) + + require.NoError(t, run(t, &c, testOptions, "--whole", "c")) + assert.Equal(t, wholeMap{"text": "c"}, c.Whole) +} + +type recursiveUintMap map[uint32]*recursiveUintMap + +func TestRecursiveConfigFileOnlyType(t *testing.T) { + type hasRecursiveMap struct { + Tree recursiveMap + } + + var c hasRecursiveMap + require.NoError(t, run(t, &c, testOptions, "--config", writeConfig(t, "[Tree.a.b]\n"))) + assert.Equal(t, hasRecursiveMap{Tree: recursiveMap{"a": {"b": new(recursiveMap)}}}, c) +} + +type namedIntWithAString int + +func (namedIntWithAString) String() string { return "name" } + +func TestHelpDefaultsCanBePassedBackIn(t *testing.T) { + type hasListMapAndText struct { + Named namedIntWithAString + NamedList []namedIntWithAString + Wait time.Duration + Ratio float32 + List []string + LoneEmpty []string + Map map[string]string + Text *config.URL + } + + c := hasListMapAndText{Named: 2, NamedList: []namedIntWithAString{3}, Wait: 90 * time.Second, Ratio: 0.1, List: []string{"a", "b,c", "d "}, LoneEmpty: []string{""}, Map: map[string]string{"a": "1", "b": "2,3", "c": " 4"}, Text: config.MustParseURL("https://x/rpc")} flags := flagsOf(t, &c, testOptions) + assert.Equal(t, "value,...", flags.Lookup("list").Value.Type()) + assert.Equal(t, "key=value,...", flags.Lookup("map").Value.Type()) + + type hasTextWithoutAWriter struct { + Reader readsItselfAsText + Readers []readsItselfAsText + ByName map[string]readsItselfAsText + } - var back hasText - args := []string{"--text", flags.Lookup("text").DefValue} + reader := readsItselfAsText{Value: "x"} + readers := flagsOf(t, &hasTextWithoutAWriter{reader, []readsItselfAsText{reader}, map[string]readsItselfAsText{"a": reader}}, testOptions) + for name, want := range map[string]string{"reader": cannotDisplay, "readers": cannotDisplay, "by-name": "a=" + cannotDisplay} { + assert.Equal(t, want, readers.Lookup(name).DefValue, name) + } + + var back hasListMapAndText + args := []string{"--named", flags.Lookup("named").DefValue, "--named-list", flags.Lookup("named-list").DefValue, + "--wait", flags.Lookup("wait").DefValue, "--ratio", flags.Lookup("ratio").DefValue, + "--list", flags.Lookup("list").DefValue, "--lone-empty", flags.Lookup("lone-empty").DefValue, "--map", flags.Lookup("map").DefValue, + "--text", flags.Lookup("text").DefValue} require.NoError(t, run(t, &back, testOptions, args...)) assert.Equal(t, c, back) } @@ -663,18 +920,25 @@ func TestTextUnmarshalerLeafFromEverySource(t *testing.T) { require.ErrorContains(t, run(t, &hasIntKindText{}, testOptions, "--value", "not-a-duration"), `time: invalid duration "not-a-duration"`) } -func TestLeafNestedInASection(t *testing.T) { +func TestLeafNestedInASectionListAndMap(t *testing.T) { type hasDuration struct { Value config.Duration } - type hasDurationInASection struct { + type hasDurationEverywhere struct { Section hasDuration + List []config.Duration + Map map[string]config.Duration + } + want := hasDurationEverywhere{ + Section: hasDuration{*config.MustNewDuration(time.Minute)}, + List: []config.Duration{*config.MustNewDuration(time.Second), *config.MustNewDuration(2 * time.Second)}, + Map: map[string]config.Duration{"a": *config.MustNewDuration(3 * time.Second)}, } - want := hasDurationInASection{Section: hasDuration{*config.MustNewDuration(time.Minute)}} - for _, tc := range everySource([]string{"--section.value", "1m"}, "", "[Section]\nValue = '1m'") { + flag := []string{"--section.value", "1m", "--list", "1s,2s", "--map", "a=3s"} + for _, tc := range everySource(flag, "", "List = ['1s', '2s']\n[Section]\nValue = '1m'\n[Map]\na = '3s'") { t.Run(tc.name, func(t *testing.T) { - var c hasDurationInASection + var c hasDurationEverywhere require.NoError(t, run(t, &c, testOptions, supplyConfig(t, "", "", tc.file, tc.args...)...)) assert.Equal(t, want, c) }) @@ -732,12 +996,17 @@ type hasEveryKind struct { Bool bool String string Duration time.Duration + IntList []int + IntMap map[string]int } func TestParserErrorsArePropagated(t *testing.T) { err := run(t, &hasEveryKind{}, testOptions, "--int=abc") require.ErrorContains(t, err, `invalid argument "abc" for "--int" flag: strconv.ParseInt: parsing "abc": invalid syntax`) + require.ErrorContains(t, run(t, &hasEveryKind{}, testOptions, "--int-list=1,x"), `element "x": strconv.ParseInt: parsing "x"`) + require.ErrorContains(t, run(t, &hasEveryKind{}, testOptions, "--int-map=a=x"), `value of "a": strconv.ParseInt: parsing "x"`) + t.Setenv("TEST_INT", "abc") require.ErrorContains(t, run(t, &hasEveryKind{}, testOptions), `invalid value "abc" for TEST_INT: strconv.ParseInt: parsing "abc": invalid syntax`) } diff --git a/x/config/cli/decode.go b/x/config/cli/decode.go index dad8d964b8..5b3cb26c60 100644 --- a/x/config/cli/decode.go +++ b/x/config/cli/decode.go @@ -1,6 +1,7 @@ package cli import ( + "cmp" "fmt" "maps" "reflect" @@ -74,7 +75,7 @@ func (n *keyNode) fileValuesType(lang markup.Markup) reflect.Type { } // A list replaces rather than merges, or a later file could never shorten it. -func (n *keyNode) overlay(dst, src reflect.Value) { +func (n *keyNode) overlay(dst, src reflect.Value, lang markup.Markup) { for _, child := range n.children { s, d := src.Field(child.index), dst.Field(child.index) switch { @@ -82,7 +83,9 @@ func (n *keyNode) overlay(dst, src reflect.Value) { case d.IsNil(): d.Set(s) case child.leaf == nil: - child.overlay(d.Elem(), s.Elem()) + child.overlay(d.Elem(), s.Elem(), lang) + case mergesByKey(child.leaf, lang): + d.Elem().Set(mergeMaps(child.leaf, d.Elem(), s.Elem())) default: d.Set(s) } @@ -102,3 +105,240 @@ func (n *keyNode) lookup(v reflect.Value, path []string) (reflect.Value, bool) { return v, true } + +var stringType = reflect.TypeFor[string]() + +// Config file map keys are text, so every key is read as a string and parsed as its flag would be (see fromFile), even +// one the markup could read itself, so two texts for one key are always caught. +// +// A recursive type that needs converting, such as type M map[uint32]*M, is an error: the inner M can't be converted. +// visiting holds the types being converted, false until one is reached again inside itself. +func fileValueType(t reflect.Type, lang markup.Markup, visiting map[reflect.Type]bool) (reflect.Type, error) { + if _, ok := visiting[t]; ok { + visiting[t] = true + // Recursion itself is not an error, for example, nothing should stop the type below from being used. + // type Endpoint struct { + // URL string + // Fallback *Endpoint + // } + // We only prevent recursive definitions if we need to modify the type + // type Endpoint struct { + // URLs map[int32]string + // Fallback *Endpoint + // } + // because Go's reflection can't redefine it for the string parsing of the key as + // struct { + // URLs map[string]string + // Fallback * + // } + return t, nil + } + + if lang.IsLeaf(t) { + return t, nil + } + + visiting[t] = false + defer delete(visiting, t) + out, err := convertFileValueType(t, lang, visiting) + if err != nil { + return nil, err + } + + if visiting[t] && out != t { + return nil, fmt.Errorf("%s holds itself and a map with non-string keys, which a config file can't set; "+ + "key the map by string", t) + } + + return out, nil +} + +func convertFileValueType(t reflect.Type, lang markup.Markup, visiting map[reflect.Type]bool) (reflect.Type, error) { + var wrap func(reflect.Type) reflect.Type + switch t.Kind() { + case reflect.Map: + if holdsIdentity(t.Key()) { + return nil, fmt.Errorf("%s has keys that hold a pointer, channel or interface, which can compare by address, "+ + "so 2 and 2 could be different keys; key it by value", t) + } + + wrap = func(elem reflect.Type) reflect.Type { return reflect.MapOf(stringType, elem) } + case reflect.Slice: + wrap = reflect.SliceOf + case reflect.Array: + wrap = func(elem reflect.Type) reflect.Type { return reflect.ArrayOf(t.Len(), elem) } + case reflect.Pointer: + wrap = reflect.PointerTo + case reflect.Struct: + return fileStructType(t, lang, visiting) + default: + return t, nil + } + + elem, err := fileValueType(t.Elem(), lang, visiting) + if err != nil { + return nil, err + } + + // !readsText ensures that a type backed by a string is still given a chance to detect duplicate parsing + // to the same value. For example if type foo string has an UnmarshalText that lower cases input, we need to ensure + // 'foo' and 'Foo' are considered collisions instead of silently replacing them. + if elem == t.Elem() && (t.Kind() != reflect.Map || t.Key().Kind() == reflect.String && !readsText(t.Key())) { + return t, nil + } + + return wrap(elem), nil +} + +// Unexported fields, which reflect.StructOf can't make, and fields the markup ignores are never set from a file, so +// they're left out. StructOf doesn't promote an embedded field's methods either, so a struct that embeds one can't be +// rebuilt. +func fileStructType(t reflect.Type, lang markup.Markup, visiting map[reflect.Type]bool) (reflect.Type, error) { + var fields []reflect.StructField + changed, embeds := false, false + for i := range t.NumField() { + f := t.Field(i) + if _, read := lang.Key(f); !read || (!f.IsExported() && !f.Anonymous) { + continue + } + + ft, err := fileValueType(f.Type, lang, visiting) + if err != nil { + return nil, err + } + + changed = changed || ft != f.Type + embeds = embeds || f.Anonymous + fields = append(fields, reflect.StructField{Name: f.Name, Type: ft, Tag: f.Tag}) + } + + switch { + case !changed: + return t, nil + case embeds: + return nil, fmt.Errorf("%s holds a map with non-string keys and embeds a field, so it can't be rebuilt to read "+ + "the keys as text; name the embedded field", t) + default: + return reflect.StructOf(fields), nil + } +} + +// Turns a value decoded as fileValueType(t) back into t, parsing the map keys that were read as strings. +func fromFile(t reflect.Type, v reflect.Value) (reflect.Value, error) { + if v.Type() == t { + return v, nil + } + + switch t.Kind() { + case reflect.Map: + out := reflect.MakeMapWithSize(t, v.Len()) + seen := map[any]string{} + // Sorted, so a collision is reported the same way every run. + rawKeys := v.MapKeys() + slices.SortFunc(rawKeys, func(a, b reflect.Value) int { return cmp.Compare(a.String(), b.String()) }) + for _, rawKey := range rawKeys { + key, err := mapKey(t.Key(), rawKey.String(), seen) + if err != nil { + return reflect.Value{}, err + } + + elem, err := fromFile(t.Elem(), v.MapIndex(rawKey)) + if err != nil { + return reflect.Value{}, err + } + + out.SetMapIndex(key, elem) + } + + return out, nil + case reflect.Slice, reflect.Array: + out := reflect.New(t).Elem() + if t.Kind() == reflect.Slice { + out = reflect.MakeSlice(t, v.Len(), v.Len()) + } + + for i := range v.Len() { + elem, err := fromFile(t.Elem(), v.Index(i)) + if err != nil { + return reflect.Value{}, err + } + + out.Index(i).Set(elem) + } + + return out, nil + case reflect.Struct: + out := reflect.New(t).Elem() + for i := range t.NumField() { + src := v.FieldByName(t.Field(i).Name) + if !src.IsValid() { + continue + } + + field, err := fromFile(t.Field(i).Type, src) + if err != nil { + return reflect.Value{}, err + } + + out.Field(i).Set(field) + } + + return out, nil + default: // reflect.Pointer, the only other kind fileValueType changes + if v.IsNil() { + return reflect.Zero(t), nil + } + + elem, err := fromFile(t.Elem(), v.Elem()) + if err != nil { + return reflect.Value{}, err + } + + out := reflect.New(t.Elem()) + out.Elem().Set(elem) + return out, nil + } +} + +// Pointers and channels compare by identity, so two parses of one text are different keys. An interface may hold +// either, or a value that can't be a key at all. Arrays and structs are the only composite key types, and neither can +// hold itself except through a pointer. +func holdsIdentity(t reflect.Type) bool { + switch t.Kind() { + case reflect.Pointer, reflect.UnsafePointer, reflect.Chan, reflect.Interface: + return true + case reflect.Array: + return holdsIdentity(t.Elem()) + case reflect.Struct: + for i := range t.NumField() { + if holdsIdentity(t.Field(i).Type) { + return true + } + } + + return false + default: + return false + } +} + +// seen makes two texts that parse to one key, such as 16 and 0x10, an error rather than one silently replacing the +// other. +func mapKey(t reflect.Type, text string, seen map[any]string) (reflect.Value, error) { + key, err := parseText(t, text) + if err != nil { + return reflect.Value{}, fmt.Errorf("key %q: %w", text, err) + } + + // NaN never equals itself, nor does a key holding one, so each would be its own key and no lookup could find one. + if !key.Equal(key) { + return reflect.Value{}, fmt.Errorf("key %q never equals itself, as NaN doesn't, so it can't be looked up", text) + } + + if other, dup := seen[key.Interface()]; dup && other != text { + return reflect.Value{}, fmt.Errorf("keys %q and %q are both %v", other, text, key.Interface()) + } + + seen[key.Interface()] = text + return key, nil +} diff --git a/x/config/cli/entry.go b/x/config/cli/entry.go index eb206b1e0c..33c2a24361 100644 --- a/x/config/cli/entry.go +++ b/x/config/cli/entry.go @@ -52,6 +52,11 @@ type leafKey struct { // goPath and goType save walking the struct again. goPath []string goType reflect.Type + + // fileType is goType with every map key a string, such as map[string]string for map[uint32]string. A config file + // is decoded into it rather than goType, so fromFile can parse each key as its flag would and catch two texts for + // one key, such as 16 and 0x10. Flags and env vars are already text, so they don't use it. + fileType reflect.Type } type typedEntry[T any] struct { @@ -119,7 +124,12 @@ func (e *typedEntry[T]) sources(k leafKey, cc commandConfig) ([]reflect.Value, e } if raw, ok := cc.keys.lookup(cc.fileValues, k.fileKey); ok { - vals = append(vals, raw) + val, err := fromFile(commentparsing.DerefType(k.goType), raw) + if err != nil { + return nil, err + } + + vals = append(vals, val) } return vals, nil @@ -230,7 +240,14 @@ func (e *typedEntry[T]) decode(cc commandConfig) error { f = f.Elem() } - f.Set(vals[0]) + if mergesByKey(f.Type(), e.b.opts.Markup) { + // The struct's own entries first, then sources lowest precedence first, so the highest wins a key. + vals = append(vals, f) + slices.Reverse(vals) + f.Set(mergeMaps(f.Type(), vals...)) + } else { + f.Set(vals[0]) + } } return nil diff --git a/x/config/cli/examples/simple/README.md b/x/config/cli/examples/simple/README.md index d8865c82f6..621a1e1d35 100644 --- a/x/config/cli/examples/simple/README.md +++ b/x/config/cli/examples/simple/README.md @@ -15,10 +15,10 @@ go run . go run . --config example.toml # environment values -APP_HOST=env.example.com APP_PORT=7070 APP_TIMEOUT=4s go run . +APP_HOST=env.example.com APP_PORT=7070 APP_TAGS=a,b APP_TIMEOUT=4s go run . # CLI values -go run . --host cli.example.com --port 6060 --timeout 11s +go run . --host cli.example.com --port 6060 --tags a --tags b --timeout 11s # all sources: host from the CLI, port from the environment, the rest from TOML # APP_HOST is intentionally left in the call to demonstrate precedence. diff --git a/x/config/cli/list.go b/x/config/cli/list.go new file mode 100644 index 0000000000..80a0edcef4 --- /dev/null +++ b/x/config/cli/list.go @@ -0,0 +1,88 @@ +package cli + +import ( + "encoding/csv" + "errors" + "fmt" + "io" + "reflect" + "strings" +) + +// Lists are CSV, as pflag's slice flags are, so an element can be quoted: --tags '"a,b",c' is two elements. The first +// occurrence replaces the default and later ones append, so a list can be replaced or built up. pflag has a slice flag +// only for a fixed set of element types; this takes any element type with a text form. +type textListValue struct { + // list starts as the default, which only shows in --help: decoding reads it only once the flag is set. + list reflect.Value + replaced bool +} + +func (l *textListValue) Set(s string) error { + elems, err := parseList(s) + if err != nil { + return err + } + + if !l.replaced { + l.list, l.replaced = reflect.MakeSlice(l.list.Type(), 0, len(elems)), true + } + + for _, elem := range elems { + v, err := parseText(l.list.Type().Elem(), elem) + if err != nil { + return fmt.Errorf("element %q: %w", elem, err) + } + + l.list = reflect.Append(l.list, v) + } + + return nil +} + +// String has no brackets, unlike pflag's slice flags, so the default reads as it would be typed. +func (l *textListValue) String() string { return writeCSV(textListOf(l.list)) } + +// Type is syntax rather than a Go type, so --help shows how to write it. +func (l *textListValue) Type() string { return "value,..." } + +func parseList(s string) ([]string, error) { + if s == "" { + return nil, nil + } + + r := csv.NewReader(strings.NewReader(s)) + fields, err := r.Read() + if err != nil { + return nil, err + } + + // A second record would otherwise be dropped silently. + if _, err = r.Read(); !errors.Is(err, io.EOF) { + return nil, fmt.Errorf("%q must be one line; quote an element that holds a newline", s) + } + + return fields, nil +} + +func writeCSV(fields []string) string { + // A lone empty field would write as an empty line, which parseList reads as no elements. + if len(fields) == 1 && fields[0] == "" { + return `""` + } + + var b strings.Builder + w := csv.NewWriter(&b) + _ = w.Write(fields) + w.Flush() + return strings.TrimSuffix(b.String(), "\n") +} + +func textListOf(v reflect.Value) []string { + out := make([]string, v.Len()) + for i := range out { + out[i] = textOf(v.Index(i)) + } + + return out +} diff --git a/x/config/cli/maps.go b/x/config/cli/maps.go new file mode 100644 index 0000000000..119fc5d5e0 --- /dev/null +++ b/x/config/cli/maps.go @@ -0,0 +1,85 @@ +package cli + +import ( + "fmt" + "reflect" + "slices" + "strings" + + "github.com/smartcontractkit/chainlink-common/x/config/markup" +) + +const mapKVSep = "=" + +// Map entries merge by key at every source, so each occurrence adds to the config file's and the default's. Keys parse +// like values, so two texts that parse to one key, such as 16 and 0x10, are an error (see mapKey). +type textMapValue struct { + // m holds only the entries given; decoding merges them over the file's and the default's. + m reflect.Value + def reflect.Value + seen map[any]string +} + +func (m *textMapValue) Set(s string) error { + entries, err := parseList(s) + if err != nil { + return err + } + + for _, entry := range entries { + if entry == "" { + continue + } + + k, v, ok := strings.Cut(entry, mapKVSep) + if !ok { + return fmt.Errorf("%q must be formatted as key%svalue", entry, mapKVSep) + } + + key, err := mapKey(m.m.Type().Key(), k, m.seen) + if err != nil { + return err + } + + val, err := parseText(m.m.Type().Elem(), v) + if err != nil { + return fmt.Errorf("value of %q: %w", k, err) + } + + m.m.SetMapIndex(key, val) + } + + return nil +} + +func (m *textMapValue) String() string { return writeCSV(textMapOf(mergeMaps(m.m.Type(), m.def, m.m))) } + +// Type is syntax rather than a Go type, so --help shows how to write it. +func (m *textMapValue) Type() string { return "key=value,..." } + +// Sorted for stable --help output. +func textMapOf(v reflect.Value) []string { + pairs := make([]string, 0, v.Len()) + for iter := v.MapRange(); iter.Next(); { + pairs = append(pairs, textOf(iter.Key())+mapKVSep+textOf(iter.Value())) + } + + slices.Sort(pairs) + return pairs +} + +// A map read whole, by the markup or as a flag's text, is one value, so a later source replaces it. +func mergesByKey(t reflect.Type, lang markup.Markup) bool { + return t.Kind() == reflect.Map && !lang.IsLeaf(t) && !readsText(t) +} + +func mergeMaps(t reflect.Type, srcs ...reflect.Value) reflect.Value { + out := reflect.MakeMap(t) + for _, m := range srcs { + for iter := m.MapRange(); iter.Next(); { + out.SetMapIndex(iter.Key(), iter.Value()) + } + } + + return out +} diff --git a/x/config/cli/register.go b/x/config/cli/register.go index 27dc53be12..661501cf9c 100644 --- a/x/config/cli/register.go +++ b/x/config/cli/register.go @@ -17,6 +17,11 @@ import ( var durationType = reflect.TypeFor[time.Duration]() func bindLeafFlag(entry targetEntry, m fieldMeta) error { + fileType, err := fileValueType(commentparsing.DerefType(m.field.Type), entry.binder().opts.Markup, map[reflect.Type]bool{}) + if err != nil { + return fmt.Errorf("%s: %s: %w", m.owner, m.field.Name, err) + } + flags := entry.command().PersistentFlags() leaf := leafKey{ key: m.key, @@ -24,6 +29,7 @@ func bindLeafFlag(entry targetEntry, m fieldMeta) error { flagName: strings.ReplaceAll(m.key, "_", "-"), goPath: m.goPath, goType: m.field.Type, + fileType: fileType, } // pflag panics on a redefinition. if flags.Lookup(leaf.flagName) != nil { @@ -83,6 +89,13 @@ func newFlag(name string, t reflect.Type, def reflect.Value, usage string) (*pfl } value, get = v, func() reflect.Value { return v.value } + + case t.Kind() == reflect.Slice && t.Elem().Kind() != reflect.Pointer && canText(t.Elem()): + l := &textListValue{list: def} + value, get = l, func() reflect.Value { return l.list } + case t.Kind() == reflect.Map && t.Elem().Kind() != reflect.Pointer && canText(t.Key()) && canText(t.Elem()): + m := &textMapValue{m: reflect.MakeMap(t), def: def, seen: map[any]string{}} + value, get = m, func() reflect.Value { return m.m } default: return nil, nil } @@ -176,7 +189,7 @@ func setText(dst reflect.Value, s string) error { return err } -// Parsed through a flag, so an env var accepts exactly what the flag does. +// Parsed through a flag, so env vars and config file map keys accept exactly what the flag does. func parseText(t reflect.Type, s string) (reflect.Value, error) { f, get := newFlag("value", t, reflect.Zero(commentparsing.DerefType(t)), "") if f == nil { @@ -190,22 +203,36 @@ func parseText(t reflect.Type, s string) (reflect.Value, error) { return get(), nil } +// Shown for a default that can't be written as text. Angle brackets, unlike fmt's {x}, don't look like a value. +const cannotDisplay = "" + // A type's own marshaller, so the default parses back and secrets stay redacted. func textOf(v reflect.Value) string { - if !v.IsValid() || (v.Kind() == reflect.Pointer && v.IsNil()) { - return "" - } - - v = reflect.Indirect(v) if text, ok := marshalText(v); ok { return text } - if v.Kind() == reflect.Slice && v.Type().Elem().Kind() == reflect.Uint8 { + // By kind, as the flag parses it, rather than through fmt, which would use a String method the flag can't read back. + switch { + // Reads text but can't write it, and its kind's form may not parse back. + case readsText(v.Type()): + case v.Type() == durationType: + return time.Duration(v.Int()).String() + case v.CanInt(): + return strconv.FormatInt(v.Int(), 10) + case v.CanUint(): + return strconv.FormatUint(v.Uint(), 10) + case v.CanFloat(): + return strconv.FormatFloat(v.Float(), 'g', -1, v.Type().Bits()) + case v.Kind() == reflect.Bool: + return strconv.FormatBool(v.Bool()) + case v.Kind() == reflect.String: + return v.String() + case v.Kind() == reflect.Slice && v.Type().Elem().Kind() == reflect.Uint8: return string(v.Bytes()) } - return fmt.Sprint(v.Interface()) + return cannotDisplay } // Copied somewhere addressable first, so a pointer-receiver MarshalText is reachable. diff --git a/x/config/cli/walk.go b/x/config/cli/walk.go index 06318a3043..f05b133d16 100644 --- a/x/config/cli/walk.go +++ b/x/config/cli/walk.go @@ -25,9 +25,10 @@ type fieldMeta struct { inOptional bool } -// A flag or env var carries only text. +// A flag or env var carries only text. An interface can't be parsed into, since there's no concrete type to make. func readsText(t reflect.Type) bool { - return t.Implements(textUnmarshaler) || reflect.PointerTo(t).Implements(textUnmarshaler) + return t.Kind() != reflect.Interface && + (t.Implements(textUnmarshaler) || reflect.PointerTo(t).Implements(textUnmarshaler)) } var textUnmarshaler = reflect.TypeFor[encoding.TextUnmarshaler]() From ee3c3bb56916a77579c8faf9e30255605e3be603 Mon Sep 17 00:00:00 2001 From: Ryan Tinianov Date: Thu, 8 Oct 2026 09:43:42 -0400 Subject: [PATCH 2/2] Small PR feedback fixes --- x/config/cli/binder_test.go | 4 +--- x/config/cli/decode.go | 24 ++++++++++++------------ x/config/cli/maps.go | 8 ++------ 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/x/config/cli/binder_test.go b/x/config/cli/binder_test.go index 417979629d..7c9261dc96 100644 --- a/x/config/cli/binder_test.go +++ b/x/config/cli/binder_test.go @@ -732,9 +732,7 @@ func TestMapEntryText(t *testing.T) { require.NoError(t, run(t, &c, testOptions, "--map", `"a=1,b=2",c=3`)) assert.Equal(t, map[string]string{"a": "1,b=2", "c": "3"}, c.Map) - var blank hasStringMap - require.NoError(t, run(t, &blank, testOptions, "--map", "a=1,,b=2")) - assert.Equal(t, map[string]string{"a": "1", "b": "2"}, blank.Map) + require.ErrorContains(t, run(t, &hasStringMap{}, testOptions, "--map", "a=1,,b=2"), `"" must be formatted as key=value`) require.ErrorContains(t, run(t, &hasStringMap{}, testOptions, "--map", `"a=1`), `extraneous or missing " in quoted-field`) diff --git a/x/config/cli/decode.go b/x/config/cli/decode.go index 5b3cb26c60..d4c3d5b742 100644 --- a/x/config/cli/decode.go +++ b/x/config/cli/decode.go @@ -157,7 +157,7 @@ func convertFileValueType(t reflect.Type, lang markup.Markup, visiting map[refle var wrap func(reflect.Type) reflect.Type switch t.Kind() { case reflect.Map: - if holdsIdentity(t.Key()) { + if holdsIdentityKind(t.Key()) { return nil, fmt.Errorf("%s has keys that hold a pointer, channel or interface, which can compare by address, "+ "so 2 and 2 could be different keys; key it by value", t) } @@ -303,15 +303,15 @@ func fromFile(t reflect.Type, v reflect.Value) (reflect.Value, error) { // Pointers and channels compare by identity, so two parses of one text are different keys. An interface may hold // either, or a value that can't be a key at all. Arrays and structs are the only composite key types, and neither can // hold itself except through a pointer. -func holdsIdentity(t reflect.Type) bool { +func holdsIdentityKind(t reflect.Type) bool { switch t.Kind() { case reflect.Pointer, reflect.UnsafePointer, reflect.Chan, reflect.Interface: return true case reflect.Array: - return holdsIdentity(t.Elem()) + return holdsIdentityKind(t.Elem()) case reflect.Struct: - for i := range t.NumField() { - if holdsIdentity(t.Field(i).Type) { + for field := range t.Fields() { + if holdsIdentityKind(field.Type) { return true } } @@ -324,21 +324,21 @@ func holdsIdentity(t reflect.Type) bool { // seen makes two texts that parse to one key, such as 16 and 0x10, an error rather than one silently replacing the // other. -func mapKey(t reflect.Type, text string, seen map[any]string) (reflect.Value, error) { - key, err := parseText(t, text) +func mapKey(keyType reflect.Type, keyText string, seen map[any]string) (reflect.Value, error) { + key, err := parseText(keyType, keyText) if err != nil { - return reflect.Value{}, fmt.Errorf("key %q: %w", text, err) + return reflect.Value{}, fmt.Errorf("key %q: %w", keyText, err) } // NaN never equals itself, nor does a key holding one, so each would be its own key and no lookup could find one. if !key.Equal(key) { - return reflect.Value{}, fmt.Errorf("key %q never equals itself, as NaN doesn't, so it can't be looked up", text) + return reflect.Value{}, fmt.Errorf("key %q never equals itself, as NaN doesn't, so it can't be looked up", keyText) } - if other, dup := seen[key.Interface()]; dup && other != text { - return reflect.Value{}, fmt.Errorf("keys %q and %q are both %v", other, text, key.Interface()) + if other, dup := seen[key.Interface()]; dup && other != keyText { + return reflect.Value{}, fmt.Errorf("keys %q and %q are both %v", other, keyText, key.Interface()) } - seen[key.Interface()] = text + seen[key.Interface()] = keyText return key, nil } diff --git a/x/config/cli/maps.go b/x/config/cli/maps.go index 119fc5d5e0..e649aa513e 100644 --- a/x/config/cli/maps.go +++ b/x/config/cli/maps.go @@ -27,10 +27,6 @@ func (m *textMapValue) Set(s string) error { } for _, entry := range entries { - if entry == "" { - continue - } - k, v, ok := strings.Cut(entry, mapKVSep) if !ok { return fmt.Errorf("%q must be formatted as key%svalue", entry, mapKVSep) @@ -60,8 +56,8 @@ func (m *textMapValue) Type() string { return "key=value,..." } // Sorted for stable --help output. func textMapOf(v reflect.Value) []string { pairs := make([]string, 0, v.Len()) - for iter := v.MapRange(); iter.Next(); { - pairs = append(pairs, textOf(iter.Key())+mapKVSep+textOf(iter.Value())) + for key, val := range v.Seq2() { + pairs = append(pairs, textOf(key)+mapKVSep+textOf(val)) } slices.Sort(pairs)