Accept valid empty collections after gRPC transport

AGENTS.md

@@ -155,4 +155,9 @@ To reproduce a codegen issue:
 
 ### Slices/Maps and Required Fields
 
-Do not rely on nil vs empty to encode presence. Goa uses `omitempty`—both nil and empty serialize as "missing". If empty is valid, do not mark the field as required.
+Do not rely on nil versus empty slices or maps to encode domain meaning.
+Required JSON fields may contain empty collections unless a length constraint
+forbids them; keep `Required` when the JSON property must be present. Protobuf
+repeated and map fields cannot distinguish absent from empty, so their generated
+validation checks length and contents, not presence. Message, scalar, and oneof
+presence checks remain independent of collection emptiness.

codegen/go_type_plan.go

@@ -60,6 +60,9 @@ type (
 		Pointer bool
 		// IgnoreRequired suppresses required checks for primitive transport fields.
 		IgnoreRequired bool
+		// IgnoreRequiredCollections suppresses array and map presence checks when
+		// the transport represents absent and empty collections identically.
+		IgnoreRequiredCollections bool
 		// UseDefault keeps optional primitive fields with defaults as values.
 		UseDefault bool
 		// UnionPointer uses pointers for optional sum-type union fields and for

codegen/transformer.go

@@ -47,6 +47,10 @@ type (
 		// (proto) uses non-pointers to hold required attributes and
 		// therefore do not need to be validated.
 		IgnoreRequired bool
+		// IgnoreRequiredCollections suppresses presence checks for arrays and maps
+		// in transports such as protobuf that cannot distinguish empty from absent.
+		// Length and element validations still apply.
+		IgnoreRequiredCollections bool
 		// UseDefault if true indicates that the attribute uses non-pointers for
 		// primitive types if they have default value. If false, the attribute with
 		// primitive types are non-pointers if they are required, otherwise they
@@ -502,12 +506,13 @@ func (a *AttributeContext) IsArrayElementPointer(array *expr.Array) bool {
 // generated value.
 func (a *AttributeContext) LayoutPolicy() GoLayoutPolicy {
 	return GoLayoutPolicy{
-		Pointer:             a.Pointer,
-		IgnoreRequired:      a.IgnoreRequired,
-		UseDefault:          a.UseDefault,
-		UnionPointer:        a.UnionPointer,
-		ArrayElementPointer: a.ArrayElementPointer,
-		SumType:             a.Scope.IsSumType(),
+		Pointer:                   a.Pointer,
+		IgnoreRequired:            a.IgnoreRequired,
+		IgnoreRequiredCollections: a.IgnoreRequiredCollections,
+		UseDefault:                a.UseDefault,
+		UnionPointer:              a.UnionPointer,
+		ArrayElementPointer:       a.ArrayElementPointer,
+		SumType:                   a.Scope.IsSumType(),
 	}
 }
 
@@ -550,12 +555,13 @@ func (a *AttributeContext) WithGoTypeLayout(layout LinkedGoType) (*AttributeCont
 // Dup creates a shallow copy of the AttributeContext.
 func (a *AttributeContext) Dup() *AttributeContext {
 	return &AttributeContext{
-		Pointer:             a.Pointer,
-		IgnoreRequired:      a.IgnoreRequired,
-		UseDefault:          a.UseDefault,
-		Scope:               a.Scope,
-		UnionPointer:        a.UnionPointer,
-		ArrayElementPointer: a.ArrayElementPointer,
+		Pointer:                   a.Pointer,
+		IgnoreRequired:            a.IgnoreRequired,
+		IgnoreRequiredCollections: a.IgnoreRequiredCollections,
+		UseDefault:                a.UseDefault,
+		Scope:                     a.Scope,
+		UnionPointer:              a.UnionPointer,
+		ArrayElementPointer:       a.ArrayElementPointer,
 	}
 }
 

codegen/validation.go

@@ -550,7 +550,7 @@ func validationCode(att *expr.AttributeExpr, attCtx *AttributeContext, req, alia
 			res = append(res, val)
 		}
 	}
-	reqs := generatedRequiredValidation(att, validation, attCtx)
+	reqs := generatedRequiredValidationNames(att, validation, attCtx.LayoutPolicy())
 	obj := expr.AsObject(att.Type)
 	for _, r := range reqs {
 		reqAtt := obj.Attribute(r)
@@ -594,39 +594,7 @@ func renderValidationPath(path validationPath) string {
 // hasValidations reports whether validating ut can write any code with the Go
 // layout described by attCtx.
 func hasValidations(attCtx *AttributeContext, ut expr.UserType) bool {
-	policy := GoLayoutPolicy{
-		Pointer:             attCtx.Pointer,
-		IgnoreRequired:      attCtx.IgnoreRequired,
-		UseDefault:          attCtx.UseDefault,
-		UnionPointer:        attCtx.UnionPointer,
-		ArrayElementPointer: attCtx.ArrayElementPointer,
-		SumType:             attCtx.Scope.IsSumType(),
-	}
-	return NeedsValidation(ut.Attribute(), policy)
-}
-
-// There is a case where there is validation but no actual validation code: if
-// the validation is a required validation that applies to attributes that
-// cannot be nil i.e. primitive types. val is the validation that effectively
-// applies to att as computed by expr.EffectiveValidation.
-func generatedRequiredValidation(att *expr.AttributeExpr, val *expr.ValidationExpr, attCtx *AttributeContext) (res []string) {
-	obj := expr.AsObject(att.Type)
-	for _, req := range val.Required {
-		reqAtt := obj.Attribute(req)
-		if reqAtt == nil {
-			continue
-		}
-		if !attCtx.Pointer && expr.IsPrimitive(reqAtt.Type) &&
-			reqAtt.Type.Kind() != expr.BytesKind &&
-			reqAtt.Type.Kind() != expr.AnyKind {
-			continue
-		}
-		if attCtx.IgnoreRequired && expr.IsPrimitive(reqAtt.Type) {
-			continue
-		}
-		res = append(res, req)
-	}
-	return
+	return NeedsValidation(ut.Attribute(), attCtx.LayoutPolicy())
 }
 
 // toSlice returns Go code that represents the given slice.

codegen/validation_plan.go

@@ -494,6 +494,9 @@ func generatedRequiredValidationNames(attribute *expr.AttributeExpr, validation
 		if policy.IgnoreRequired && expr.IsPrimitive(required.Type) {
 			continue
 		}
+		if policy.IgnoreRequiredCollections && (expr.IsArray(required.Type) || expr.IsMap(required.Type)) {
+			continue
+		}
 		names = append(names, name)
 	}
 	return names

grpc/codegen/protobuf.go

@@ -106,6 +106,7 @@ func (p *protoBufScope) Scope() *codegen.NameScope {
 // remain slices. Message fields, including Any, already use pointers.
 func protoBufTypeContext(pkg string, service *ServiceData) *codegen.AttributeContext {
 	ctx := codegen.NewAttributeContext(true, false, false, pkg, service.Scope)
+	ctx.IgnoreRequiredCollections = true
 	ctx.Scope = &protoBufScope{service: service, pkg: pkg}
 	return ctx
 }

grpc/codegen/protobuf_test.go

@@ -101,6 +101,8 @@ func TestProtoBufTypeContextPreservesPrimitivePresence(t *testing.T) {
 	ctx := protoBufTypeContext("proto", sd)
 	require.True(t, ctx.Pointer)
 	require.False(t, ctx.IgnoreRequired)
+	require.True(t, ctx.IgnoreRequiredCollections)
+	require.True(t, ctx.Dup().LayoutPolicy().IgnoreRequiredCollections)
 	require.False(t, ctx.UseDefault)
 }
 

grpc/codegen/required_collections_runtime_test.go

@@ -0,0 +1,181 @@
+// These tests run generated gRPC and HTTP validators against the same required
+// collections. Protobuf has no collection presence; JSON still requires fields.
+package codegen
+
+import (
+	"go/format"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"goa.design/goa/v3/codegen/service"
+	d "goa.design/goa/v3/dsl"
+	"goa.design/goa/v3/expr"
+	httpcodegen "goa.design/goa/v3/http/codegen"
+)
+
+func TestRequiredCollectionsPreserveTransportContracts(t *testing.T) {
+	root := expr.RunDSL(t, requiredCollectionsDSL)
+	generation, servicePlans := grpcServicePlans(t, []*expr.RootExpr{root})
+	grpcPlans, err := NewPlans(generation, PlanInput{Root: root, Service: servicePlans[0]})
+	require.NoError(t, err)
+	httpPlans, err := httpcodegen.NewPlans(generation, httpcodegen.PlanInput{Root: root, Service: servicePlans[0]})
+	require.NoError(t, err)
+	require.NoError(t, generation.Freeze())
+	require.NoError(t, servicePlans[0].Link())
+	require.NoError(t, grpcPlans[0].Link())
+	require.NoError(t, httpPlans[0].Link())
+	files, err := service.Files(servicePlans...)
+	require.NoError(t, err)
+	files = append(files, grpcPlans[0].ServerFiles()...)
+	files = append(files, grpcPlans[0].ClientFiles()...)
+	files = append(files, grpcPlans[0].ServerTypeFiles()...)
+	files = append(files, grpcPlans[0].ClientTypeFiles()...)
+	files = append(files, grpcPlans[0].ProtoFiles()...)
+	files = append(files, httpPlans[0].ServerTypeFiles()...)
+	files = append(files, httpPlans[0].ServerFiles()...)
+	files = append(files, httpPlans[0].PathFiles()...)
+	directory := t.TempDir()
+	writeProtobufDescriptorModule(t, directory)
+	for _, file := range files {
+		_, err := file.Render(directory)
+		require.NoError(t, err)
+	}
+	source, err := format.Source([]byte(requiredCollectionsRuntimeTest))
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(filepath.Join(directory, "collections_test.go"), source, 0o600))
+	compileProtobufDescriptorModule(t, directory)
+}
+
+func requiredCollectionsDSL() {
+	item := d.Type("Item", func() {
+		d.Field(1, "label", d.String)
+		d.Required("label")
+	})
+	fields := func() {
+		d.Field(1, "items", d.ArrayOfRequired(item))
+		d.Field(2, "labels", d.MapOf(d.String, d.String))
+		d.Field(3, "detail", item)
+		d.Field(4, "enabled", d.Boolean)
+		d.Field(5, "count", d.Int)
+		d.Field(6, "text", d.String)
+		d.Required("items", "labels", "detail", "enabled", "count", "text")
+	}
+	bounded := func() {
+		d.Field(1, "items", d.ArrayOf(d.String), func() { d.MinLength(1) })
+		d.Field(2, "labels", d.MapOf(d.String, d.String), func() { d.MinLength(1) })
+		d.Required("items", "labels")
+	}
+	d.Service("Collections", func() {
+		d.Method("Exchange", func() {
+			d.Payload(fields)
+			d.Result(fields)
+			d.GRPC(func() {})
+			d.HTTP(func() { d.POST("/exchange") })
+		})
+		d.Method("Bounded", func() {
+			d.Payload(bounded)
+			d.Result(bounded)
+			d.GRPC(func() {})
+		})
+	})
+}
+
+const requiredCollectionsRuntimeTest = `package collections_test
+
+import (
+ "encoding/json"
+ "testing"
+
+ gengrpcclient "generated.local/gen/grpc/collections/client"
+ gengrpcserver "generated.local/gen/grpc/collections/server"
+ genpb "generated.local/gen/grpc/collections/pb"
+ genhttpserver "generated.local/gen/http/collections/server"
+ "google.golang.org/protobuf/proto"
+)
+
+func TestEmptyAndPopulatedCollectionsRoundTrip(t *testing.T) {
+ for _, populated := range []bool{false, true} {
+  input := &genpb.ExchangeRequest{
+   Items: []*genpb.Item{}, Labels: map[string]string{},
+   Detail: &genpb.Item{Label: proto.String("good")},
+   Enabled: proto.Bool(false), Count: proto.Int32(0), Text: proto.String(""),
+  }
+  if populated {
+   input.Items = []*genpb.Item{{Label: proto.String("item")}}
+   input.Labels["key"] = "value"
+  }
+  wire, err := proto.Marshal(input)
+  if err != nil { t.Fatal(err) }
+  request := new(genpb.ExchangeRequest)
+  response := new(genpb.ExchangeResponse)
+  if err := proto.Unmarshal(wire, request); err != nil { t.Fatal(err) }
+  if err := proto.Unmarshal(wire, response); err != nil { t.Fatal(err) }
+  if !populated && (request.Items != nil || request.Labels != nil) {
+   t.Fatal("test must exercise protobuf's loss of empty collection presence")
+  }
+  if err := gengrpcserver.ValidateExchangeRequest(request); err != nil { t.Fatal(err) }
+  if err := gengrpcclient.ValidateExchangeResponse(response); err != nil { t.Fatal(err) }
+ }
+}
+
+func TestRequiredNonCollectionFieldsAndElementsRemainValidated(t *testing.T) {
+ for _, field := range []string{"detail", "enabled", "count", "text", "item"} {
+  t.Run(field, func(t *testing.T) {
+   request := &genpb.ExchangeRequest{
+    Detail: &genpb.Item{Label: proto.String("good")},
+    Enabled: proto.Bool(false), Count: proto.Int32(0), Text: proto.String(""),
+   }
+   switch field {
+   case "detail": request.Detail = nil
+   case "enabled": request.Enabled = nil
+   case "count": request.Count = nil
+   case "text": request.Text = nil
+   case "item": request.Items = []*genpb.Item{{}}
+   }
+   wire, err := proto.Marshal(request)
+   if err != nil { t.Fatal(err) }
+   response := new(genpb.ExchangeResponse)
+   if err := proto.Unmarshal(wire, response); err != nil { t.Fatal(err) }
+   if err := gengrpcserver.ValidateExchangeRequest(request); err == nil { t.Fatal("server accepted missing required " + field) }
+   if err := gengrpcclient.ValidateExchangeResponse(response); err == nil { t.Fatal("client accepted missing required " + field) }
+  })
+ }
+ request := &genpb.ExchangeRequest{Items: []*genpb.Item{nil}, Detail: &genpb.Item{Label: proto.String("good")}, Enabled: proto.Bool(false), Count: proto.Int32(0), Text: proto.String("")}
+ if err := gengrpcserver.ValidateExchangeRequest(request); err == nil { t.Fatal("accepted nil required element") }
+}
+
+func TestCollectionMinimumLengthsRemainValidated(t *testing.T) {
+ for _, field := range []string{"items", "labels"} {
+  request := &genpb.BoundedRequest{Items: []string{"one"}, Labels: map[string]string{"key":"value"}}
+  if field == "items" { request.Items = nil } else { request.Labels = nil }
+  wire, err := proto.Marshal(request)
+  if err != nil { t.Fatal(err) }
+  response := new(genpb.BoundedResponse)
+  if err := proto.Unmarshal(wire, response); err != nil { t.Fatal(err) }
+  if err := gengrpcserver.ValidateBoundedRequest(request); err == nil { t.Fatal("server ignored minimum length for " + field) }
+  if err := gengrpcclient.ValidateBoundedResponse(response); err == nil { t.Fatal("client ignored minimum length for " + field) }
+ }
+}
+
+func TestJSONStillRequiresPresentNonNullCollections(t *testing.T) {
+ for _, test := range []struct{ name, fields string; valid bool }{
+  {"empty", "\"items\":[],\"labels\":{}", true},
+  {"populated", "\"items\":[{\"label\":\"item\"}],\"labels\":{\"key\":\"value\"}", true},
+  {"missing items", "\"labels\":{}", false},
+  {"missing labels", "\"items\":[]", false},
+  {"null items", "\"items\":null,\"labels\":{}", false},
+  {"null labels", "\"items\":[],\"labels\":null", false},
+ } {
+  t.Run(test.name, func(t *testing.T) {
+   data := "{" + test.fields + ",\"detail\":{\"label\":\"good\"},\"enabled\":false,\"count\":0,\"text\":\"\"}"
+   var body genhttpserver.ExchangeRequestBody
+   if err := json.Unmarshal([]byte(data), &body); err != nil { t.Fatal(err) }
+   err := genhttpserver.ValidateExchangeRequestBody(&body)
+   if (err == nil) != test.valid { t.Fatalf("valid=%v, error=%v", test.valid, err) }
+  })
+ }
+}
+`

grpc/codegen/testdata/golden/client_types_client-struct-field-name-meta-type.go.golden

@@ -31,19 +31,3 @@ func NewMethodResult(message *using_meta_typespb.MethodResponse) *usingmetatypes
 	}
 	return result
 }
-
-// ValidateMethodRequest runs the validations defined on MethodRequest.
-func ValidateMethodRequest(message *using_meta_typespb.MethodRequest) (err error) {
-	if message.B == nil {
-		err = goa.MergeErrors(err, goa.MissingFieldError("b", "message"))
-	}
-	return
-}
-
-// ValidateMethodResponse runs the validations defined on MethodResponse.
-func ValidateMethodResponse(message *using_meta_typespb.MethodResponse) (err error) {
-	if message.B == nil {
-		err = goa.MergeErrors(err, goa.MissingFieldError("b", "message"))
-	}
-	return
-}

grpc/codegen/testdata/golden/server_types_server-struct-field-name-meta-type.go.golden

@@ -31,11 +31,3 @@ func NewProtoMethodResponse(result *usingmetatypes.MethodResult) *using_meta_typ
 	}
 	return message
 }
-
-// ValidateMethodRequest runs the validations defined on MethodRequest.
-func ValidateMethodRequest(message *using_meta_typespb.MethodRequest) (err error) {
-	if message.B == nil {
-		err = goa.MergeErrors(err, goa.MissingFieldError("b", "message"))
-	}
-	return
-}