Repository: goadesign/goa (go)

Issue:
Element validation of a Map generates code that doesn't compile
Consider the following example design:
```go
package design

import (
	. "goa.design/goa/v3/dsl"
)

var _ = API("SomeAPI", func() {
	Title("Some API")
})

var _ = Service("SomeService", func() {
	Method("SomeMethod", func() {
		Payload(SomeType)
		GRPC(func(){})
	})
})

var SomeType = Type("SomeType", func() {
	Field(1, "foo", MapOf(String, ArrayOf(String)), "Map of string to array of strings", func() {
		Elem(func() {
			MinLength(1)
		})
		Example(map[string][]string{"foo": {"bar1", "bar2"}})
	})
	Required("foo")
})
```
and it produces the following generated code in `gen/grpc/some_service/server/types.go`:
```
// Code generated by goa v3.2.3, DO NOT EDIT.
//
// SomeService gRPC server types
//
// Command:
// $ goa gen goa-elem-issue/design

package server

import (
	some_servicepb "goa-elem-issue/gen/grpc/some_service/pb"
	someservice "goa-elem-issue/gen/some_service"

	goa "goa.design/goa/v3/pkg"
)

// NewSomeMethodPayload builds the payload of the "SomeMethod" endpoint of the
// "SomeService" service from the gRPC request type.
func NewSomeMethodPayload(message *some_servicepb.SomeMethodRequest) *someservice.SomeType {
	v := &someservice.SomeType{}
	if message.Foo != nil {
		v.Foo = make(map[string][]string, len(message.Foo))
		for key, val := range message.Foo {
			tk := key
			tv := make([]string, len(val.Field))
			for i, val := range val.Field {
				tv[i] = val
			}
			v.Foo[tk] = tv
		}
	}
	return v
}

// NewSomeMethodResponse builds the gRPC response type from the result of the
// "SomeMethod" endpoint of the "SomeService" service.
func NewSomeMethodResponse() *some_servicepb.SomeMethodResponse {
	message := &some_servicepb.SomeMethodResponse{}
	return message
}

// ValidateSomeMethodRequest runs the validations defined on SomeMethodRequest.
func ValidateSomeMethodRequest(message *some_servicepb.SomeMethodRequest) (err error) {
	if message.Foo == nil {
		err = goa.MergeErrors(err, goa.MissingFieldError("foo", "message"))
	}
	return
}

// ValidateArrayOfString runs the validations defined on ArrayOfString.
func ValidateArrayOfString(message *some_servicepb.ArrayOfString) (err error) {
       // <<<<<<<<<< Compilation Error >>>>>>>>>>>>>>>
	if len(message) < 1 {
		err = goa.MergeErrors(err, goa.InvalidLengthError("message", message, len(message), 1, true))
	}
	return
}
```

You can see the compilation error in the code above in the highlighted method where the `len` function is used on `*some_servicepb.ArrayOfString` instead of `*some_servicepb.ArrayOfString.Field` which contains the actual slice.

