From 408bcf915396318261b06407a3f2cd37bf5ff7f0 Mon Sep 17 00:00:00 2001
From: Alec Thomas <aat@tbd.email>
Date: Thu, 8 Feb 2024 07:12:14 +1100
Subject: [PATCH] chore: implement DataRef.schemaChildren() (#893)

It was returning nil previously, resulting in the Visit() function never
visiting the type parameters and all sorts of things failing!
---
 backend/schema/dataref.go     | 10 ++++--
 backend/schema/module.go      |  8 -----
 backend/schema/schema_test.go | 66 +++++++++--------------------------
 3 files changed, 24 insertions(+), 60 deletions(-)

diff --git a/backend/schema/dataref.go b/backend/schema/dataref.go
index fdef34431d..394eb92200 100644
--- a/backend/schema/dataref.go
+++ b/backend/schema/dataref.go
@@ -52,8 +52,14 @@ func (d *DataRef) ToProto() protoreflect.ProtoMessage {
 	}
 }
 
-func (*DataRef) schemaChildren() []Node { return nil }
-func (*DataRef) schemaType()            {}
+func (d *DataRef) schemaChildren() []Node {
+	out := make([]Node, 0, len(d.TypeParameters))
+	for _, t := range d.TypeParameters {
+		out = append(out, t)
+	}
+	return out
+}
+func (*DataRef) schemaType() {}
 
 func ParseDataRef(ref string) (*DataRef, error) {
 	return dataRefParser.ParseString("", ref)
diff --git a/backend/schema/module.go b/backend/schema/module.go
index 571102afc8..85bc6dfc81 100644
--- a/backend/schema/module.go
+++ b/backend/schema/module.go
@@ -149,14 +149,6 @@ func (m *Module) Imports() []string {
 			if n.Module != "" && n.Module != m.Name {
 				imports[n.Module] = true
 			}
-			// add imports for type parameters DataRefs as well
-			for _, p := range n.TypeParameters {
-				if p, ok := p.(*DataRef); ok {
-					if p.Module != "" && p.Module != m.Name {
-						imports[p.Module] = true
-					}
-				}
-			}
 
 		case *VerbRef:
 			if n.Module != "" && n.Module != m.Name {
diff --git a/backend/schema/schema_test.go b/backend/schema/schema_test.go
index 35d7b5f60f..10e9a631a8 100644
--- a/backend/schema/schema_test.go
+++ b/backend/schema/schema_test.go
@@ -63,11 +63,13 @@ func TestImports(t *testing.T) {
 			ref another.Data
 			ref Generic<new.Data>
 		}
+		verb myVerb(test.Data) test.Data
+			calls verbose.verb
 	}
 	`
 	schema, err := ParseModuleString("", input)
 	assert.NoError(t, err)
-	assert.Equal(t, []string{"another", "new", "other"}, schema.Imports())
+	assert.Equal(t, []string{"another", "new", "other", "verbose"}, schema.Imports())
 }
 
 func TestVisit(t *testing.T) {
@@ -98,7 +100,9 @@ Module
       VerbRef
   Verb
     DataRef
+      DataRef
     DataRef
+      DataRef
     MetadataIngress
       IngressPathLiteral
       IngressPathLiteral
@@ -115,7 +119,7 @@ Module
 		return next()
 	})
 	assert.NoError(t, err)
-	assert.Equal(t, normaliseString(expected), normaliseString(actual.String()), "%s", actual.String())
+	assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual.String()), "%s", actual.String())
 }
 
 func TestParserRoundTrip(t *testing.T) {
@@ -188,35 +192,15 @@ func TestParsing(t *testing.T) {
 			input:  `module int { data String { name String } verb verb(String) String }`,
 			errors: []string{"1:14: data structure name \"String\" is a reserved word"}},
 		{name: "BuiltinRef",
-			input: `module test { verb myIngress(HttpRequest<string>) HttpResponse<string> }`,
+			input: `module test { verb myIngress(HttpRequest<String>) HttpResponse<String> }`,
 			expected: &Schema{
 				Modules: []*Module{{
 					Name: "test",
 					Decls: []Decl{
 						&Verb{
-							Name: "myIngress",
-							Request: &DataRef{Module: "builtin", Name: "HttpRequest", TypeParameters: []Type{
-								&DataRef{
-									Pos: Position{
-										Offset: 41,
-										Line:   1,
-										Column: 42,
-									},
-									Name:           "string",
-									TypeParameters: []Type{},
-								},
-							}},
-							Response: &DataRef{Module: "builtin", Name: "HttpResponse", TypeParameters: []Type{
-								&DataRef{
-									Pos: Position{
-										Offset: 63,
-										Line:   1,
-										Column: 64,
-									},
-									Name:           "string",
-									TypeParameters: []Type{},
-								},
-							}},
+							Name:     "myIngress",
+							Request:  &DataRef{Module: "builtin", Name: "HttpRequest", TypeParameters: []Type{&String{}}},
+							Response: &DataRef{Module: "builtin", Name: "HttpResponse", TypeParameters: []Type{&String{}}},
 						},
 					},
 				}},
@@ -310,32 +294,14 @@ func TestParsing(t *testing.T) {
 							Comments: []string{},
 							Name:     "test",
 							Request: &DataRef{
-								Module: "test",
-								Name:   "Data",
-								TypeParameters: []Type{
-									&String{
-										Pos: Position{
-											Offset: 81,
-											Line:   7,
-											Column: 21,
-										},
-										Str: true,
-									},
-								},
+								Module:         "test",
+								Name:           "Data",
+								TypeParameters: []Type{&String{}},
 							},
 							Response: &DataRef{
-								Module: "test",
-								Name:   "Data",
-								TypeParameters: []Type{
-									&String{
-										Pos: Position{
-											Offset: 95,
-											Line:   7,
-											Column: 35,
-										},
-										Str: true,
-									},
-								},
+								Module:         "test",
+								Name:           "Data",
+								TypeParameters: []Type{&String{}},
 							},
 						},
 					},