Repository: goadesign/goa
Pull request #3022 (2022-03-04 09:38:49)

pkg:path types that reference their own package types
If you have a user type placed in a package, ie. common, and it has a
field of another type in package common, we codegen a type reference of
common.Thing which will fail, as that code is already in the common
package.

lawrencejones: @raphael this is a failing test for what we were discussing in Slack!

Patch:
diff --git a/codegen/scope.go b/codegen/scope.go
index a1cc7133..b2140ba2 100644
--- a/codegen/scope.go
+++ b/codegen/scope.go
@@ -96,6 +96,10 @@ func (s *NameScope) Name(name string) string {
 // useDefault if true indicates that the attribute must not be a pointer
 // if it has a default value.
 func (s *NameScope) GoTypeDef(att *expr.AttributeExpr, ptr, useDefault bool) string {
+	return s.goTypeDef(att, ptr, useDefault, "")
+}
+
+func (s *NameScope) goTypeDef(att *expr.AttributeExpr, ptr, useDefault bool, pkg string) string {
 	switch actual := att.Type.(type) {
 	case expr.Primitive:
 		if t, _ := GetMetaType(att); t != "" {
@@ -103,17 +107,17 @@ func (s *NameScope) GoTypeDef(att *expr.AttributeExpr, ptr, useDefault bool) str
 		}
 		return GoNativeTypeName(actual)
 	case *expr.Array:
-		d := s.GoTypeDef(actual.ElemType, ptr, useDefault)
+		d := s.goTypeDef(actual.ElemType, ptr, useDefault, pkg)
 		if expr.IsObject(actual.ElemType.Type) {
 			d = "*" + d
 		}
 		return "[]" + d
 	case *expr.Map:
-		keyDef := s.GoTypeDef(actual.KeyType, ptr, useDefault)
+		keyDef := s.goTypeDef(actual.KeyType, ptr, useDefault, pkg)
 		if expr.IsObject(actual.KeyType.Type) {
 			keyDef = "*" + keyDef
 		}
-		elemDef := s.GoTypeDef(actual.ElemType, ptr, useDefault)
+		elemDef := s.goTypeDef(actual.ElemType, ptr, useDefault, pkg)
 		if expr.IsObject(actual.ElemType.Type) {
 			elemDef = "*" + elemDef
 		}
@@ -133,7 +137,13 @@ func (s *NameScope) GoTypeDef(att *expr.AttributeExpr, ptr, useDefault bool) str
 			)
 			{
 				fn = GoifyAtt(at, name, true)
-				tdef = s.GoTypeDef(at, ptr, useDefault)
+				var parentPkg string
+				if ut, ok := at.Type.(expr.UserType); ok {
+					if UserTypeLocation(ut) != nil {
+						parentPkg = UserTypeLocation(ut).PackageName()
+					}
+				}
+				tdef = s.goTypeDef(at, ptr, useDefault, parentPkg)
 				if expr.IsObject(at.Type) ||
 					att.IsPrimitivePointer(name, useDefault) ||
 					(ptr && expr.IsPrimitive(at.Type) && at.Type.Kind() != expr.AnyKind && at.Type.Kind() != expr.BytesKind) {
@@ -150,7 +160,7 @@ func (s *NameScope) GoTypeDef(att *expr.AttributeExpr, ptr, useDefault bool) str
 		return strings.Join(ss, "\n")
 	case expr.UserType:
 		var prefix string
-		if loc := UserTypeLocation(actual); loc != nil {
+		if loc := UserTypeLocation(actual); loc != nil && loc.PackageName() != pkg {
 			prefix = loc.PackageName() + "."
 		}
 		return prefix + s.GoTypeName(att)


Test patch:
diff --git a/codegen/service/service_test.go b/codegen/service/service_test.go
index 58a132c0..c607d8ff 100644
--- a/codegen/service/service_test.go
+++ b/codegen/service/service_test.go
@@ -69,6 +69,7 @@ func TestService(t *testing.T) {
 
 func TestStructPkgPath(t *testing.T) {
 	fooPath := filepath.Join("gen", "foo", "foo.go")
+	recursiveFooPath := filepath.Join("gen", "foo", "recursive_foo.go")
 	barPath := filepath.Join("gen", "bar", "bar.go")
 	bazPath := filepath.Join("gen", "baz", "baz.go")
 	cases := []struct {
@@ -80,6 +81,7 @@ func TestStructPkgPath(t *testing.T) {
 	}{
 		{"none", testdata.SingleMethodDSL, []string{testdata.SingleMethod}, nil, nil},
 		{"single", testdata.PkgPathDSL, []string{testdata.PkgPath}, []string{fooPath}, []string{testdata.PkgPathFoo}},
+		{"recursive", testdata.PkgPathRecursiveDSL, []string{testdata.PkgPathRecursive}, []string{fooPath, recursiveFooPath}, []string{testdata.PkgPathRecursiveFooFoo, testdata.PkgPathRecursiveFoo}},
 		{"multiple", testdata.PkgPathMultipleDSL, []string{testdata.PkgPathMultiple}, []string{barPath, bazPath}, []string{testdata.PkgPathBar, testdata.PkgPathBaz}},
 		{"nopkg", testdata.PkgPathNoDirDSL, []string{testdata.PkgPathNoDir}, nil, nil},
 		{"dupes", testdata.PkgPathDupeDSL, []string{testdata.PkgPathDupe1, testdata.PkgPathDupe2}, []string{fooPath}, []string{testdata.PkgPathFooDupe}},
@@ -88,7 +90,7 @@ func TestStructPkgPath(t *testing.T) {
 		t.Run(c.Name, func(t *testing.T) {
 			userTypePkgs := make(map[string][]string)
 			codegen.RunDSLWithFunc(t, c.DSL, func() {
-				expr.Root.Types = []expr.UserType{testdata.APayload, testdata.AResult, testdata.Foo, testdata.Bar, testdata.Baz, testdata.NoDir}
+				expr.Root.Types = []expr.UserType{testdata.APayload, testdata.AResult, testdata.RecursiveFoo, testdata.Foo, testdata.Bar, testdata.Baz, testdata.NoDir}
 			})
 			if len(expr.Root.Services) != len(c.SvcCodes) {
 				t.Fatalf("got %d services, expected %d", len(expr.Root.Services), len(c.SvcCodes))
diff --git a/codegen/service/testdata/service_code.go b/codegen/service/testdata/service_code.go
index 1a85300f..4e81bfb7 100644
--- a/codegen/service/testdata/service_code.go
+++ b/codegen/service/testdata/service_code.go
@@ -2392,6 +2392,24 @@ const ServiceName = "PkgPathMethod"
 var MethodNames = [1]string{"A"}
 `
 
+const PkgPathRecursive = `
+// Service is the PkgPathRecursiveMethod service interface.
+type Service interface {
+	// A implements A.
+	A(context.Context, *foo.RecursiveFoo) (res *foo.RecursiveFoo, err error)
+}
+
+// ServiceName is the name of the service as defined in the design. This is the
+// same value that is set in the endpoint request contexts under the ServiceKey
+// key.
+const ServiceName = "PkgPathRecursiveMethod"
+
+// MethodNames lists the service method names as defined in the design. These
+// are the same values that are set in the endpoint request contexts under the
+// MethodKey key.
+var MethodNames = [1]string{"A"}
+`
+
 const PkgPathMultiple = `
 // Service is the MultiplePkgPathMethod service interface.
 type Service interface {
@@ -2416,13 +2434,13 @@ var MethodNames = [3]string{"A", "B", "EnvelopedB"}
 // EnvelopedBPayload is the payload type of the MultiplePkgPathMethod service
 // EnvelopedB method.
 type EnvelopedBPayload struct {
-	Baz *baz.Baz
+	Baz *Baz
 }
 
 // EnvelopedBResult is the result type of the MultiplePkgPathMethod service
 // EnvelopedB method.
 type EnvelopedBResult struct {
-	Baz *baz.Baz
+	Baz *Baz
 }
 `
 
@@ -2432,6 +2450,19 @@ type Foo struct {
 }
 `
 
+const PkgPathRecursiveFooFoo = `
+type Foo struct {
+	IntField *int
+}
+`
+
+const PkgPathRecursiveFoo = `// RecursiveFoo is the payload type of the PkgPathRecursiveMethod service A
+// method.
+type RecursiveFoo struct {
+	Foo *Foo
+}
+`
+
 const PkgPathBar = `// Bar is the payload type of the MultiplePkgPathMethod service A method.
 type Bar struct {
 	IntField *int
diff --git a/codegen/service/testdata/service_dsls.go b/codegen/service/testdata/service_dsls.go
index fab2ea78..95d89fc8 100644
--- a/codegen/service/testdata/service_dsls.go
+++ b/codegen/service/testdata/service_dsls.go
@@ -612,6 +612,11 @@ var Foo = Type("Foo", func() {
 	Meta("struct:pkg:path", "foo")
 })
 
+var RecursiveFoo = Type("RecursiveFoo", func() {
+	Attribute("Foo", Foo)
+	Meta("struct:pkg:path", "foo")
+})
+
 var Bar = Type("Bar", func() {
 	Attribute("IntField", Int)
 	Meta("struct:pkg:path", "bar")
@@ -636,6 +641,15 @@ var PkgPathDSL = func() {
 	})
 }
 
+var PkgPathRecursiveDSL = func() {
+	Service("PkgPathRecursiveMethod", func() {
+		Method("A", func() {
+			Payload(RecursiveFoo)
+			Result(RecursiveFoo)
+		})
+	})
+}
+
 var PkgPathMultipleDSL = func() {
 	Service("MultiplePkgPathMethod", func() {
 		Method("A", func() {