Additional observations in this example:
1. If no examples are provided in the design, the default example generation panics with the following error:
```
goa gen goa-elem-issue/design
exit status 2
panic: invalid type for length validation: ArrayOfString_

goroutine 1 [running]:
goa.design/goa/v3/expr.byLength(0xc0000b0790, 0xc0000abe60, 0x15e6cd4, 0xf)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/expr/example.go:161 +0x52d
goa.design/goa/v3/expr.(*AttributeExpr).Example(0xc0000b0790, 0xc0000abe60, 0x151dfc0, 0xc0000ec2c0)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/expr/example.go:34 +0xe5
goa.design/goa/v3/expr.(*Map).Example(0xc000077b00, 0xc0000abe60, 0x15e6cd4, 0xf)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/expr/types.go:623 +0x1c5
goa.design/goa/v3/expr.(*AttributeExpr).Example(0xc0000b0630, 0xc0000abe60, 0xf41c4c55, 0xb541ab3de336f28c)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/expr/example.go:70 +0x2b0
goa.design/goa/v3/expr.(*Object).Example(0xc0000be880, 0xc0000abe60, 0xc0000270c0, 0x1d)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/expr/types.go:578 +0x8f
goa.design/goa/v3/expr.(*UserTypeExpr).recExample(0xc0000d02a0, 0xc0000abe60, 0x1)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/expr/user_type.go:101 +0x11a
goa.design/goa/v3/expr.(*UserTypeExpr).Example(0xc0000d02a0, 0xc0000abe60, 0x15e6cd4, 0xf)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/expr/user_type.go:85 +0x35
goa.design/goa/v3/expr.(*AttributeExpr).Example(0xc0000b0210, 0xc0000abe60, 0xc000025220, 0xe)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/expr/example.go:70 +0x2b0
goa.design/goa/v3/grpc/codegen.buildInitData(0xc0000b0210, 0xc0002e2370, 0x15ca24f, 0x7, 0x15bddcb, 0x1, 0xc0000d03c0, 0x0, 0xc00009fe60, 0x28)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/grpc/codegen/service_data.go:993 +0x6d2
goa.design/goa/v3/grpc/codegen.buildRequestConvertData(0xc0000b0210, 0xc0002e2370, 0x0, 0x0, 0x0, 0xc0002b36c0, 0xc00009fe60, 0x1, 0xc000025250)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/grpc/codegen/service_data.go:806 +0x1a5
goa.design/goa/v3/grpc/codegen.ServicesData.analyze(0xc00027c2a0, 0xc0002d6d20, 0xb)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/grpc/codegen/service_data.go:523 +0xab8
goa.design/goa/v3/grpc/codegen.ServicesData.Get(0xc00027c2a0, 0x15d7c3e, 0xb, 0xc000237928)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/grpc/codegen/service_data.go:389 +0xbc
goa.design/goa/v3/grpc/codegen.protoFile(0xc000026a80, 0x12, 0xc0002d6d20, 0xc0000104a0)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/grpc/codegen/proto.go:29 +0x65
goa.design/goa/v3/grpc/codegen.ProtoFiles(0xc000026a80, 0x12, 0x199e120, 0x0, 0x0, 0x0)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/grpc/codegen/proto.go:23 +0xc5
goa.design/goa/v3/codegen/generator.Transport(0xc000026a80, 0x12, 0xc00028d740, 0x2, 0x2, 0x3, 0x0, 0x4, 0x0, 0x0)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/codegen/generator/transport.go:34 +0x570
goa.design/goa/v3/codegen/generator.Generate(0x7ffeefbfea06, 0x1, 0x15be6b6, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/amanickam/gocode/pkg/mod/goa.design/goa/v3@v3.2.3/codegen/generator/generate.go:82 +0x54b
main.main()
	/Users/amanickam/Projects/arangamani/goa-elem-issue/goa267690556/main.go:65 +0x285
```
2. The function `ValidateArrayOfString` is not called from anywhere and it is not actually validated.
3. If the field `foo` is not required, none of the validation functions are generated at all. The expectation I had is that the field will be validated if it is required. 

Pull request:
Fix gRPC validations
Make sure that validations are properly moved when wrapping data
structures for gRPC.

Fix #2674

