From c70f8701a68243b64de26746014a0f804498cbad Mon Sep 17 00:00:00 2001 From: fei <204683769+feiiiiii5@users.noreply.github.com> Date: Mon, 21 Sep 2026 02:45:59 +0800 Subject: [PATCH] fix(requestflag): merge inner flags into untyped array literals cliValue.SetInnerField dispatched the slice case on the static element kind, so an untyped Flag[any] holding a []any from a JSON or YAML array literal returned before the merge path and the inner-field value was dropped while the flag still counted as supplied. Dispatch elements by their dynamic type and check the trailing-element assertion, so a Flag[[]map[string]any] outer keeps its existing behavior and a []any outer follows the same merge or append-new-element rules. --- .../innerflag_untyped_array_test.go | 107 ++++++++++++++++++ internal/requestflag/requestflag.go | 25 ++-- 2 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 internal/requestflag/innerflag_untyped_array_test.go diff --git a/internal/requestflag/innerflag_untyped_array_test.go b/internal/requestflag/innerflag_untyped_array_test.go new file mode 100644 index 00000000..67e063e8 --- /dev/null +++ b/internal/requestflag/innerflag_untyped_array_test.go @@ -0,0 +1,107 @@ +package requestflag + +import ( + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" +) + +// An untyped outer flag (Flag[any], used for nullable array-of-objects schemas) that is +// given a JSON or YAML array literal holds a []any, not a []map[string]any. Inner flags +// must merge into the trailing element the same way they do for a typed slice flag. +func TestInnerFlagAfterUntypedArrayLiteral(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + args []string + want string + }{ + { + "merge into the trailing element", + []string{"--entry", `[{"name":"earlier"}]`, "--entry.description", "details"}, + `{"entries":[{"name":"earlier","description":"details"}]}`, + }, + { + "repeated field starts another element", + []string{"--entry", `[{"name":"earlier"}]`, "--entry.name", "demo"}, + `{"entries":[{"name":"earlier"},{"name":"demo"}]}`, + }, + { + "two inner fields fill an empty element", + []string{"--entry", "[{}]", "--entry.name", "demo", "--entry.description", "details"}, + `{"entries":[{"name":"demo","description":"details"}]}`, + }, + { + "yaml flow array", + []string{"--entry", "- {name: earlier}", "--entry.description", "details"}, + `{"entries":[{"name":"earlier","description":"details"}]}`, + }, + { + "non-object element starts a new one", + []string{"--entry", `["plain"]`, "--entry.name", "demo"}, + `{"entries":["plain",{"name":"demo"}]}`, + }, + { + "null element starts a new one", + []string{"--entry", `[null]`, "--entry.name", "demo"}, + `{"entries":[null,{"name":"demo"}]}`, + }, + { + "empty array", + []string{"--entry", "[]", "--entry.name", "demo"}, + `{"entries":[{"name":"demo"}]}`, + }, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + outer := &Flag[any]{Name: "entry", BodyPath: "entries"} + var body []byte + command := WithInnerFlags(cli.Command{ + Name: "test", + Flags: []cli.Flag{outer}, + Action: func(_ context.Context, cmd *cli.Command) error { + var err error + body, err = json.Marshal(ExtractRequestContents(cmd).Body) + return err + }, + }, map[string][]HasOuterFlag{ + "entry": { + &InnerFlag[string]{Name: "entry.name", InnerField: "name", OuterIsArrayOfObjects: true}, + &InnerFlag[string]{Name: "entry.description", InnerField: "description", OuterIsArrayOfObjects: true}, + }, + }) + + require.NoError(t, command.Run(context.Background(), append([]string{"test"}, tt.args...))) + assert.JSONEq(t, tt.want, string(body)) + }) + } +} + +// Same sequence driven directly, mirroring the existing coverage for a `null` element. +func TestInnerFlagAfterUntypedArrayLiteralDirect(t *testing.T) { + t.Parallel() + + outer := &Flag[any]{Name: "entry"} + require.NoError(t, outer.PreParse()) + require.NoError(t, outer.Set(outer.Name, `[{"name":"earlier"}]`)) + + name := &InnerFlag[string]{ + Name: "entry.name", InnerField: "name", OuterFlag: outer, OuterIsArrayOfObjects: true, + } + description := &InnerFlag[string]{ + Name: "entry.description", InnerField: "description", OuterFlag: outer, OuterIsArrayOfObjects: true, + } + require.NoError(t, name.Set(name.Name, "demo")) + require.NoError(t, description.Set(description.Name, "details")) + + assert.Equal(t, + []any{map[string]any{"name": "earlier"}, map[string]any{"name": "demo", "description": "details"}}, + outer.Get(), + ) +} diff --git a/internal/requestflag/requestflag.go b/internal/requestflag/requestflag.go index cc56adf0..aa7fed27 100644 --- a/internal/requestflag/requestflag.go +++ b/internal/requestflag/requestflag.go @@ -946,22 +946,29 @@ func (c *cliValue[T]) SetInnerField(field string, val any) { flagValReflect := reflect.ValueOf(flagVal) switch flagValReflect.Kind() { case reflect.Slice: - if flagValReflect.Type().Elem().Kind() != reflect.Map { + // An untyped outer flag holds a []any once it is set from a JSON or YAML array + // literal, so elements are dispatched by their dynamic type rather than by the + // static element kind. + switch flagValReflect.Type().Elem().Kind() { + case reflect.Map, reflect.Interface: + default: return } sliceLen := flagValReflect.Len() if sliceLen > 0 { // Check if the last element already has the InnerField - lastElement := flagValReflect.Index(sliceLen - 1).Interface().(map[string]any) - if _, hasInnerField := lastElement[field]; !hasInnerField { - if lastElement == nil { - lastElement = make(map[string]any) - flagValReflect.Index(sliceLen - 1).Set(reflect.ValueOf(lastElement)) + lastElement, isObject := flagValReflect.Index(sliceLen - 1).Interface().(map[string]any) + if isObject { + if _, hasInnerField := lastElement[field]; !hasInnerField { + if lastElement == nil { + lastElement = make(map[string]any) + flagValReflect.Index(sliceLen - 1).Set(reflect.ValueOf(lastElement)) + } + // Last element doesn't have the field, set it + lastElement[field] = val + return } - // Last element doesn't have the field, set it - lastElement[field] = val - return } }