Repository: goadesign/goa (go)

Issue:
HTTP clients don't generate correctly when using a CommonType
When we generate our HTTP client from a design that uses a `CommonType`, the http `types.go` files still looks in the specific service package, instead of common.

e.g.
```
// NewBuildApplicableFieldsResponseBody builds the HTTP response body from the
// result of the "BuildApplicableFields" endpoint of the "Incidents" service.
func NewBuildApplicableFieldsResponseBody(res *incidents.IncidentApplicableFields) *BuildApplicableFieldsResponseBody {
	body := &BuildApplicableFieldsResponseBody{}
	return body
}
```

Should be 
```
// NewBuildApplicableFieldsResponseBody builds the HTTP response body from the
// result of the "BuildApplicableFields" endpoint of the "Incidents" service.
func NewBuildApplicableFieldsResponseBody(res *common.IncidentApplicableFields) *BuildApplicableFieldsResponseBody {
	body := &BuildApplicableFieldsResponseBody{}
	return body
}
```

Pull request:
Fix use of custom packages in error types
Fix #3099 

Patch:
diff --git a/codegen/service/service_data.go b/codegen/service/service_data.go
index 695c2ea1..b9706e70 100644
--- a/codegen/service/service_data.go
+++ b/codegen/service/service_data.go
@@ -166,6 +166,9 @@ type (
 		ResultEx interface{}
 		// Errors list the possible errors defined in the design if any.
 		Errors []*ErrorInitData
+		// ErrorLocs lists the file and Go package of the error type
+		// if overridden via Meta indexed by error name.
+		ErrorLocs map[string]*codegen.Location
 		// Requirements contains the security requirements for the
 		// method.
 		Requirements RequirementsData
@@ -468,6 +471,9 @@ func (d *Data) initUserTypeImports(genpkg string) {
 	for _, m := range d.Methods {
 		initLoc(m.PayloadLoc)
 		initLoc(m.ResultLoc)
+		for _, l := range m.ErrorLocs {
+			initLoc(l)
+		}
 		for _, ut := range d.userTypes {
 			initLoc(ut.Loc)
 		}
@@ -885,6 +891,7 @@ func buildMethodData(m *expr.MethodExpr, scope *codegen.NameScope) *MethodData {
 		resultDesc  string
 		resultEx    interface{}
 		errors      []*ErrorInitData
+		errorLocs   map[string]*codegen.Location
 		reqs        RequirementsData
 		schemes     SchemesData
 	)
@@ -923,8 +930,10 @@ func buildMethodData(m *expr.MethodExpr, scope *codegen.NameScope) *MethodData {
 	}
 	if len(m.Errors) > 0 {
 		errors = make([]*ErrorInitData, len(m.Errors))
+		errorLocs = make(map[string]*codegen.Location, len(m.Errors))
 		for i, er := range m.Errors {
 			errors[i] = buildErrorInitData(er, scope)
+			errorLocs[er.Name] = codegen.UserTypeLocation(er.AttributeExpr.Type)
 		}
 	}
 	for _, req := range m.Requirements {
@@ -958,6 +967,7 @@ func buildMethodData(m *expr.MethodExpr, scope *codegen.NameScope) *MethodData {
 		ResultDesc:                   resultDesc,
 		ResultEx:                     resultEx,
 		Errors:                       errors,
+		ErrorLocs:                    errorLocs,
 		Requirements:                 reqs,
 		Schemes:                      schemes,
 		StreamKind:                   m.Stream,
@@ -1223,7 +1233,6 @@ func collectProjectedTypes(projected, att *expr.AttributeExpr, viewspkg string,
 // buildProjectedType builds projected type for the given user type.
 //
 // viewspkg is the name of the views package
-//
 func buildProjectedType(projected, att *expr.AttributeExpr, viewspkg string, scope, viewScope *codegen.NameScope) *ProjectedTypeData {
 	var (
 		projections []*InitData
@@ -1693,7 +1702,6 @@ func buildValidations(projected *expr.AttributeExpr, scope *codegen.NameScope) [
 // target data structures in the transformation code.
 //
 // view is used to generate the constructor function name.
-//
 func buildConstructorCode(src, tgt *expr.AttributeExpr, sourceVar, targetVar string, sourceCtx, targetCtx *codegen.AttributeContext, view string) (string, []*codegen.TransformFunctionData) {
 	var (
 		helpers []*codegen.TransformFunctionData
diff --git a/expr/method.go b/expr/method.go
index 4d6a422f..95d57156 100644
--- a/expr/method.go
+++ b/expr/method.go
@@ -284,6 +284,18 @@ func (m *MethodExpr) Finalize() {
 			rt.Finalize()
 		}
 	}
+	for _, e := range m.Service.Errors {
+		found := false
+		for _, f := range m.Errors {
+			if e.Name == f.Name {
+				found = true
+				break
+			}
+		}
+		if !found {
+			m.Errors = append(m.Errors, e)
+		}
+	}
 	for _, e := range m.Errors {
 		e.Finalize()
 	}
diff --git a/grpc/codegen/service_data.go b/grpc/codegen/service_data.go
index e1ec125a..312683dd 100644
--- a/grpc/codegen/service_data.go
+++ b/grpc/codegen/service_data.go
@@ -829,10 +829,10 @@ func collectValidations(att *expr.AttributeExpr, ctx *codegen.AttributeContext,
 
 // buildRequestConvertData builds the convert data for the server and client
 // requests.
-//    * server side - converts generated gRPC request type in *.pb.go and the
-//      gRPC  metadata to method payload type.
-//    * client side - converts method payload type to generated gRPC request
-//      type in *.pb.go.
+//   - server side - converts generated gRPC request type in *.pb.go and the
+//     gRPC  metadata to method payload type.
+//   - client side - converts method payload type to generated gRPC request
+//     type in *.pb.go.
 //
 // svr param indicates that the convert data is generated for server side.
 func buildRequestConvertData(request, payload *expr.AttributeExpr, md []*MetadataData, e *expr.GRPCEndpointExpr, sd *ServiceData, svr bool) *ConvertData {
@@ -906,10 +906,10 @@ func buildRequestConvertData(request, payload *expr.AttributeExpr, md []*Metadat
 
 // buildResponseConvertData builds the convert data for the server and client
 // responses.
-//     * server side - converts method result type to generated gRPC response
-//       type in *.pb.go
-//     * client side - converts generated gRPC response type in *.pb.go and
-//       response metadata to method result type.
+//   - server side - converts method result type to generated gRPC response
+//     type in *.pb.go
+//   - client side - converts generated gRPC response type in *.pb.go and
+//     response metadata to method result type.
 //
 // svr param indicates that the convert data is generated for server side.
 func buildResponseConvertData(response, result *expr.AttributeExpr, svcCtx *codegen.AttributeContext, hdrs, trlrs []*MetadataData, e *expr.GRPCEndpointExpr, sd *ServiceData, svr bool) *ConvertData {
@@ -999,7 +999,6 @@ func buildResponseConvertData(response, result *expr.AttributeExpr, svcCtx *code
 // svcCtx is the attribute context for service type
 //
 // proto if true indicates the target type is a protocol buffer type
-//
 func buildInitData(source, target *expr.AttributeExpr, sourceVar, targetVar string, svcCtx *codegen.AttributeContext, proto bool, sd *ServiceData) *InitData {
 	var (
 		name     string
@@ -1077,9 +1076,10 @@ func buildErrorsData(e *expr.GRPCEndpointExpr, sd *ServiceData) []*ErrorData {
 				ClientConvert: buildErrorConvertData(v, e, sd, false),
 			}
 		}
+		errorLoc := svc.Method(e.MethodExpr.Name).ErrorLocs[v.Name]
 		errors = append(errors, &ErrorData{
 			Name:     v.Name,
-			Ref:      svc.Scope.GoFullTypeRef(v.ErrorExpr.AttributeExpr, svc.PkgName),
+			Ref:      svc.Scope.GoFullTypeRef(v.ErrorExpr.AttributeExpr, pkgWithDefault(errorLoc, svc.PkgName)),
 			Response: responseData,
 		})
 	}
diff --git a/http/codegen/service_data.go b/http/codegen/service_data.go
index 7b8397be..e2254fa1 100644
--- a/http/codegen/service_data.go
+++ b/http/codegen/service_data.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"fmt"
 	"net/http"
-	"path"
 	"sort"
 	"strconv"
 	"strings"
@@ -917,7 +916,6 @@ func (d ServicesData) analyze(hs *expr.HTTPServiceExpr) *ServiceData {
 //
 // * removes aliased user type by replacing them with the underlying type.
 // * changes unions into structs with Type and Value fields.
-//
 func makeHTTPType(att *expr.AttributeExpr) *expr.AttributeExpr {
 	if att == nil {
 		return nil
@@ -1483,16 +1481,15 @@ func buildResultData(e *expr.HTTPEndpointExpr, sd *ServiceData) *ResultData {
 func buildResponses(e *expr.HTTPEndpointExpr, result *expr.AttributeExpr, viewed bool, sd *ServiceData) []*ResponseData {
 	var (
 		responses []*ResponseData
-		scope     *codegen.NameScope
 
 		svc        = sd.Service
 		md         = svc.Method(e.Name())
 		pkg        = pkgWithDefault(md.ResultLoc, svc.PkgName)
 		httpclictx = httpContext("", sd.Scope, false, false)
+		scope      = svc.Scope
 		svcctx     = serviceContext(pkg, sd.Service.Scope)
 	)
 	{
-		scope = svc.Scope
 		if viewed {
 			scope = svc.ViewScope
 			svcctx = viewContext(sd.Service.ViewsPkg, sd.Service.ViewScope)
@@ -1535,14 +1532,14 @@ func buildResponses(e *expr.HTTPEndpointExpr, result *expr.AttributeExpr, viewed
 					if origin != "" {
 						// Response body is explicitly set to an attribute in the method
 						// result type. No need to do any view-based projections server side.
-						if sbd := buildResponseBodyType(resp.Body, result, e, true, &vname, sd); sbd != nil {
+						if sbd := buildResponseBodyType(resp.Body, result, md.ResultLoc, e, true, &vname, sd); sbd != nil {
 							serverBodyData = append(serverBodyData, sbd)
 						}
 					} else if v, ok := e.MethodExpr.Result.Meta["view"]; ok && len(v) > 0 {
 						// Design explicitly sets the view to render the result.
 						// We generate only one server body type which will be rendered
 						// using the specified view.
-						if sbd := buildResponseBodyType(resp.Body, result, e, true, &v[0], sd); sbd != nil {
+						if sbd := buildResponseBodyType(resp.Body, result, md.ResultLoc, e, true, &v[0], sd); sbd != nil {
 							serverBodyData = append(serverBodyData, sbd)
 						}
 					} else {
@@ -1555,17 +1552,17 @@ func buildResponses(e *expr.HTTPEndpointExpr, result *expr.AttributeExpr, viewed
 						// attributes defined in the view in the response (NOTE: a required
 						// attribute in the result type may not be present in all its views)
 						for _, view := range md.ViewedResult.Views {
-							if sbd := buildResponseBodyType(resp.Body, result, e, true, &view.Name, sd); sbd != nil {
+							if sbd := buildResponseBodyType(resp.Body, result, md.ResultLoc, e, true, &view.Name, sd); sbd != nil {
 								serverBodyData = append(serverBodyData, sbd)
 							}
 						}
 					}
-					clientBodyData = buildResponseBodyType(resp.Body, result, e, false, &vname, sd)
+					clientBodyData = buildResponseBodyType(resp.Body, result, md.ResultLoc, e, false, &vname, sd)
 				} else {
-					if sbd := buildResponseBodyType(resp.Body, result, e, true, nil, sd); sbd != nil {
+					if sbd := buildResponseBodyType(resp.Body, result, md.ResultLoc, e, true, nil, sd); sbd != nil {
 						serverBodyData = append(serverBodyData, sbd)
 					}
-					clientBodyData = buildResponseBodyType(resp.Body, result, e, false, nil, sd)
+					clientBodyData = buildResponseBodyType(resp.Body, result, md.ResultLoc, e, false, nil, sd)
 				}
 				if clientBodyData != nil {
 					sd.ClientTypeNames[clientBodyData.Name] = false
@@ -1757,6 +1754,7 @@ func buildResponses(e *expr.HTTPEndpointExpr, result *expr.AttributeExpr, viewed
 func buildErrorsData(e *expr.HTTPEndpointExpr, sd *ServiceData) []*ErrorGroupData {
 	var (
 		svc        = sd.Service
+		ep         = svc.Method(e.MethodExpr.Name)
 		httpclictx = httpContext("", sd.Scope, false, false)
 	)
 
@@ -1768,12 +1766,7 @@ func buildErrorsData(e *expr.HTTPEndpointExpr, sd *ServiceData) []*ErrorGroupDat
 			body = v.Response.Body.Type
 		)
 
-		pkg := sd.Service.PkgName
-		if ut, ok := v.ErrorExpr.Type.(expr.UserType); ok {
-			if p, ok := ut.Attribute().Meta.Last("struct:pkg:path"); ok && p != "" {
-				pkg = codegen.Goify(path.Base(p), false)
-			}
-		}
+		pkg := pkgWithDefault(ep.ErrorLocs[v.Name], svc.PkgName)
 		errctx := serviceContext(pkg, sd.Service.Scope)
 
 		if needInit(v.ErrorExpr.Type) {
@@ -1784,7 +1777,6 @@ func buildErrorsData(e *expr.HTTPEndpointExpr, sd *ServiceData) []*ErrorGroupDat
 				args     []*InitArgData
 			)
 			{
-				ep := svc.Method(e.MethodExpr.Name)
 				name = fmt.Sprintf("New%s%s", codegen.Goify(ep.Name, true), codegen.Goify(v.ErrorExpr.Name, true))
 				desc = fmt.Sprintf("%s builds a %s service %s endpoint %s error.",
 					name, svc.Name, e.Name(), v.ErrorExpr.Name)
@@ -1887,10 +1879,11 @@ func buildErrorsData(e *expr.HTTPEndpointExpr, sd *ServiceData) []*ErrorGroupDat
 				clientBodyData *TypeData
 			)
 			{
-				if sbd := buildResponseBodyType(v.Response.Body, v.ErrorExpr.AttributeExpr, e, true, nil, sd); sbd != nil {
+				errorLoc := ep.ErrorLocs[v.ErrorExpr.Name]
+				if sbd := buildResponseBodyType(v.Response.Body, v.ErrorExpr.AttributeExpr, errorLoc, e, true, nil, sd); sbd != nil {
 					serverBodyData = append(serverBodyData, sbd)
 				}
-				clientBodyData = buildResponseBodyType(v.Response.Body, v.ErrorExpr.AttributeExpr, e, false, nil, sd)
+				clientBodyData = buildResponseBodyType(v.Response.Body, v.ErrorExpr.AttributeExpr, errorLoc, e, false, nil, sd)
 				if clientBodyData != nil {
 					sd.ClientTypeNames[clientBodyData.Name] = false
 					clientBodyData.Description = fmt.Sprintf("%s is the type of the %q service %q endpoint HTTP response body for the %q error.",
@@ -1985,7 +1978,6 @@ func buildErrorsData(e *expr.HTTPEndpointExpr, sd *ServiceData) []*ErrorGroupDat
 // svr is true if the function is generated for server side code.
 //
 // sd is the service data
-//
 func buildRequestBodyType(body, att *expr.AttributeExpr, e *expr.HTTPEndpointExpr, svr bool, sd *ServiceData) *TypeData {
 	if body.Type == expr.Empty {
 		return nil
@@ -2115,8 +2107,7 @@ func buildRequestBodyType(body, att *expr.AttributeExpr, e *expr.HTTPEndpointExp
 // svr is true if the function is generated for server side code
 //
 // view is the view name to add as a suffix to the type name.
-//
-func buildResponseBodyType(body, att *expr.AttributeExpr, e *expr.HTTPEndpointExpr, svr bool, view *string, sd *ServiceData) *TypeData {
+func buildResponseBodyType(body, att *expr.AttributeExpr, loc *codegen.Location, e *expr.HTTPEndpointExpr, svr bool, view *string, sd *ServiceData) *TypeData {
 	if body.Type == expr.Empty {
 		return nil
 	}
@@ -2133,8 +2124,7 @@ func buildResponseBodyType(body, att *expr.AttributeExpr, e *expr.HTTPEndpointEx
 
 		svc     = sd.Service
 		httpctx = httpContext("", sd.Scope, false, svr)
-		ep      = sd.Service.Method(e.Name())
-		pkg     = pkgWithDefault(ep.PayloadLoc, sd.Service.PkgName)
+		pkg     = pkgWithDefault(loc, sd.Service.PkgName)
 		svcctx  = serviceContext(pkg, sd.Service.Scope)
 	)
 	{
@@ -2650,7 +2640,6 @@ func attributeTypeData(ut expr.UserType, req, ptr, server bool, rd *ServiceData)
 // type
 //
 // svr if true indicates that the type is a server type, else client type
-//
 func httpContext(pkg string, scope *codegen.NameScope, request, svr bool) *codegen.AttributeContext {
 	marshal := !request && svr || request && !svr
 	return codegen.NewAttributeContext(!marshal, false, marshal, pkg, scope)
@@ -2686,7 +2675,6 @@ func pkgWithDefault(loc *codegen.Location, def string) string {
 // the transformation code
 //
 // sourceCtx, targetCtx are the source and target attribute contexts
-//
 func unmarshal(source, target *expr.AttributeExpr, sourceVar, targetVar string, sourceCtx, targetCtx *codegen.AttributeContext) (string, []*codegen.TransformFunctionData, error) {
 	return codegen.GoTransform(source, target, sourceVar, targetVar, sourceCtx, targetCtx, "unmarshal", true)
 }
@@ -2701,7 +2689,6 @@ func unmarshal(source, target *expr.AttributeExpr, sourceVar, targetVar string,
 // the transformation code
 //
 // sourceCtx, targetCtx are the source and target attribute contexts
-//
 func marshal(source, target *expr.AttributeExpr, sourceVar, targetVar string, sourceCtx, targetCtx *codegen.AttributeContext) (string, []*codegen.TransformFunctionData, error) {
 	return codegen.GoTransform(source, target, sourceVar, targetVar, sourceCtx, targetCtx, "marshal", true)
 }


Test patch:
diff --git a/http/codegen/server_types_test.go b/http/codegen/server_types_test.go
index 3e56d225..14923850 100644
--- a/http/codegen/server_types_test.go
+++ b/http/codegen/server_types_test.go
@@ -23,6 +23,7 @@ func TestServerTypes(t *testing.T) {
 		{"with-result-collection", testdata.ResultWithResultCollectionDSL, ResultWithResultCollectionServerTypesFile},
 		{"with-result-view", testdata.ResultWithResultViewDSL, ResultWithResultViewServerTypesFile},
 		{"empty-error-response-body", testdata.EmptyErrorResponseBodyDSL, ""},
+		{"with-error-custom-pkg", testdata.WithErrorCustomPkgDSL, WithErrorCustomPkgServerTypesFile},
 	}
 	for _, c := range cases {
 		t.Run(c.Name, func(t *testing.T) {
@@ -302,3 +303,21 @@ func NewMethodResultWithResultViewResponseBodyFull(res *serviceresultwithresultv
 	return body
 }
 `
+
+const WithErrorCustomPkgServerTypesFile = `// MethodWithErrorCustomPkgErrorNameResponseBody is the type of the
+// "ServiceWithErrorCustomPkg" service "MethodWithErrorCustomPkg" endpoint HTTP
+// response body for the "error_name" error.
+type MethodWithErrorCustomPkgErrorNameResponseBody struct {
+	Name string ` + "`" + `form:"name" json:"name" xml:"name"` + "`" + `
+}
+
+// NewMethodWithErrorCustomPkgErrorNameResponseBody builds the HTTP response
+// body from the result of the "MethodWithErrorCustomPkg" endpoint of the
+// "ServiceWithErrorCustomPkg" service.
+func NewMethodWithErrorCustomPkgErrorNameResponseBody(res *custom.CustomError) *MethodWithErrorCustomPkgErrorNameResponseBody {
+	body := &MethodWithErrorCustomPkgErrorNameResponseBody{
+		Name: res.Name,
+	}
+	return body
+}
+`
diff --git a/http/codegen/testdata/result_dsls.go b/http/codegen/testdata/result_dsls.go
index 90dfa11b..2c6cff88 100644
--- a/http/codegen/testdata/result_dsls.go
+++ b/http/codegen/testdata/result_dsls.go
@@ -874,6 +874,23 @@ var EmptyErrorResponseBodyDSL = func() {
 	})
 }
 
+var WithErrorCustomPkgDSL = func() {
+	var CustomError = Type("CustomError", func() {
+		Meta("struct:pkg:path", "custom")
+		ErrorName("name")
+		Required("name")
+	})
+	Service("ServiceWithErrorCustomPkg", func() {
+		Method("MethodWithErrorCustomPkg", func() {
+			Error("error_name", CustomError)
+			HTTP(func() {
+				GET("/")
+				Response("error_name", StatusBadRequest)
+			})
+		})
+	})
+}
+
 var EmptyCustomErrorResponseBodyDSL = func() {
 	var ErrorType = Type("Error", func() {
 		Attribute("err", String)