Patch:
diff --git a/expr/attribute.go b/expr/attribute.go
index 49ba2084..087d750f 100644
--- a/expr/attribute.go
+++ b/expr/attribute.go
@@ -508,7 +508,8 @@ func (a *AttributeExpr) AddMeta(name string, vals ...string) {
 // Debug dumps the attribute to STDOUT in a goa developer friendly way.
 func (a *AttributeExpr) Debug(prefix string) { a.debug(prefix, make(map[*AttributeExpr]int), 0) }
 func (a *AttributeExpr) debug(prefix string, seen map[*AttributeExpr]int, indent int) {
-	prefix = strings.Repeat("  ", indent) + prefix
+	tab := strings.Repeat("  ", indent)
+	prefix = tab + prefix
 	if c, ok := seen[a]; ok && c > 1 {
 		fmt.Printf("%s: ...\n", prefix)
 		return
@@ -521,23 +522,23 @@ func (a *AttributeExpr) debug(prefix string, seen map[*AttributeExpr]int, indent
 		}
 	}
 	if a := AsArray(a.Type); a != nil {
-		a.ElemType.debug(" Elem", seen, indent+2)
+		a.ElemType.debug("elem", seen, indent+1)
 	}
 	if m := AsMap(a.Type); m != nil {
-		m.KeyType.debug(" Key", seen, indent+2)
-		m.ElemType.debug(" Elem", seen, indent+2)
+		m.KeyType.debug("key", seen, indent+1)
+		m.ElemType.debug("elem", seen, indent+1)
 	}
 	if d := a.DefaultValue; d != nil {
-		fmt.Printf("%s: default value\n", prefix)
-		fmt.Printf("%s%#v", strings.Repeat("  ", indent+1), a.DefaultValue)
+		fmt.Printf("%sdefault\n", tab)
+		fmt.Printf("%s%#v", tab+"  ", a.DefaultValue)
 	}
 	if v := a.Validation; v != nil {
 		v.Debug(indent + 1)
 	}
 	if len(a.Meta) > 0 {
-		fmt.Printf("%smeta\n", strings.Repeat(" ", indent+3))
+		fmt.Printf("%smeta\n", tab)
 		for k, v := range a.Meta {
-			fmt.Printf("%s- %s: %s\n", strings.Repeat(" ", indent+4), k, strings.Join(v, ", "))
+			fmt.Printf("%s- %s: %s\n", tab+"  ", k, strings.Join(v, ", "))
 		}
 	}
 	if t, ok := a.Type.(UserType); ok {
diff --git a/grpc/codegen/protobuf.go b/grpc/codegen/protobuf.go
index c5eb39c6..4f852f77 100644
--- a/grpc/codegen/protobuf.go
+++ b/grpc/codegen/protobuf.go
@@ -81,27 +81,20 @@ func makeProtoBufMessage(att *expr.AttributeExpr, tname string, sd *ServiceData)
 		}
 	}
 	n := ""
-	makeProtoBufMessageR(att, &n, sd)
+	makeProtoBufMessageR(att, &n, sd, make(map[string]struct{}))
 	return att
 }
 
 // makeProtoBufMessageR is the recursive implementation of makeProtoBufMessage.
-func makeProtoBufMessageR(att *expr.AttributeExpr, tname *string, sd *ServiceData, seen ...map[string]struct{}) {
+func makeProtoBufMessageR(att *expr.AttributeExpr, tname *string, sd *ServiceData, seen map[string]struct{}) {
 	ut, isut := att.Type.(expr.UserType)
 
 	// handle infinite recursions
 	if isut {
-		var s map[string]struct{}
-		if len(seen) > 0 {
-			s = seen[0]
-		} else {
-			s = make(map[string]struct{})
-			seen = append(seen, s)
-		}
-		if _, ok := s[ut.ID()]; ok {
+		if _, ok := seen[ut.ID()]; ok {
 			return
 		}
-		s[ut.ID()] = struct{}{}
+		seen[ut.ID()] = struct{}{}
 	}
 
 	wrap := func(att *expr.AttributeExpr, tname string) {
@@ -124,18 +117,18 @@ func makeProtoBufMessageR(att *expr.AttributeExpr, tname *string, sd *ServiceDat
 		if expr.IsArray(ut) {
 			wrapAttr(ut.Attribute(), ut.Name(), sd)
 		}
-		makeProtoBufMessageR(ut.Attribute(), tname, sd, seen...)
+		makeProtoBufMessageR(ut.Attribute(), tname, sd, seen)
 	case expr.IsArray(att.Type):
 		ar := expr.AsArray(att.Type)
-		makeProtoBufMessageR(ar.ElemType, tname, sd, seen...)
+		makeProtoBufMessageR(ar.ElemType, tname, sd, seen)
 		wrap(ar.ElemType, *tname)
 	case expr.IsMap(att.Type):
 		m := expr.AsMap(att.Type)
-		makeProtoBufMessageR(m.ElemType, tname, sd, seen...)
+		makeProtoBufMessageR(m.ElemType, tname, sd, seen)
 		wrap(m.ElemType, *tname)
 	case expr.IsObject(att.Type):
 		for _, nat := range *(expr.AsObject(att.Type)) {
-			makeProtoBufMessageR(nat.Attribute, tname, sd, seen...)
+			makeProtoBufMessageR(nat.Attribute, tname, sd, seen)
 		}
 	}
 }
@@ -143,14 +136,15 @@ func makeProtoBufMessageR(att *expr.AttributeExpr, tname *string, sd *ServiceDat
 // wrapAttr makes the attribute type a user type by wrapping the given
 // attribute into an attribute named "field".
 func wrapAttr(att *expr.AttributeExpr, tname string, sd *ServiceData) {
-	wrap := func(att *expr.AttributeExpr) *expr.AttributeExpr {
+	wrap := func(attr *expr.AttributeExpr) *expr.AttributeExpr {
 		return &expr.AttributeExpr{
 			Type: &expr.Object{
 				&expr.NamedAttributeExpr{
 					Name: "field",
 					Attribute: &expr.AttributeExpr{
-						Type: att.Type,
-						Meta: expr.MetaExpr{"rpc:tag": []string{"1"}},
+						Type:       attr.Type,
+						Meta:       expr.MetaExpr{"rpc:tag": []string{"1"}},
+						Validation: attr.Validation,
 					},
 				},
 			},
@@ -169,6 +163,8 @@ func wrapAttr(att *expr.AttributeExpr, tname string, sd *ServiceData) {
 			UID:           sd.Name + "#" + tname,
 		}
 	}
+	// Validation is moved to wrapped attribute.
+	att.Validation = nil
 }
 
 // unwrapAttr returns the attribute under the attribute name "field".
diff --git a/grpc/codegen/service_data.go b/grpc/codegen/service_data.go
index c67c4490..c2137e64 100644
--- a/grpc/codegen/service_data.go
+++ b/grpc/codegen/service_data.go
@@ -947,8 +947,6 @@ func buildResponseConvertData(response, result *expr.AttributeExpr, svcCtx *code
 //
 // proto if true indicates the target type is a protocol buffer type
 //
-// sd is the ServiceData
-//
 func buildInitData(source, target *expr.AttributeExpr, sourceVar, targetVar string, svcCtx *codegen.AttributeContext, proto bool, sd *ServiceData) *InitData {
 	var (
 		name     string
@@ -984,15 +982,13 @@ func buildInitData(source, target *expr.AttributeExpr, sourceVar, targetVar stri
 		}
 		sd.transformHelpers = codegen.AppendHelpers(sd.transformHelpers, helpers)
 		if (!proto && !isEmpty(source.Type)) || (proto && !isEmpty(target.Type)) {
-			args = []*InitArgData{
-				&InitArgData{
-					Name:     sourceVar,
-					Ref:      sourceVar,
-					TypeName: srcCtx.Scope.Name(source, srcCtx.Pkg, srcCtx.Pointer, srcCtx.UseDefault),
-					TypeRef:  srcCtx.Scope.Ref(source, srcCtx.Pkg),
-					Example:  source.Example(expr.Root.API.Random()),
-				},
-			}
+			args = []*InitArgData{{
+				Name:     sourceVar,
+				Ref:      sourceVar,
+				TypeName: srcCtx.Scope.Name(source, srcCtx.Pkg, srcCtx.Pointer, srcCtx.UseDefault),
+				TypeRef:  srcCtx.Scope.Ref(source, srcCtx.Pkg),
+				Example:  source.Example(expr.Root.API.Random()),
+			}}
 		}
 	}
 	return &InitData{


Test patch:
diff --git a/grpc/codegen/server_types_test.go b/grpc/codegen/server_types_test.go
index 1b35897d..707710a1 100644
--- a/grpc/codegen/server_types_test.go
+++ b/grpc/codegen/server_types_test.go
@@ -20,6 +20,7 @@ func TestServerTypeFiles(t *testing.T) {
 		{"payload-with-mixed-attributes", testdata.PayloadWithMixedAttributesDSL, testdata.PayloadWithMixedAttributesServerTypeCode},
 		{"result-collection", testdata.ResultWithCollectionDSL, testdata.ResultWithCollectionServerTypeCode},
 		{"with-errors", testdata.UnaryRPCWithErrorsDSL, testdata.WithErrorsServerTypeCode},
+		{"elem-validation", testdata.ElemValidationDSL, testdata.ElemValidationServerTypesFile},
 	}
 	for _, c := range cases {
 		t.Run(c.Name, func(t *testing.T) {
diff --git a/grpc/codegen/testdata/dsls.go b/grpc/codegen/testdata/dsls.go
index 780adf5a..bc69b58c 100644
--- a/grpc/codegen/testdata/dsls.go
+++ b/grpc/codegen/testdata/dsls.go
@@ -85,6 +85,20 @@ var UnaryRPCWithErrorsDSL = func() {
 	})
 }
 
+var ElemValidationDSL = func() {
+	var ResultType = Type("ResultType", func() {
+		Field(1, "foo", MapOf(String, ArrayOf(String)), func() {
+			Elem(func() { MinLength(1) })
+		})
+	})
+	Service("ServiceElemValidation", func() {
+		Method("MethodElemValidation", func() {
+			Payload(ResultType)
+			GRPC(func() {})
+		})
+	})
+}
+
 var UnaryRPCAcronymDSL = func() {
 	Service("ServiceUnaryRPCAcronym", func() {
 		Method("MethodUnaryRPCAcronym_jwt", func() {
diff --git a/grpc/codegen/testdata/server_type_code.go b/grpc/codegen/testdata/server_type_code.go
index 0039e51e..c85681c9 100644
--- a/grpc/codegen/testdata/server_type_code.go
+++ b/grpc/codegen/testdata/server_type_code.go
@@ -298,3 +298,52 @@ func NewMethodUnaryRPCWithErrorsCustomErrorError(er *serviceunaryrpcwitherrors.E
 	return message
 }
 `
+
+const ElemValidationServerTypesFile = `// NewMethodElemValidationPayload builds the payload of the
+// "MethodElemValidation" endpoint of the "ServiceElemValidation" service from
+// the gRPC request type.
+func NewMethodElemValidationPayload(message *service_elem_validationpb.MethodElemValidationRequest) *serviceelemvalidation.ResultType {
+	v := &serviceelemvalidation.ResultType{}
+	if message.Foo != nil {
+		v.Foo = make(map[string][]string, len(message.Foo))
+		for key, val := range message.Foo {
+			tk := key
+			tv := make([]string, len(val.Field))
+			for i, val := range val.Field {
+				tv[i] = val
+			}
+			v.Foo[tk] = tv
+		}
+	}
+	return v
+}
+
+// NewMethodElemValidationResponse builds the gRPC response type from the
+// result of the "MethodElemValidation" endpoint of the "ServiceElemValidation"
+// service.
+func NewMethodElemValidationResponse() *service_elem_validationpb.MethodElemValidationResponse {
+	message := &service_elem_validationpb.MethodElemValidationResponse{}
+	return message
+}
+
+// ValidateMethodElemValidationRequest runs the validations defined on
+// MethodElemValidationRequest.
+func ValidateMethodElemValidationRequest(message *service_elem_validationpb.MethodElemValidationRequest) (err error) {
+	for _, v := range message.Foo {
+		if v != nil {
+			if err2 := ValidateArrayOfString(v); err2 != nil {
+				err = goa.MergeErrors(err, err2)
+			}
+		}
+	}
+	return
+}
+
+// ValidateArrayOfString runs the validations defined on ArrayOfString.
+func ValidateArrayOfString(message *service_elem_validationpb.ArrayOfString) (err error) {
+	if len(message.Field) < 1 {
+		err = goa.MergeErrors(err, goa.InvalidLengthError("message.field", message.Field, len(message.Field), 1, true))
+	}
+	return
+}
+`